Re: [PATCH] nfsd: Fix NFSD_MAY_BYPASS_GSS and NFSD_MAY_BYPASS_GSS_ON_ROOT

[NeilBrown]

On Thu, 10 Oct 2024, Chuck Lever wrote:
> On Tue, Oct 08, 2024 at 05:47:55PM -0400, NeilBrown wrote:
> > And NFSD_MAY_LOCK should be discarded, and nlm_fopen() should set
> > NFSD_MAY_BYPASS_SEC.
> 
> 366         /*                                                                      
> 367          * pseudoflavor restrictions are not enforced on NLM,                   
> 
> Wrt the mention of "NLM", nfsd4_lock() also sets NFSD_MAY_LOCK.

True, but it shouldn't.  NFSD_MAY_LOCK is only used to bypass the GSS
requirement.  It must have been copied into nfsd4_lock() without a full
understanding of its purpose.

> 
> 368          * which clients virtually always use auth_sys for,                     
> 369          * even while using RPCSEC_GSS for NFS.                                 
> 370          */                                                                     
> 371         if (access & NFSD_MAY_LOCK)                                             
> 372                 goto skip_pseudoflavor_check;                                   
> 373         if (access & NFSD_MAY_BYPASS_GSS)                                       
> 374                 may_bypass_gss = true;
> 375         /*                                                                      
> 376          * Clients may expect to be able to use auth_sys during mount,          
> 377          * even if they use gss for everything else; see section 2.3.2          
> 378          * of rfc 2623.                                                         
> 379          */                                                                     
> 380         if (access & NFSD_MAY_BYPASS_GSS_ON_ROOT                                
> 381                         && exp->ex_path.dentry == dentry)                       
> 382                 may_bypass_gss = true;                                          
> 383                                                                                 
> 384         error = check_nfsd_access(exp, rqstp, may_bypass_gss);                  
> 385         if (error)                                                              
> 386                 goto out;                                                       
> 387                                                                                 
> 388 skip_pseudoflavor_check:                                                        
> 389         /* Finally, check access permissions. */                                
> 390         error = nfsd_permission(cred, exp, dentry, access);     
> 
> MAY_LOCK is checked in nfsd_permission() and __fh_verify().
> 
> But MAY_BYPASS_GSS is set in loads of places that use those two
> functions. How can we be certain that the two flags are equivalent? 

We can be certain by looking at the effect.  Before a recent patch they
both did "goto skip_pseudoflavor_check" and nothing else.

> 
> Though I agree, simplifying this hot path would both help
> performance scalability and reduce reader headaches. It might be a
> little nicer to pass the NFSD_MAY flags directly to
> check_nfsd_access(), for example.

Yes, that might be cleaner.

Thanks,
NeilBrown