Re: [PATCH 2/3] nfsd: adjust nfsd4_spo_must_allow checking order

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2025-03-21 at 20:13 -0400, Olga Kornievskaia wrote:
> Prior to this patch, some non-4.x NFS operations such as NLM
> calls have to go thru export policy checking would end up
> calling nfsd4_spo_must_allow() function and lead to an
> out-of-bounds error because no compound state structures
> needed by nfsd4_spo_must_allow() are present in the svc_rqst
> request structure.
> 
> Instead, do the nfsd4_spo_must_allow() checking after the
> may_bypass_gss check which is geared towards allowing various
> calls such as NLM while export policy is set with sec=krb5:...
> 
> Fixes: 4cc9b9f2bf4d ("nfsd: refine and rename NFSD_MAY_LOCK")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>  fs/nfsd/export.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 88ae410b4113..02f26cbd59d0 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1143,15 +1143,6 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
>  			return nfs_ok;
>  	}
>  
> -	/* If the compound op contains a spo_must_allowed op,
> -	 * it will be sent with integrity/protection which
> -	 * will have to be expressly allowed on mounts that
> -	 * don't support it
> -	 */
> -
> -	if (nfsd4_spo_must_allow(rqstp))
> -		return nfs_ok;
> -
>  	/* Some calls may be processed without authentication
>  	 * on GSS exports. For example NFS2/3 calls on root
>  	 * directory, see section 2.3.2 of rfc 2623.
> @@ -1168,6 +1159,14 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
>  				return 0;
>  		}
>  	}
> +	/* If the compound op contains a spo_must_allowed op,
> +	 * it will be sent with integrity/protection which
> +	 * will have to be expressly allowed on mounts that
> +	 * don't support it
> +	 */
> +	if (nfsd4_spo_must_allow(rqstp))
> +		return nfs_ok;
> +
>  
>  denied:
>  	return nfserr_wrongsec;

Is this enough to fully fix the OOB problem? It looks like you could
still get past the may_bypass_gss if statement above this with a
carefully crafted RPC.

Maybe the right fix is to make nfsd4_spo_must_allow() check the rq_prog
and rq_vers fields to ensure that this is NFSv4? It can just return
false if it's not.

-- 
Jeff Layton <jlayton@kernel.org>