On Mon, Apr 7, 2025 at 11:36 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> > ---
> > 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.
A crafted RPC can and thus Neil's patch that checks the version in
nfsd4_spo_must_allow is needed.
I still feel changing the order would be beneficial as it would take
care of realistic requests.
> 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@xxxxxxxxxx>
>
