[<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