On Wed, Aug 28, 2024 at 11:12:00AM +1000, NeilBrown wrote:
> On Wed, 28 Aug 2024, cel@xxxxxxxxxx wrote:
> > From: NeilBrown <neilb@xxxxxxx>
> >
> > 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@xxxxxxx>
> > Co-developed-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> > ---
> > 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
