Messages in this thread | | | Date | Fri, 28 Jul 2023 15:26:57 -0700 | From | Kees Cook <> | Subject | Re: [PATCH 2/6] scsi: mpt3sas: Use flexible arrays when less obviously possible |
| |
On Tue, Jul 25, 2023 at 09:13:27AM -0700, James Seo wrote: > These old-style 1-length variable arrays can be directly converted > into C99 flexible array members without any further source changes > and without any meaningful binary changes. All uses of the affected > structs were investigated, and the existing code somehow manages to > weather the reduced sizeof() the affected structs in every case.
Doing build comparisons here, I see a lot of binary changes. They may be, as you say, harmless, and since you've actually got hardware then this is a good verification of the changes, but I do wonder if this needs more detailed commit log (or split up patches).
However, the problem I see is that this code was already doing weird stuff with structs that appear to not have been using flex arrays actually. With "pahole" I can see struct MPT3SAS_ADAPTER changes:
- Mpi2IOUnitPage8_t iounit_pg8; /* 3668 40 */ - Mpi2IOCPage1_t ioc_pg1_copy; /* 3708 24 */ + Mpi2IOUnitPage8_t iounit_pg8; /* 3668 16 */ + Mpi2IOCPage1_t ioc_pg1_copy; /* 3684 24 */
struct _MPI2_CONFIG_PAGE_IO_UNIT_8 (Mpi2IOUnitPage8_t) is in the _middle_ of struct MPT3SAS_ADAPTER.... :|
In the earlier attempts at this conversion, it seemed that some of these are actually fixed-size:
https://lore.kernel.org/lkml/20210202235118.GA314410@embeddedor/
I think this patch needs to be broken up into per-struct changes, so they can be reviewed individually.
-Kees
-- Kees Cook
| |