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] [thread-next>] [day] [month] [year] [list]
Message-id: <174657431631.3924073.6654014165534485350@noble.neil.brown.name>
Date: Wed, 07 May 2025 09:31:56 +1000
From: "NeilBrown" <neil@...wn.name>
To: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: "Chuck Lever" <chuck.lever@...cle.com>, "Jeff Layton" <jlayton@...nel.org>,
 "Olga Kornievskaia" <okorniev@...hat.com>, "Dai Ngo" <Dai.Ngo@...cle.com>,
 "Tom Talpey" <tom@...pey.com>, linux-nfs@...r.kernel.org,
 linux-kernel@...r.kernel.org, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] NFSD: Avoid multiple -Wflex-array-member-not-at-end
 warnings

On Wed, 07 May 2025, 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
> structures. Notice that `struct knfsd_fh` is a flexible structure --a
> structure that contains a flexible-array member.

I don't really like this solution.

struct knfsd_fh has a fixed size determined by NFS4_FHSIZE.
The fact that fh_fsid is "flexible" doesn't mean it is unlimited in
size.  So moving it to the end of other structures is silencing a
warning but leaving the code as potentially confusing.

Maybe just make it
    u32 fh_fsid[4]; /* in practice the size varies but is always
                     * limited by fh_raw above
                     */

There are places was [0] [1] and [2] are explictly indexed, so size
needs to be atleast 3 else we invite warnings.  But maybe other memcpy
etc will trigger warnings anyway??
Maybe
    u32 fh_fsid[NFS4_FHSIZE/4-1];
That will always fit in the available space and we never use anywhere
close to that size.

I'd really rather use [] and have some way to tell the compiler that we
have the size under control and it doesn't need to worry.

Thanks,
NeilBrown


> 
> Fix the following warnings:
> 
> fs/nfsd/nfsfh.h:79:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/nfsd/state.h:763:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/nfsd/state.h:669:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/nfsd/state.h:549:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/nfsd/xdr4.h:705:33: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> fs/nfsd/xdr4.h:678:33: 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/nfsd/nfsfh.h |  4 +++-
>  fs/nfsd/state.h | 12 +++++++++---
>  fs/nfsd/xdr4.h  |  8 ++++++--
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 5103c2f4d225..bbee43674a2a 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -76,7 +76,6 @@ static inline ino_t u32_to_ino_t(__u32 uino)
>   * pre_mtime/post_version will be used to support wcc_attr's in NFSv3.
>   */
>  typedef struct svc_fh {
> -	struct knfsd_fh		fh_handle;	/* FH data */
>  	int			fh_maxsize;	/* max size for fh_handle */
>  	struct dentry *		fh_dentry;	/* validated dentry */
>  	struct svc_export *	fh_export;	/* export pointer */
> @@ -107,6 +106,9 @@ typedef struct svc_fh {
>  	/* Post-op attributes saved in fh_fill_post_attrs() */
>  	struct kstat		fh_post_attr;	/* full attrs after operation */
>  	u64			fh_post_change; /* nfsv4 change; see above */
> +
> +	/* Must be last -ends in a flexible-array member. */
> +	struct knfsd_fh		fh_handle;	/* FH data */
>  } svc_fh;
>  #define NFSD4_FH_FOREIGN (1<<0)
>  #define SET_FH_FLAG(c, f) ((c)->fh_flags |= (f))
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 1995bca158b8..ffd3fd8c34a0 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -546,9 +546,11 @@ struct nfs4_replay {
>  	__be32			rp_status;
>  	unsigned int		rp_buflen;
>  	char			*rp_buf;
> -	struct knfsd_fh		rp_openfh;
>  	int			rp_locked;
>  	char			rp_ibuf[NFSD4_REPLAY_ISIZE];
> +
> +	/* Must be last -ends in a flexible-array member. */
> +	struct knfsd_fh		rp_openfh;
>  };
>  
>  struct nfs4_stateowner;
> @@ -666,12 +668,14 @@ struct nfs4_file {
>  	u32			fi_share_deny;
>  	struct nfsd_file	*fi_deleg_file;
>  	int			fi_delegees;
> -	struct knfsd_fh		fi_fhandle;
>  	bool			fi_had_conflict;
>  #ifdef CONFIG_NFSD_PNFS
>  	struct list_head	fi_lo_states;
>  	atomic_t		fi_lo_recalls;
>  #endif
> +
> +	/* Must be last -ends in a flexible-array member. */
> +	struct knfsd_fh		fi_fhandle;
>  };
>  
>  /*
> @@ -760,9 +764,11 @@ struct nfsd4_blocked_lock {
>  	struct list_head	nbl_lru;
>  	time64_t		nbl_time;
>  	struct file_lock	nbl_lock;
> -	struct knfsd_fh		nbl_fh;
>  	struct nfsd4_callback	nbl_cb;
>  	struct kref		nbl_kref;
> +
> +	/* Must be last -ends in a flexible-array member. */
> +	struct knfsd_fh		nbl_fh;
>  };
>  
>  struct nfsd4_compound_state;
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index aa2a356da784..e453ea5ebab6 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -675,11 +675,13 @@ struct nfsd4_cb_offload {
>  	struct nfsd42_write_res	co_res;
>  	__be32			co_nfserr;
>  	unsigned int		co_retries;
> -	struct knfsd_fh		co_fh;
>  
>  	struct nfs4_sessionid	co_referring_sessionid;
>  	u32			co_referring_slotid;
>  	u32			co_referring_seqno;
> +
> +	/* Must be last -ends in a flexible-array member. */
> +	struct knfsd_fh		co_fh;
>  };
>  
>  struct nfsd4_copy {
> @@ -702,7 +704,6 @@ struct nfsd4_copy {
>  	/* response */
>  	__be32			nfserr;
>  	struct nfsd42_write_res	cp_res;
> -	struct knfsd_fh		fh;
>  
>  	/* offload callback */
>  	struct nfsd4_cb_offload	cp_cb_offload;
> @@ -723,6 +724,9 @@ struct nfsd4_copy {
>  	struct nfs_fh		c_fh;
>  	nfs4_stateid		stateid;
>  	struct nfsd_net		*cp_nn;
> +
> +	/* Must be last -ends in a flexible-array member. */
> +	struct knfsd_fh		fh;
>  };
>  
>  static inline void nfsd4_copy_set_sync(struct nfsd4_copy *copy, bool sync)
> -- 
> 2.43.0
> 
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ