On 8/5/25 10:32 AM, Scott Mayhew wrote:
> On Fri, 01 Aug 2025, Chuck Lever wrote:
>
>> On 7/31/25 5:14 PM, Scott Mayhew wrote:
>>> +}
>>> +
>>> +/**
>>> + * 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).
>>> + * @may_bypass_gss: reduce strictness of authorization check
>>> + *
>>> + * 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,
>>> + bool may_bypass_gss)
>>> +{
>>> + struct exp_flavor_info *f, *end = exp->ex_flavors + exp->ex_nflavors;
>>> + struct svc_xprt *xprt;
>>> +
>>> + /*
>>> + * If rqstp is NULL, this is a LOCALIO request which will only
>>> + * ever use a filehandle/credential pair for which access has
>>> + * been affirmed (by ACCESS or OPEN NFS requests) over the
>>> + * wire. So there is no need for further checks here.
>>> + */
>>> + if (!rqstp)
>>> + return nfs_ok;
>>
>> Is this true of all of check_nfsd_access's callers, or only of
>> __fh_verify ?
>>
> Looking at the commit where this check was added, and looking at the
> other callers, it looks like this is only true of __fh_verify().
>
> I'm splitting up check_nfsd_access() into two helpers has you suggested,
> having __fh_verify() call the helpers directly while having the other
> callers continue to use check_nfsd_access().
>
> Should I add an argument to the helpers indicate when they have been
> called directly? Something like 'bool maybe_localio', which can
> then be incorporated into the above check, e.g.
>
> if (!rqstp) {
> if (maybe_localio) {
> return nfs_ok;
> } else {
> WARN_ON_ONCE(1);
> return nfserr_wrongsec;
> }
> }
If __fh_verify is the only call site that can invoke these helpers with
rqstp == NULL, then __fh_verify seems like the place to do this check,
not in the helpers. But maybe I've misunderstood something?
>>> +
>>> + xprt = rqstp->rq_xprt;
>>> +
>>> /* legacy gss-only clients are always OK: */
>>> if (exp->ex_client == rqstp->rq_gssclient)
>>> return nfs_ok;
>>> @@ -1167,7 +1202,6 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
>>> }
>>> }
>>>
>>> -denied:
>>> return nfserr_wrongsec;
>>> }
>>>
--
Chuck Lever
