Re: [RFC PATCH 1/6] NFSD: Handle @rqstp == NULL in check_nfsd_access()

[Mike Snitzer <snitzer@kernel.org>]

On Wed, Aug 28, 2024 at 11:12:00AM +1000, NeilBrown wrote:
> On Wed, 28 Aug 2024, cel@kernel.org wrote:
> > From: NeilBrown <neilb@suse.de>
> > 
> > LOCALIO-initiated open operations are not running in an nfsd thread
> > and thus do not have an associated svc_rqst context.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > Co-developed-by: Mike Snitzer <snitzer@kernel.org>
> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  fs/nfsd/export.c | 29 ++++++++++++++++++++++++-----
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > index 7bb4f2075ac5..46a4d989c850 100644
> > --- a/fs/nfsd/export.c
> > +++ b/fs/nfsd/export.c
> > @@ -1074,10 +1074,29 @@ static struct svc_export *exp_find(struct cache_detail *cd,
> >  	return exp;
> >  }
> >  
> > +/**
> > + * check_nfsd_access - check if access to export is allowed.
> > + * @exp: svc_export that is being accessed.
> > + * @rqstp: svc_rqst attempting to access @exp (will be NULL for LOCALIO).
> > + *
> > + * Return values:
> > + *   %nfs_ok if access is granted, or
> > + *   %nfserr_wrongsec if access is denied
> > + */
> >  __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp)
> >  {
> >  	struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
> > -	struct svc_xprt *xprt = rqstp->rq_xprt;
> > +	struct svc_xprt *xprt;
> > +
> > +	/*
> > +	 * The target use case for rqstp being NULL is LOCALIO, which
> > +	 * currently only supports AUTH_UNIX. The behavior for LOCALIO
> > +	 * is therefore the same as the AUTH_UNIX check below.
> 
> The "AUTH_UNIX check below" only applies if exp->ex_flavours == 0.
> To make "rqstp == NULL" mean "treat like AUTH_UNIX" I think we need
> to confirm that 
>   exp->ex_xprtsec_mods & NFSEXP_XPRTSEC_NONE
> and either
>   exp->ex_nflavours == 0
> or
>   one for the exp->ex_flavors->pseudoflavor values is RPC_AUTH_UNIX
> 
> I'm not sure that is all really necessary, but if not then we probably
> need a better comment...

Think extra checks aren't needed (unless you think a NULL rqstp
_without_ the use of LOCALIO possible?  which could trigger a false
positive granting of access? seems unlikely but...)

Mike