* [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning @ 2025-09-03 18:44 Gustavo A. R. Silva 2025-09-04 5:27 ` Jinpu Wang 2025-09-04 6:52 ` John Garry 0 siblings, 2 replies; 6+ messages in thread From: Gustavo A. R. Silva @ 2025-09-03 18:44 UTC (permalink / raw) To: Jack Wang, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linux-kernel, Gustavo A. R. Silva, linux-hardening -Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally. Move the conflicting declarations to the end of the corresponding structures. Notice that `struct ssp_response_iu` is a flexible structure, this is a structure that contains a flexible-array member. With these changes fix the following warnings: drivers/scsi/pm8001/pm8001_hwi.h:342:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] drivers/scsi/pm8001/pm80xx_hwi.h:561:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/scsi/pm8001/pm8001_hwi.h | 4 +++- drivers/scsi/pm8001/pm80xx_hwi.h | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_hwi.h b/drivers/scsi/pm8001/pm8001_hwi.h index fc2127dcb58d..7dc7870a8f86 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.h +++ b/drivers/scsi/pm8001/pm8001_hwi.h @@ -339,8 +339,10 @@ struct ssp_completion_resp { __le32 status; __le32 param; __le32 ssptag_rescv_rescpad; - struct ssp_response_iu ssp_resp_iu; __le32 residual_count; + + /* Must be last --ends in a flexible-array member. */ + struct ssp_response_iu ssp_resp_iu; } __attribute__((packed, aligned(4))); diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h index eb8fd37b2066..21afc28d9875 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.h +++ b/drivers/scsi/pm8001/pm80xx_hwi.h @@ -558,8 +558,10 @@ struct ssp_completion_resp { __le32 status; __le32 param; __le32 ssptag_rescv_rescpad; - struct ssp_response_iu ssp_resp_iu; __le32 residual_count; + + /* Must be last --ends in a flexible-array member. */ + struct ssp_response_iu ssp_resp_iu; } __attribute__((packed, aligned(4))); #define SSP_RESCV_BIT 0x00010000 -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning 2025-09-03 18:44 [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning Gustavo A. R. Silva @ 2025-09-04 5:27 ` Jinpu Wang 2025-09-04 6:52 ` John Garry 1 sibling, 0 replies; 6+ messages in thread From: Jinpu Wang @ 2025-09-04 5:27 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Jack Wang, James E.J. Bottomley, Martin K. Petersen, linux-scsi, linux-kernel, linux-hardening On Wed, Sep 3, 2025 at 8:44 PM Gustavo A. R. Silva <gustavoars@kernel.org> wrote: > > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are > getting ready to enable it, globally. > > Move the conflicting declarations to the end of the corresponding > structures. Notice that `struct ssp_response_iu` is a flexible > structure, this is a structure that contains a flexible-array member. > > With these changes fix the following warnings: > > drivers/scsi/pm8001/pm8001_hwi.h:342:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > drivers/scsi/pm8001/pm80xx_hwi.h:561:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Acked-by: Jack Wang <jinpu.wang@ionos.com> > --- > drivers/scsi/pm8001/pm8001_hwi.h | 4 +++- > drivers/scsi/pm8001/pm80xx_hwi.h | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_hwi.h b/drivers/scsi/pm8001/pm8001_hwi.h > index fc2127dcb58d..7dc7870a8f86 100644 > --- a/drivers/scsi/pm8001/pm8001_hwi.h > +++ b/drivers/scsi/pm8001/pm8001_hwi.h > @@ -339,8 +339,10 @@ struct ssp_completion_resp { > __le32 status; > __le32 param; > __le32 ssptag_rescv_rescpad; > - struct ssp_response_iu ssp_resp_iu; > __le32 residual_count; > + > + /* Must be last --ends in a flexible-array member. */ > + struct ssp_response_iu ssp_resp_iu; > } __attribute__((packed, aligned(4))); > > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h > index eb8fd37b2066..21afc28d9875 100644 > --- a/drivers/scsi/pm8001/pm80xx_hwi.h > +++ b/drivers/scsi/pm8001/pm80xx_hwi.h > @@ -558,8 +558,10 @@ struct ssp_completion_resp { > __le32 status; > __le32 param; > __le32 ssptag_rescv_rescpad; > - struct ssp_response_iu ssp_resp_iu; > __le32 residual_count; > + > + /* Must be last --ends in a flexible-array member. */ > + struct ssp_response_iu ssp_resp_iu; > } __attribute__((packed, aligned(4))); > > #define SSP_RESCV_BIT 0x00010000 > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning 2025-09-03 18:44 [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning Gustavo A. R. Silva 2025-09-04 5:27 ` Jinpu Wang @ 2025-09-04 6:52 ` John Garry 2025-09-04 12:39 ` James Bottomley 1 sibling, 1 reply; 6+ messages in thread From: John Garry @ 2025-09-04 6:52 UTC (permalink / raw) To: Gustavo A. R. Silva, Jack Wang, James E.J. Bottomley, Martin K. Petersen Cc: linux-scsi, linux-kernel, linux-hardening On 03/09/2025 19:44, Gustavo A. R. Silva wrote: > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are > getting ready to enable it, globally. > > Move the conflicting declarations to the end of the corresponding > structures. Notice that `struct ssp_response_iu` is a flexible > structure, this is a structure that contains a flexible-array member. > > With these changes fix the following warnings: > > drivers/scsi/pm8001/pm8001_hwi.h:342:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > drivers/scsi/pm8001/pm80xx_hwi.h:561:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/scsi/pm8001/pm8001_hwi.h | 4 +++- > drivers/scsi/pm8001/pm80xx_hwi.h | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_hwi.h b/drivers/scsi/pm8001/pm8001_hwi.h > index fc2127dcb58d..7dc7870a8f86 100644 > --- a/drivers/scsi/pm8001/pm8001_hwi.h > +++ b/drivers/scsi/pm8001/pm8001_hwi.h > @@ -339,8 +339,10 @@ struct ssp_completion_resp { > __le32 status; > __le32 param; > __le32 ssptag_rescv_rescpad; > - struct ssp_response_iu ssp_resp_iu; > __le32 residual_count; > + > + /* Must be last --ends in a flexible-array member. */ > + struct ssp_response_iu ssp_resp_iu; this is a HW structure, right? I did not think that it is ok to simply re-order them... > } __attribute__((packed, aligned(4))); > > > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h > index eb8fd37b2066..21afc28d9875 100644 > --- a/drivers/scsi/pm8001/pm80xx_hwi.h > +++ b/drivers/scsi/pm8001/pm80xx_hwi.h > @@ -558,8 +558,10 @@ struct ssp_completion_resp { > __le32 status; > __le32 param; > __le32 ssptag_rescv_rescpad; > - struct ssp_response_iu ssp_resp_iu; > __le32 residual_count; > + > + /* Must be last --ends in a flexible-array member. */ > + struct ssp_response_iu ssp_resp_iu; > } __attribute__((packed, aligned(4))); > > #define SSP_RESCV_BIT 0x00010000 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning 2025-09-04 6:52 ` John Garry @ 2025-09-04 12:39 ` James Bottomley 2025-09-04 13:49 ` John Garry 2025-09-04 15:57 ` Gustavo A. R. Silva 0 siblings, 2 replies; 6+ messages in thread From: James Bottomley @ 2025-09-04 12:39 UTC (permalink / raw) To: John Garry, Gustavo A. R. Silva, Jack Wang, Martin K. Petersen Cc: linux-scsi, linux-kernel, linux-hardening On Thu, 2025-09-04 at 07:52 +0100, John Garry wrote: > On 03/09/2025 19:44, Gustavo A. R. Silva wrote: > > diff --git a/drivers/scsi/pm8001/pm8001_hwi.h > > b/drivers/scsi/pm8001/pm8001_hwi.h > > index fc2127dcb58d..7dc7870a8f86 100644 > > --- a/drivers/scsi/pm8001/pm8001_hwi.h > > +++ b/drivers/scsi/pm8001/pm8001_hwi.h > > @@ -339,8 +339,10 @@ struct ssp_completion_resp { > > __le32 status; > > __le32 param; > > __le32 ssptag_rescv_rescpad; > > - struct ssp_response_iu ssp_resp_iu; > > __le32 residual_count; > > + > > + /* Must be last --ends in a flexible-array member. */ > > + struct ssp_response_iu ssp_resp_iu; > > this is a HW structure, right? I did not think that it is ok to > simply re-order them... Agreed, this is a standards defined information unit corresponding to an on the wire data structure. The patch is clearly wrong. That being said, the three things the flexible member can contain are no data, response data or sense data. None of them has a residual count at the beginning and, indeed, this field is never referred to in the driver, so it looks like it can simply be deleted to fix the warning. That being said, this pattern of adding fields after flexible members to represent data that's common to all content types of the union is not unknown in SCSI so if you want to enable this warning, what are we supposed to do when we encounter a genuine use case? Regards, James ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning 2025-09-04 12:39 ` James Bottomley @ 2025-09-04 13:49 ` John Garry 2025-09-04 15:57 ` Gustavo A. R. Silva 1 sibling, 0 replies; 6+ messages in thread From: John Garry @ 2025-09-04 13:49 UTC (permalink / raw) To: James Bottomley, Gustavo A. R. Silva, Jack Wang, Martin K. Petersen Cc: linux-scsi, linux-kernel, linux-hardening On 04/09/2025 13:39, James Bottomley wrote: > On Thu, 2025-09-04 at 07:52 +0100, John Garry wrote: >> On 03/09/2025 19:44, Gustavo A. R. Silva wrote: >>> diff --git a/drivers/scsi/pm8001/pm8001_hwi.h >>> b/drivers/scsi/pm8001/pm8001_hwi.h >>> index fc2127dcb58d..7dc7870a8f86 100644 >>> --- a/drivers/scsi/pm8001/pm8001_hwi.h >>> +++ b/drivers/scsi/pm8001/pm8001_hwi.h >>> @@ -339,8 +339,10 @@ struct ssp_completion_resp { >>> __le32 status; >>> __le32 param; >>> __le32 ssptag_rescv_rescpad; >>> - struct ssp_response_iu ssp_resp_iu; >>> __le32 residual_count; >>> + >>> + /* Must be last --ends in a flexible-array member. */ >>> + struct ssp_response_iu ssp_resp_iu; >> this is a HW structure, right? I did not think that it is ok to >> simply re-order them... > Agreed, this is a standards defined information unit corresponding to > an on the wire data structure. The patch is clearly wrong. > > That being said, the three things the flexible member can contain are > no data, response data or sense data. None of them has a residual > count at the beginning and, indeed, this field is never referred to in > the driver, so it looks like it can simply be deleted to fix the > warning. Seems reasonable. I don't see how the sizeof(struct ssp_completion_resp) is relevant, as the size of the memory to hold this structure (and other response types) will be defined elsewhere. > > That being said, this pattern of adding fields after flexible members > to represent data that's common to all content types of the union is > not unknown in SCSI so if you want to enable this warning, what are we > supposed to do when we encounter a genuine use case? Such a problem was solved in commit cd6856d38881, but I can't say that it is a good example as we simply dropped the flex array usage. Thanks, John ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning 2025-09-04 12:39 ` James Bottomley 2025-09-04 13:49 ` John Garry @ 2025-09-04 15:57 ` Gustavo A. R. Silva 1 sibling, 0 replies; 6+ messages in thread From: Gustavo A. R. Silva @ 2025-09-04 15:57 UTC (permalink / raw) To: James Bottomley, John Garry, Gustavo A. R. Silva, Jack Wang, Martin K. Petersen Cc: linux-scsi, linux-kernel, linux-hardening On 9/4/25 14:39, James Bottomley wrote: > On Thu, 2025-09-04 at 07:52 +0100, John Garry wrote: >> On 03/09/2025 19:44, Gustavo A. R. Silva wrote: >>> diff --git a/drivers/scsi/pm8001/pm8001_hwi.h >>> b/drivers/scsi/pm8001/pm8001_hwi.h >>> index fc2127dcb58d..7dc7870a8f86 100644 >>> --- a/drivers/scsi/pm8001/pm8001_hwi.h >>> +++ b/drivers/scsi/pm8001/pm8001_hwi.h >>> @@ -339,8 +339,10 @@ struct ssp_completion_resp { >>> __le32 status; >>> __le32 param; >>> __le32 ssptag_rescv_rescpad; >>> - struct ssp_response_iu ssp_resp_iu; >>> __le32 residual_count; >>> + >>> + /* Must be last --ends in a flexible-array member. */ >>> + struct ssp_response_iu ssp_resp_iu; >> >> this is a HW structure, right? I did not think that it is ok to >> simply re-order them... > > Agreed, this is a standards defined information unit corresponding to > an on the wire data structure. The patch is clearly wrong. > > That being said, the three things the flexible member can contain are > no data, response data or sense data. None of them has a residual > count at the beginning and, indeed, this field is never referred to in > the driver, so it looks like it can simply be deleted to fix the > warning. I see. I just submitted v2. Thanks! > > That being said, this pattern of adding fields after flexible members > to represent data that's common to all content types of the union is > not unknown in SCSI so if you want to enable this warning, what are we > supposed to do when we encounter a genuine use case? It depends on the situation. Some people prefer to have a separate struct with only the header part of the flexible structure --this is excluding the flexible-array member (FAM), and then use an object of that header type embedded anywhere in any other struct. This of course (sometimes) implies having to do many other changes to accommodate the rest of the code accordingly, as in this[1] case. To address the situation described above without necessarily having to create a separate header struct and change a lot of code, the struct_group() helper can be used [2]. In other cases, when for some reason the FAM has to be accessed through a composite struct, both struct_group() and container_of() (this to retrieve a pointer to the flexible structure and then access the FAM) can be used [3]. We have the new TRAILING_OVERLAP() helper that in many cases can be superior to the struct_group()/container_of() approach. For instance this[4] could've been fixed with the following shorter and simpler patch: struct nfsacl_simple_acl { - struct posix_acl acl; - struct posix_acl_entry ace[4]; + TRAILING_OVERLAP(struct posix_acl, acl, a_entries, + struct posix_acl_entry ace[4]; + ); }; However, at the time we didn't have TRAILING_OVERLAP(). Also, this new macro is helping us to detect alignment issues and correct them [5]. These[6][7] are a couple more example of when and how to use this helper. We also have the DEFINE_FLEX()/DEFINE_RAW_FLEX() helpers [8][9]. So, again, we can do different things depending on the situation and maintainer's preferences. Ideally, FAMs-in-the-middle should be avoided, because it's so easy for them to open the door to memory corruption bugs[10] (to mention some). Thanks -Gustavo [1] https://git.kernel.org/linus/d2af710d6d50 [2] https://git.kernel.org/linus/c54979a3abc4 [3] https://git.kernel.org/linus/a7e8997ae18c [4] https://git.kernel.org/linus/dfd500d89545 [5] https://lore.kernel.org/linux-hardening/aLiYrQGdGmaDTtLF@kspp/ [6] https://git.kernel.org/linus/5e54510a9389 [7] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=8abbbbb588f1 [8] https://git.kernel.org/linus/1d717123bb1a [9] https://git.kernel.org/linus/34116ec67cc1 [10a] https://git.kernel.org/linus/eea03d18af9c [10b] https://git.kernel.org/linus/6e4bf018bb04 [10c] https://git.kernel.org/linus/cf44e745048d [10d] https://git.kernel.org/linus/d761bb01c85b ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-04 15:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-03 18:44 [PATCH][next] scsi: pm80xx: Avoid -Wflex-array-member-not-at-end warning Gustavo A. R. Silva 2025-09-04 5:27 ` Jinpu Wang 2025-09-04 6:52 ` John Garry 2025-09-04 12:39 ` James Bottomley 2025-09-04 13:49 ` John Garry 2025-09-04 15:57 ` Gustavo A. R. Silva
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.