Re: [PATCH v6 17/20] nfsd: add the filehandle to returned attributes in CB_NOTIFY

["Chuck Lever" <cel@kernel.org>]

On Thu, Jun 11, 2026, at 1:50 PM, Jeff Layton wrote:
> nfsd's usual fh_compose routine requires a svc_export and fills out a
> svc_fh. In the context of a CB_NOTIFY there is no such export to
> consult.
>
> Add a new routine that composes a filehandle with only a parent
> filehandle and nfs4_file. Use that to fill out the fhandle field in the
> nfsd4_fattr_args.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4xdr.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 7b19248b1503..15ccd54ffdb6 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -4197,6 +4197,39 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, 
> struct xdr_stream *xdr,
>  	goto out;
>  }
> 
> +static bool
> +setup_notify_fhandle(struct dentry *dentry, struct nfs4_file *fi,
> +		     struct nfsd_file *nf, struct nfsd4_fattr_args *args)
> +{
> +	int fileid_type, fsid_len, maxsize, flags = 0;
> +	struct knfsd_fh *fhp = &args->fhandle;
> +	struct inode *inode = d_inode(dentry);
> +	struct inode *parent = NULL;
> +	struct fid *fid;
> +
> +	fsid_len = key_len(fi->fi_fhandle.fh_fsid_type);
> +	fhp->fh_size = 4 + fsid_len;
> +
> +	/* Copy first 4 bytes + fsid */
> +	memcpy(&fhp->fh_raw, &fi->fi_fhandle.fh_raw, fhp->fh_size);
> +
> +	fid = (struct fid *)(fh_fsid(fhp) + fsid_len/4);
> +	maxsize = (NFS4_FHSIZE - fhp->fh_size)/4;
> +
> +	if (fi->fi_connectable && !S_ISDIR(inode->i_mode)) {
> +		parent = d_inode(nf->nf_file->f_path.dentry);
> +		flags = EXPORT_FH_CONNECTABLE;
> +	}
> +
> +	fileid_type = exportfs_encode_inode_fh(inode, fid, &maxsize, parent, 
> flags);
> +	if (fileid_type < 0 || fileid_type == FILEID_INVALID)
> +		return false;
> +
> +	fhp->fh_fileid_type = fileid_type;
> +	fhp->fh_size += maxsize * 4;
> +	return true;
> +}
> +
>  #define CB_NOTIFY_STATX_REQUEST_MASK (STATX_BASIC_STATS   | \
>  				      STATX_BTIME	  | \
>  				      STATX_CHANGE_COOKIE)
> @@ -4206,6 +4239,7 @@ nfsd4_setup_notify_entry4(struct notify_entry4 
> *ne, struct xdr_stream *xdr,
>  			  struct dentry *dentry, struct nfs4_delegation *dp,
>  			  struct nfsd_file *nf, char *name, u32 namelen)
>  {
> +	struct nfs4_file *fi = dp->dl_stid.sc_file;
>  	struct path path =  { .mnt = nf->nf_file->f_path.mnt,
>  			      .dentry = dentry };
>  	struct nfsd4_fattr_args args = { };
> @@ -4244,6 +4278,9 @@ nfsd4_setup_notify_entry4(struct notify_entry4 
> *ne, struct xdr_stream *xdr,
>  		      FATTR4_WORD1_TIME_METADATA | FATTR4_WORD1_TIME_MODIFY;
>  	attrmask[2] = 0;
> 
> +	if (setup_notify_fhandle(dentry, fi, nf, &args))
> +		attrmask[0] |= FATTR4_WORD0_FILEHANDLE;
> +
>  	if (args.stat.result_mask & STATX_BTIME)
>  		attrmask[1] |= FATTR4_WORD1_TIME_CREATE;
> 

Codex flagged setup_notify_fhandle() for constructing a child FILEHANDLE
attribute without calling fh_append_mac(): for exports with sign_fh,
fh_compose() appends a MAC, and nfsd_set_fh_dentry() rejects every
non-root signed-export handle whose MAC is absent or mismatched, so a
client using the CB_NOTIFY filehandle gets a stale/bad handle. It
recommends signing the constructed handle or suppressing the attribute
when the export requires signed filehandles.

A client that does not receive the FH falls back to a LOOKUP, so
suppression degrades gracefully.

-- 
Chuck Lever