linux-kernel - Re: [RESEND PATCH][next] fs: nfs: acl: Avoid -Wflex-array-member-not-at-end warning

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: <b16ac12f-166a-4d25-bf33-b1ccf6e0dac7@oracle.com>
Date: Mon, 10 Feb 2025 14:48:39 -0500
From: Chuck Lever <chuck.lever@...cle.com>
To: Trond Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-hardening@...r.kernel.org,
        "Gustavo A. R. Silva"
 <gustavoars@...nel.org>,
        Dai Ngo <Dai.Ngo@...cle.com>, Jeff Layton <jlayton@...nel.org>,
        Olga Kornievskaia <okorniev@...hat.com>, Tom Talpey <tom@...pey.com>,
        Neil Brown <neilb@...e.de>
Subject: Re: [RESEND PATCH][next] fs: nfs: acl: Avoid
 -Wflex-array-member-not-at-end warning

On 2/3/25 10:03 PM, 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.
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of other structs, we use the `struct_group_tagged()` helper
> to create a new tagged `struct posix_acl_hdr`. This structure
> groups together all the members of the flexible `struct posix_acl`
> 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 member currently causing
> trouble from `struct posix_acl` to `struct posix_acl_hdr`.
> 
> We also want to ensure that when 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 posix_acl_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.
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure, through which we can access the flexible-array
> member, if necessary.
> 
> So, with these changes, fix the following warning:
> 
> fs/nfs_common/nfsacl.c:45:26: 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>
> ---
>  fs/nfs_common/nfsacl.c    |  8 +++++---
>  include/linux/posix_acl.h | 11 ++++++++---
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfs_common/nfsacl.c b/fs/nfs_common/nfsacl.c
> index ea382b75b26c..e2eaac14fd8e 100644
> --- a/fs/nfs_common/nfsacl.c
> +++ b/fs/nfs_common/nfsacl.c
> @@ -42,7 +42,7 @@ struct nfsacl_encode_desc {
>  };
>  
>  struct nfsacl_simple_acl {
> -	struct posix_acl acl;
> +	struct posix_acl_hdr acl;
>  	struct posix_acl_entry ace[4];
>  };
>  
> @@ -112,7 +112,8 @@ int nfsacl_encode(struct xdr_buf *buf, unsigned int base, struct inode *inode,
>  	    xdr_encode_word(buf, base, entries))
>  		return -EINVAL;
>  	if (encode_entries && acl && acl->a_count == 3) {
> -		struct posix_acl *acl2 = &aclbuf.acl;
> +		struct posix_acl *acl2 =
> +			container_of(&aclbuf.acl, struct posix_acl, hdr);
>  
>  		/* Avoid the use of posix_acl_alloc().  nfsacl_encode() is
>  		 * invoked in contexts where a memory allocation failure is
> @@ -177,7 +178,8 @@ bool nfs_stream_encode_acl(struct xdr_stream *xdr, struct inode *inode,
>  		return false;
>  
>  	if (encode_entries && acl && acl->a_count == 3) {
> -		struct posix_acl *acl2 = &aclbuf.acl;
> +		struct posix_acl *acl2 =
> +			container_of(&aclbuf.acl, struct posix_acl, hdr);
>  
>  		/* Avoid the use of posix_acl_alloc().  nfsacl_encode() is
>  		 * invoked in contexts where a memory allocation failure is
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index e2d47eb1a7f3..62d497763e25 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -27,11 +27,16 @@ struct posix_acl_entry {
>  };
>  
>  struct posix_acl {
> -	refcount_t		a_refcount;
> -	unsigned int		a_count;
> -	struct rcu_head		a_rcu;
> +	/* New members MUST be added within the struct_group() macro below. */
> +	struct_group_tagged(posix_acl_hdr, hdr,
> +		refcount_t		a_refcount;
> +		unsigned int		a_count;
> +		struct rcu_head		a_rcu;
> +	);
>  	struct posix_acl_entry	a_entries[] __counted_by(a_count);
>  };
> +static_assert(offsetof(struct posix_acl, a_entries) == sizeof(struct posix_acl_hdr),
> +	      "struct member likely outside of struct_group_tagged()");
>  
>  #define FOREACH_ACL_ENTRY(pa, acl, pe) \
>  	for(pa=(acl)->a_entries, pe=pa+(acl)->a_count; pa<pe; pa++)

Trond, Anna -

Let me know if I need to take this one via the NFSD tree. If not,

Acked-by: Chuck Lever <chuck.lever@...cle.com>


-- 
Chuck Lever