Re: [PATCH v6 15/20] nfsd: allow encoding a filehandle into fattr4 without a svc_fh

[Chuck Lever]

On Thu, Jun 11, 2026, at 1:50 PM, Jeff Layton wrote:
> The current fattr4 encoder requires a svc_fh in order to encode the
> filehandle. This is not available in a CB_NOTIFY callback. Add a a new
> "fhandle" field to struct nfsd4_fattr_args and copy the filehandle into
> there from the svc_fh. CB_NOTIFY will populate it via other means.
>
> A filehandle composed this way may still need a MAC appended on signed
> exports, so generalize fh_append_mac() to operate on a bare knfsd_fh
> (plus its maximum size and net) rather than a svc_fh.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  fs/nfsd/nfs4xdr.c | 36 +++++++++++++++++++++---------------
>  fs/nfsd/nfsfh.c   | 10 +++++-----
>  fs/nfsd/nfsfh.h   |  1 +
>  3 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 4fb61d05a4a7..7b19248b1503 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c

> @@ -4015,19 +4016,24 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, 
> struct xdr_stream *xdr,
>  		if (err)
>  			goto out_nfserr;
>  	}
> -	if ((attrmask[0] & (FATTR4_WORD0_FILEHANDLE | FATTR4_WORD0_FSID)) &&
> -	    !fhp) {
> -		tempfh = kmalloc_obj(struct svc_fh);
> -		status = nfserr_jukebox;
> -		if (!tempfh)
> -			goto out;
> -		fh_init(tempfh, NFS4_FHSIZE);
> -		status = fh_compose(tempfh, exp, dentry, NULL);
> -		if (status)
> -			goto out;
> -		args.fhp = tempfh;
> -	} else
> -		args.fhp = fhp;
> +
> +	args.fhp = fhp;
> +	if ((attrmask[0] & (FATTR4_WORD0_FILEHANDLE | FATTR4_WORD0_FSID))) {
> +		if (!args.fhp) {
> +			tempfh = kmalloc_obj(struct svc_fh);
> +			status = nfserr_jukebox;
> +			if (!tempfh)
> +				goto out;
> +			fh_init(tempfh, NFS4_FHSIZE);
> +			status = fh_compose(tempfh, exp, dentry, NULL);
> +			if (status)
> +				goto out;
> +			args.fhp = tempfh;
> +		}
> +		if (args.fhp)

Nit: here, "args.fhp" is never false.

Note that nfsd4_encode_fattr4_fsid() calls fsid_source(args->fhp)
without a NULL check. After this patch is applied, filehandle
encoding is svc_fh-free but FSID encoding is not, and the two
share the same attrmask gate:

   (FATTR4_WORD0_FILEHANDLE | FATTR4_WORD0_FSID).

No current caller trips it: the CB_NOTIFY path does not request
FSID. But a future CB_NOTIFY attrset that adds FSID would
dereference a NULL fhp.


> +			fh_copy_shallow(&args.fhandle, &args.fhp->fh_handle);
> +	}
> +
>  	if (attrmask[0] & (FATTR4_WORD0_CASE_INSENSITIVE |
>  			   FATTR4_WORD0_CASE_PRESERVING)) {
>  		/*


-- 
Chuck Lever