linux-kernel - Re: [PATCH v2 00/12] scsi: mpt3sas: Use flexible arrays and do a few cleanups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202310230929.494FD6E14E@keescook>
Date:   Mon, 23 Oct 2023 09:30:07 -0700
From:   Kees Cook <keescook@...omium.org>
To:     "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     Borislav Petkov <bp@...en8.de>,
        Niklas Cassel <Niklas.Cassel@....com>,
        James Seo <james@...iv.tech>,
        Sathya Prakash <sathya.prakash@...adcom.com>,
        Sreekanth Reddy <sreekanth.reddy@...adcom.com>,
        Suganath Prabu Subramani 
        <suganath-prabu.subramani@...adcom.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        MPT-FusionLinux.pdl@...adcom.com, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/12] scsi: mpt3sas: Use flexible arrays and do a few
 cleanups

On Sun, Aug 06, 2023 at 10:05:52AM -0700, James Seo wrote:
> Commit df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3") has
> resulted in the only arrays that UBSAN_BOUNDS considers unbounded
> being trailing arrays declared with [] as the last member of a
> struct. Unbounded trailing arrays declared with [1] are common in
> mpt3sas, which is causing spurious warnings to appear in some
> situations, e.g. when more than one physical disk is connected:
> 
>   UBSAN: array-index-out-of-bounds in drivers/scsi/mpt3sas/mpt3sas_scsih.c:6810:36
>   index 1 is out of range for type 'MPI2_SAS_IO_UNIT0_PHY_DATA [1]'
> 
> which relates to this unbounded array access:
> 
>   port_id = sas_iounit_pg0->PhyData[i].Port;
> 
> and is just one example of 10 similar warnings currently occurring
> for me during boot.
> 
> This series converts most trailing arrays declared with [1] in mptsas
> into proper C99 flexible array members. Those that are not unbounded
> and really are fixed-length arrays of length 1 are left alone.
> 
> I didn't find any conversions that required further source edits
> besides changing [1] to [], and everything seems to work with my
> SAS2008-based add-in card, but please look things over in case I
> missed something subtle.
> 
> Rounding out the series are some opportunistic cleanups.
> 
> The only dependency is that patch 7 ("Use struct_size() for struct
> size calculations") depends on patches 3-5.
> 
> History:
> v1: https://lore.kernel.org/linux-scsi/20230725161331.27481-1-james@equiv.tech/
> 
> Changes v1->v2:
> - Slightly reword and add Reviewed-by: tags to commit messages
> - Split up a commit that was resulting in many binary changes
> - Remove the iounit_pg8 member of the per-adapter struct
> - Replace more dynamic allocations with local variables

Here's a tested-by: from Boris:

https://lore.kernel.org/all/20231023135615.GBZTZ7fwRh48euq3ew@fat_crate.local

-Kees

> 
> James Seo (12):
>   scsi: mpt3sas: Use flexible arrays when obviously possible
>   scsi: mpt3sas: Make MPI2_CONFIG_PAGE_IO_UNIT_8::Sensor[] a flexible
>     array
>   scsi: mpt3sas: Make MPI2_CONFIG_PAGE_RAID_VOL_0::PhysDisk[] a flexible
>     array
>   scsi: mpt3sas: Make MPI2_CONFIG_PAGE_SASIOUNIT_0::PhyData[] a flexible
>     array
>   scsi: mpt3sas: Make MPI2_CONFIG_PAGE_SASIOUNIT_1::PhyData[] a flexible
>     array
>   scsi: mpt3sas: Make MPI26_CONFIG_PAGE_PIOUNIT_1::PhyData[] a flexible
>     array
>   scsi: mpt3sas: Use struct_size() for struct size calculations
>   scsi: mpt3sas: Remove the iounit_pg8 member of the per-adapter struct
>   scsi: mpt3sas: Fix an outdated comment
>   scsi: mpt3sas: Fix typo of "TRIGGER"
>   scsi: mpt3sas: Replace a dynamic allocation with a local variable
>   scsi: mpt3sas: Replace dynamic allocations with local variables
> 
>  drivers/scsi/mpt3sas/mpi/mpi2_cnfg.h         | 231 ++++++-------------
>  drivers/scsi/mpt3sas/mpi/mpi2_image.h        |  32 +--
>  drivers/scsi/mpt3sas/mpi/mpi2_ioc.h          |  27 +--
>  drivers/scsi/mpt3sas/mpt3sas_base.c          |  35 ++-
>  drivers/scsi/mpt3sas/mpt3sas_base.h          |   2 -
>  drivers/scsi/mpt3sas/mpt3sas_config.c        |   6 +-
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c         |  55 ++---
>  drivers/scsi/mpt3sas/mpt3sas_transport.c     |   9 +-
>  drivers/scsi/mpt3sas/mpt3sas_trigger_pages.h |  44 ++--
>  drivers/scsi/mpt3sas/mpt3sas_warpdrive.c     |   3 +-
>  10 files changed, 151 insertions(+), 293 deletions(-)
> 
> 
> base-commit: 6cae9a3910ac1b5daf5ac3db9576b78cc4eff5aa
> -- 
> 2.39.2
> 

-- 
Kees Cook