iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

[PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 3 months ago

-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
getting ready to enable it, globally.

Move the conflicting declaration to the end of the corresponding
structure. Notice that struct iommufd_vevent is a flexible
structure, this is a structure that contains a flexible-array
member.

Fix the following warning:

drivers/iommu/iommufd/iommufd_private.h:621:31: 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/iommu/iommufd/iommufd_private.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 627f9b78483a..85d0843ed07b 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -614,7 +614,6 @@ struct iommufd_veventq {
 	struct iommufd_eventq common;
 	struct iommufd_viommu *viommu;
 	struct list_head node; /* for iommufd_viommu::veventqs */
-	struct iommufd_vevent lost_events_header;
 
 	enum iommu_veventq_type type;
 	unsigned int depth;
@@ -622,6 +621,9 @@ struct iommufd_veventq {
 	/* Use common.lock for protection */
 	u32 num_events;
 	u32 sequence;
+
+	/* Must be last as it ends in a flexible-array member. */
+	struct iommufd_vevent lost_events_header;
 };
 
 static inline struct iommufd_veventq *
-- 
2.43.0

Re: [PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Jason Gunthorpe 2 months, 2 weeks ago

On Mon, Nov 10, 2025 at 08:35:31PM +0900, 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 declaration to the end of the corresponding
> structure. Notice that struct iommufd_vevent is a flexible
> structure, this is a structure that contains a flexible-array
> member.
> 
> Fix the following warning:
> 
> drivers/iommu/iommufd/iommufd_private.h:621:31: 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/iommu/iommufd/iommufd_private.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied thanks

Jason

RE: [PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Tian, Kevin 2 months, 3 weeks ago

> From: Gustavo A. R. Silva <gustavoars@kernel.org>
> Sent: Monday, November 10, 2025 7:36 PM
> 
> -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> getting ready to enable it, globally.
> 
> Move the conflicting declaration to the end of the corresponding
> structure. Notice that struct iommufd_vevent is a flexible
> structure, this is a structure that contains a flexible-array
> member.
> 
> Fix the following warning:
> 
> drivers/iommu/iommufd/iommufd_private.h:621:31: 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>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Re: [PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Nicolin Chen 3 months ago

On Mon, Nov 10, 2025 at 08:35:31PM +0900, 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 declaration to the end of the corresponding
> structure. Notice that struct iommufd_vevent is a flexible
> structure, this is a structure that contains a flexible-array
> member.
> 
> Fix the following warning:
> 
> drivers/iommu/iommufd/iommufd_private.h:621:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

IIUIC, there might be data corruption due to this? If so, I think
we should do:

[PATCH rc] iommufd: Fix flex-array-member-not-at-end in struct iommufd_veventq

And add:

Fixes: e36ba5ab808e ("iommufd: Add IOMMUFD_OBJ_VEVENTQ and IOMMUFD_CMD_VEVENTQ_ALLOC")
Cc: stable@vger.kernel.org

> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>

With that,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Thanks for the patch!

Re: [PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 3 months ago


On 11/11/25 03:02, Nicolin Chen wrote:
> On Mon, Nov 10, 2025 at 08:35:31PM +0900, 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 declaration to the end of the corresponding
>> structure. Notice that struct iommufd_vevent is a flexible
>> structure, this is a structure that contains a flexible-array
>> member.
>>
>> Fix the following warning:
>>
>> drivers/iommu/iommufd/iommufd_private.h:621:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> IIUIC, there might be data corruption due to this? If so, I think

Yep. Also, after taking a look at the commit you mention, the counted_by annotation
in struct iommufd_vevent is wrong in commit e8e1ef9b77a7 ("iommufd/viommu: Add
iommufd_viommu_report_event helper"). The counter, in this case vevent->data_len
must always be initialized before the first reference to the flexible array:

diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c
index 6f1010da221c..21d4a35538f6 100644
--- a/drivers/iommu/iommufd/driver.c
+++ b/drivers/iommu/iommufd/driver.c
@@ -161,8 +161,8 @@ int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
                 vevent = &veventq->lost_events_header;
                 goto out_set_header;
         }
-       memcpy(vevent->event_data, event_data, data_len);
         vevent->data_len = data_len;
+       memcpy(vevent->event_data, event_data, data_len);
         veventq->num_events++;

  out_set_header:

I'll turn this into a small patch series to fix the above issue as well.

Thanks
-Gustavo

Re: [PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Gustavo A. R. Silva 3 months ago


On 11/11/25 16:20, Gustavo A. R. Silva wrote:
> 
> 
> On 11/11/25 03:02, Nicolin Chen wrote:
>> On Mon, Nov 10, 2025 at 08:35:31PM +0900, 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 declaration to the end of the corresponding
>>> structure. Notice that struct iommufd_vevent is a flexible
>>> structure, this is a structure that contains a flexible-array
>>> member.
>>>
>>> Fix the following warning:
>>>
>>> drivers/iommu/iommufd/iommufd_private.h:621:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array- 
>>> member-not-at-end]
>>
>> IIUIC, there might be data corruption due to this? If so, I think

Okay, I didn't find evidence of data corruption. So, this patch can be applied to
a -next tree.

> 
> Yep. Also, after taking a look at the commit you mention, the counted_by annotation
> in struct iommufd_vevent is wrong in commit e8e1ef9b77a7 ("iommufd/viommu: Add
> iommufd_viommu_report_event helper"). The counter, in this case vevent->data_len
> must always be initialized before the first reference to the flexible array:
> 
> diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c
> index 6f1010da221c..21d4a35538f6 100644
> --- a/drivers/iommu/iommufd/driver.c
> +++ b/drivers/iommu/iommufd/driver.c
> @@ -161,8 +161,8 @@ int iommufd_viommu_report_event(struct iommufd_viommu *viommu,
>                  vevent = &veventq->lost_events_header;
>                  goto out_set_header;
>          }
> -       memcpy(vevent->event_data, event_data, data_len);
>          vevent->data_len = data_len;
> +       memcpy(vevent->event_data, event_data, data_len);
>          veventq->num_events++;
> 
>   out_set_header:

I will submit the above as a separate patch.

Thanks
-Gustavo

Re: [PATCH][next] iommufd/iommufd_private.h: Avoid -Wflex-array-member-not-at-end warning

Posted by Nicolin Chen 3 months ago

On Tue, Nov 11, 2025 at 05:49:20PM +0900, Gustavo A. R. Silva wrote:
> On 11/11/25 16:20, Gustavo A. R. Silva wrote:
> > On 11/11/25 03:02, Nicolin Chen wrote:
> > > On Mon, Nov 10, 2025 at 08:35:31PM +0900, 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 declaration to the end of the corresponding
> > > > structure. Notice that struct iommufd_vevent is a flexible
> > > > structure, this is a structure that contains a flexible-array
> > > > member.
> > > > 
> > > > Fix the following warning:
> > > > 
> > > > drivers/iommu/iommufd/iommufd_private.h:621:31: warning:
> > > > structure containing a flexible array member is not at the end
> > > > of another structure [-Wflex-array- member-not-at-end]
> > > 
> > > IIUIC, there might be data corruption due to this? If so, I think
> 
> Okay, I didn't find evidence of data corruption. So, this patch can be applied to
> a -next tree.

Revisiting the design, the "lost_events_header" doesn't allocate
data but only uses its internal header to raise a flag. So there
should not be any data corruption.

Yea, I think we are fine with your for-next patch.

Thanks
Nicolin