On Wed, 28 Aug 2024, cel@xxxxxxxxxx wrote:
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> Currently, fh_verify() makes some daring assumptions about which
> version of file handle the caller wants, based on the things it can
> find in the passed-in rqstp. The about-to-be-introduced LOCALIO use
> case sometimes has no svc_rqst context, so this logic won't work in
> that case.
>
> Instead, examine the passed-in file handle. It's .max_size field
> should carry information to allow nfsd_set_fh_dentry() to initialize
> the file handle appropriately.
>
> lockd appears to be the only kernel consumer that does not set the
> file handle .max_size when during initialization.
>
> write_filehandle() is the other question mark, as it looks possible
> to specify a maxsize between NFS_FHSIZE and NFS3_FHSIZE here.
The file handle used by lockd and the one created by write_filehandle
never need any of the version-specific fields.
Those fields affect things like write requests and getattr requests and
pre/post attributes.
I wonder if the filehandle is really the best place of these flag.
Having them in the file handle works really well for fh_fill_pre_attrs()
and reasonably well in other places, But it makes nfsd_set_fh_dentry a
little clumsy. Maybe it would be better to moved them to
rqstp->rq_flags. Or possibly in the "Catering to nfsd" section of
'struct svc_rqst'.
NeilBrown
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> fs/nfsd/lockd.c | 6 ++++--
> fs/nfsd/nfsfh.c | 11 +++++++----
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> index 46a7f9b813e5..e636d2a1e664 100644
> --- a/fs/nfsd/lockd.c
> +++ b/fs/nfsd/lockd.c
> @@ -32,8 +32,10 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> int access;
> struct svc_fh fh;
>
> - /* must initialize before using! but maxsize doesn't matter */
> - fh_init(&fh,0);
> + if (rqstp->rq_vers == 4)
> + fh_init(&fh, NFS3_FHSIZE);
> + else
> + fh_init(&fh, NFS_FHSIZE);
> fh.fh_handle.fh_size = f->size;
> memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> fh.fh_export = NULL;
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 4b964a71a504..77acc26e8b02 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -267,25 +267,28 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
> fhp->fh_dentry = dentry;
> fhp->fh_export = exp;
>
> - switch (rqstp->rq_vers) {
> - case 4:
> + switch (fhp->fh_maxsize) {
> + case NFS4_FHSIZE:
> if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOATOMIC_ATTR)
> fhp->fh_no_atomic_attr = true;
> fhp->fh_64bit_cookies = true;
> break;
> - case 3:
> + case NFS3_FHSIZE:
> if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC)
> fhp->fh_no_wcc = true;
> fhp->fh_64bit_cookies = true;
> if (exp->ex_flags & NFSEXP_V4ROOT)
> goto out;
> break;
> - case 2:
> + case NFS_FHSIZE:
> fhp->fh_no_wcc = true;
> if (EX_WGATHER(exp))
> fhp->fh_use_wgather = true;
> if (exp->ex_flags & NFSEXP_V4ROOT)
> goto out;
> + break;
> + case 0:
> + WARN_ONCE(1, "Uninitialized file handle");
> }
>
> return 0;
> --
> 2.45.2
>
>
