At one time, nfsd would scrape inode information directly out of struct inode in order to populate the change_info4. At that time, the BUG_ON in set_change_info made some sense, since having it unset meant a coding error. More recently, it calls vfs_getattr to get this information, which can fail. If that fails, fh_pre_saved can end up not being set. While this situation is unfortunate, we don't need to crash the box. Move set_change_info to nfs4proc.c since all of the callers are there. Revise the condition for setting "atomic" to also check for fh_pre_saved. Drop the BUG_ON and make it a WARN_ON, and just have it zero out both change_attr4s when this occurs. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2223560 Reported-by: Boyang Xue <bxue@xxxxxxxxxx> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> --- fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++++++++++ fs/nfsd/xdr4.h | 11 ----------- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index d8e7a533f9d2..e6f406f27821 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -380,6 +380,36 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, return status; } +/** + * set_change_info - set up the change_info4 for a reply + * @cinfo: pointer to nfsd4_change_info to be populated + * @fhp: pointer to svc_fh to use as source + * + * Many operations in NFSv4 require change_info4 in the reply. This function + * populates that from the info that we (should!) have already collected. In + * the event that we didn't get any pre-attrs, throw a warning and just + * zero out both change_attr4 fields. + */ +static void +set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) +{ + cinfo->atomic = (u32)(fhp->fh_pre_saved && fhp->fh_post_saved && !fhp->fh_no_atomic_attr); + + /* + * In the event that we couldn't fetch attributes from the + * server for some reason, just zero out the before and after + * change values, after throwing a warning. + */ + if (WARN_ON_ONCE(!fhp->fh_pre_saved)) { + cinfo->before_change = 0; + cinfo->after_change = 0; + return; + } + + cinfo->before_change = fhp->fh_pre_change; + cinfo->after_change = fhp->fh_post_change; +} + static __be32 do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_open *open, struct svc_fh **resfh) { diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index b2931fdf53be..9e67f63c5f4d 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -775,17 +775,6 @@ void warn_on_nonidempotent_op(struct nfsd4_op *op); #define NFS4_SVC_XDRSIZE sizeof(struct nfsd4_compoundargs) -static inline void -set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) -{ - BUG_ON(!fhp->fh_pre_saved); - cinfo->atomic = (u32)(fhp->fh_post_saved && !fhp->fh_no_atomic_attr); - - cinfo->before_change = fhp->fh_pre_change; - cinfo->after_change = fhp->fh_post_change; -} - - bool nfsd4_mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp); bool nfs4svc_decode_compoundargs(struct svc_rqst *rqstp, struct xdr_stream *xdr); bool nfs4svc_encode_compoundres(struct svc_rqst *rqstp, struct xdr_stream *xdr); --- base-commit: 070f391ca4d48e1750ee6108eb44f751a9e9448e change-id: 20230720-bz2223560-9c4690a8217b Best regards, -- Jeff Layton <jlayton@xxxxxxxxxx>
