Commit 97662607 authored by Jeff Layton's avatar Jeff Layton Committed by Chuck Lever
Browse files

nfsd: remove unsafe BUG_ON from set_change_info 



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 just have it zero out both
change_attr4s when this occurs.

Reported-by: default avatarBoyang Xue <bxue@redhat.com>
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2223560


Signed-off-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent a332018a

fs/nfsd/nfs4proc.c

+32 −0
Original line number Diff line number Diff line
@@ -382,6 +382,38 @@ 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, just zero out both.

 */

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);

	cinfo->before_change = fhp->fh_pre_change;

	cinfo->after_change = fhp->fh_post_change;



	/*

	 * If fetching the pre-change attributes failed, then we should

	 * have already failed the whole operation. We could have still

	 * failed to fetch post-change attributes however.

	 *

	 * If we didn't get post-op attrs, just zero-out the after

	 * field since we don't know what it should be. If the pre_saved

	 * field isn't set for some reason, throw warning and just copy

	 * whatever is in the after field.

	 */

	if (WARN_ON_ONCE(!fhp->fh_pre_saved))

		cinfo->before_change = 0;

	if (!fhp->fh_post_saved)

		cinfo->after_change = 0;

}



static __be32

do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_open *open, struct svc_fh **resfh)

{

fs/nfsd/xdr4.h

+0 −11
Original line number Diff line number Diff line
@@ -774,17 +774,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);