Re: [PATCH v14 05/25] NFSD: Refactor nfsd_setuser_and_check_port()

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2024-08-28 at 21:04 -0400, Mike Snitzer wrote:
> From: NeilBrown <neilb@suse.de>
> 
> There are several places where __fh_verify unconditionally dereferences
> rqstp to check that the connection is suitably secure.  They look at
> rqstp->rq_xprt which is not meaningful in the target use case of
> "localio" NFS in which the client talks directly to the local server.
> 
> Prepare these to always succeed when rqstp is NULL.
> 
> 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/nfsfh.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 50d23d56f403..4b964a71a504 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -87,23 +87,24 @@ nfsd_mode_check(struct dentry *dentry, umode_t requested)
>  	return nfserr_wrong_type;
>  }
>  
> -static bool nfsd_originating_port_ok(struct svc_rqst *rqstp, int flags)
> +static bool nfsd_originating_port_ok(struct svc_rqst *rqstp,
> +				     struct svc_cred *cred,
> +				     struct svc_export *exp)
>  {
> -	if (flags & NFSEXP_INSECURE_PORT)
> +	if (nfsexp_flags(cred, exp) & NFSEXP_INSECURE_PORT)
>  		return true;
>  	/* We don't require gss requests to use low ports: */
> -	if (rqstp->rq_cred.cr_flavor >= RPC_AUTH_GSS)
> +	if (cred->cr_flavor >= RPC_AUTH_GSS)
>  		return true;
>  	return test_bit(RQ_SECURE, &rqstp->rq_flags);
>  }
>  
>  static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
> +					  struct svc_cred *cred,
>  					  struct svc_export *exp)
>  {
> -	int flags = nfsexp_flags(&rqstp->rq_cred, exp);
> -
>  	/* Check if the request originated from a secure port. */
> -	if (!nfsd_originating_port_ok(rqstp, flags)) {
> +	if (rqstp && !nfsd_originating_port_ok(rqstp, cred, exp)) {
>  		RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
>  		dprintk("nfsd: request from insecure port %s!\n",
>  		        svc_print_addr(rqstp, buf, sizeof(buf)));
> @@ -111,7 +112,7 @@ static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
>  	}
>  
>  	/* Set user creds for this exportpoint */
> -	return nfserrno(nfsd_setuser(&rqstp->rq_cred, exp));
> +	return nfserrno(nfsd_setuser(cred, exp));
>  }
>  
>  static inline __be32 check_pseudo_root(struct dentry *dentry,
> @@ -219,7 +220,7 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
>  		put_cred(override_creds(new));
>  		put_cred(new);
>  	} else {
> -		error = nfsd_setuser_and_check_port(rqstp, exp);
> +		error = nfsd_setuser_and_check_port(rqstp, &rqstp->rq_cred, exp);
>  		if (error)
>  			goto out;
>  	}
> @@ -358,7 +359,7 @@ fh_verify(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type, int access)
>  	if (error)
>  		goto out;
>  
> -	error = nfsd_setuser_and_check_port(rqstp, exp);
> +	error = nfsd_setuser_and_check_port(rqstp, &rqstp->rq_cred, exp);
>  	if (error)
>  		goto out;
>  

Reviewed-by: Jeff Layton <jlayton@kernel.org>