linux-hardening - Re: [PATCH][next] scsi: fc: Avoid -Wflex-array-member-not-at-end warnings

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] [day] [month] [year] [list]
Message-ID: <yq1seheonya.fsf@ca-mkp.ca.oracle.com>
Date: Mon, 25 Aug 2025 21:53:21 -0400
From: "Martin K. Petersen" <martin.petersen@...cle.com>
To: James Smart <james.smart@...adcom.com>,
        Justin Tee
 <justin.tee@...adcom.com>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Hannes Reinecke
 <hare@...e.de>, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] scsi: fc: Avoid -Wflex-array-member-not-at-end
 warnings


James & Justin,

Please help review Gustavo's patch.

I also noticed that Justin is not listed as lpfc maintainer. Please
submit patch. Thanks!

> -Wflex-array-member-not-at-end has been introduced in GCC-14, and we
> are getting ready to enable it, globally.
>
> So, in order to avoid ending up with a flexible-array member in the
> middle of multiple other structs, we use the `__struct_group()`
> helper to create a new tagged `struct fc_df_desc_fpin_reg_hdr`.
> This structure groups together all the members of the flexible
> `struct fc_df_desc_fpin_reg` except the flexible array.
>
> As a result, the array is effectively separated from the rest of the
> members without modifying the memory layout of the flexible structure.
> We then change the type of the middle struct members currently causing
> trouble from `struct fc_df_desc_fpin_reg` to `struct
> fc_df_desc_fpin_reg_hdr`.
>
> We also want to ensure that in case new members need to be added to the
> flexible structure, they are always included within the newly created
> tagged struct. For this, we use `_Static_assert()`. This ensures that
> the memory layout for both the flexible structure and the new tagged
> struct is the same after any changes.
>
> This approach avoids having to implement `struct fc_df_desc_fpin_reg_hdr`
> as a completely separate structure, thus preventing having to maintain
> two independent but basically identical structures, closing the door
> to potential bugs in the future.
>
> The above is also done for flexible `struct fc_els_rdf`.
>
> So, with these changes, fix the following warnings:
> drivers/scsi/lpfc/lpfc_hw4.h:4936:41: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/scsi/lpfc/lpfc_hw4.h:4942:41: warning: structure containing a
> flexible array member is not at the end of another structure
> [-Wflex-array-member-not-at-end]
> drivers/scsi/lpfc/lpfc_hw4.h:4947:41: 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@...nel.org>
> ---
>  drivers/scsi/lpfc/lpfc_hw4.h  |  4 ++--
>  include/uapi/scsi/fc/fc_els.h | 40 ++++++++++++++++++++++++-----------
>  2 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_hw4.h b/drivers/scsi/lpfc/lpfc_hw4.h
> index dcd7204d4eec..e319858c88ba 100644
> --- a/drivers/scsi/lpfc/lpfc_hw4.h
> +++ b/drivers/scsi/lpfc/lpfc_hw4.h
> @@ -4909,13 +4909,13 @@ struct send_frame_wqe {
>  
>  #define ELS_RDF_REG_TAG_CNT		4
>  struct lpfc_els_rdf_reg_desc {
> -	struct fc_df_desc_fpin_reg	reg_desc;	/* descriptor header */
> +	struct fc_df_desc_fpin_reg_hdr	reg_desc;	/* descriptor header */
>  	__be32				desc_tags[ELS_RDF_REG_TAG_CNT];
>  							/* tags in reg_desc */
>  };
>  
>  struct lpfc_els_rdf_req {
> -	struct fc_els_rdf		rdf;	   /* hdr up to descriptors */
> +	struct fc_els_rdf_hdr		rdf;	   /* hdr up to descriptors */
>  	struct lpfc_els_rdf_reg_desc	reg_d1;	/* 1st descriptor */
>  };
>  
> diff --git a/include/uapi/scsi/fc/fc_els.h b/include/uapi/scsi/fc/fc_els.h
> index 16782c360de3..81b9f87943f4 100644
> --- a/include/uapi/scsi/fc/fc_els.h
> +++ b/include/uapi/scsi/fc/fc_els.h
> @@ -11,6 +11,12 @@
>  #include <linux/types.h>
>  #include <asm/byteorder.h>
>  
> +#ifdef __KERNEL__
> +#include <linux/stddef.h>	/* for offsetof */
> +#else
> +#include <stddef.h>		/* for offsetof */
> +#endif
> +
>  /*
>   * Fibre Channel Switch - Enhanced Link Services definitions.
>   * From T11 FC-LS Rev 1.2 June 7, 2005.
> @@ -1109,12 +1115,15 @@ struct fc_els_fpin {
>  
>  /* Diagnostic Function Descriptor - FPIN Registration */
>  struct fc_df_desc_fpin_reg {
> -	__be32		desc_tag;	/* FPIN Registration (0x00030001) */
> -	__be32		desc_len;	/* Length of Descriptor (in bytes).
> -					 * Size of descriptor excluding
> -					 * desc_tag and desc_len fields.
> -					 */
> -	__be32		count;		/* Number of desc_tags elements */
> +	/* New members MUST be added within the __struct_group() macro below. */
> +	__struct_group(fc_df_desc_fpin_reg_hdr, hdr, /* no attrs */,
> +		__be32		desc_tag; /* FPIN Registration (0x00030001) */
> +		__be32		desc_len; /* Length of Descriptor (in bytes).
> +					   * Size of descriptor excluding
> +					   * desc_tag and desc_len fields.
> +					   */
> +		__be32		count;	  /* Number of desc_tags elements */
> +	);
>  	__be32		desc_tags[];	/* Array of Descriptor Tags.
>  					 * Each tag indicates a function
>  					 * supported by the N_Port (request)
> @@ -1124,19 +1133,26 @@ struct fc_df_desc_fpin_reg {
>  					 * See ELS_FN_DTAG_xxx for tag values.
>  					 */
>  };
> +_Static_assert(offsetof(struct fc_df_desc_fpin_reg, desc_tags) == sizeof(struct fc_df_desc_fpin_reg_hdr),
> +	      "struct member likely outside of __struct_group()");
>  
>  /*
>   * ELS_RDF - Register Diagnostic Functions
>   */
>  struct fc_els_rdf {
> -	__u8		fpin_cmd;	/* command (0x19) */
> -	__u8		fpin_zero[3];	/* specified as zero - part of cmd */
> -	__be32		desc_len;	/* Length of Descriptor List (in bytes).
> -					 * Size of ELS excluding fpin_cmd,
> -					 * fpin_zero and desc_len fields.
> -					 */
> +	/* New members MUST be added within the __struct_group() macro below. */
> +	__struct_group(fc_els_rdf_hdr, hdr, /* no attrs */,
> +		__u8		fpin_cmd;	/* command (0x19) */
> +		__u8		fpin_zero[3];	/* specified as zero - part of cmd */
> +		__be32		desc_len;	/* Length of Descriptor List (in bytes).
> +						 * Size of ELS excluding fpin_cmd,
> +						 * fpin_zero and desc_len fields.
> +						 */
> +	);
>  	struct fc_tlv_desc	desc[];	/* Descriptor list */
>  };
> +_Static_assert(offsetof(struct fc_els_rdf, desc) == sizeof(struct fc_els_rdf_hdr),
> +	       "struct member likely outside of __struct_group()");
>  
>  /*
>   * ELS RDF LS_ACC Response.

-- 
Martin K. Petersen