* [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
@ 2024-10-17 21:42 NeilBrown
2024-10-18 13:39 ` cel
2025-03-11 15:28 ` Olga Kornievskaia
0 siblings, 2 replies; 15+ messages in thread
From: NeilBrown @ 2024-10-17 21:42 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs
NFSD_MAY_LOCK means a few different things.
- it means that GSS is not required.
- it means that with NFSEXP_NOAUTHNLM, authentication is not required
- it means that OWNER_OVERRIDE is allowed.
None of these are specific to locking, they are specific to the NLM
protocol.
So:
- rename to NFSD_MAY_NLM
- set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
so that NFSD_MAY_NLM doesn't need to imply these.
- move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
into fh_verify where other special-case tests on the MAY flags
happen. nfsd_permission() can be called from other places than
fh_verify(), but none of these will have NFSD_MAY_NLM.
Signed-off-by: NeilBrown <neilb@suse.de>
---
No change from previous patch - the corruption in the email has been
avoided (I hope).
fs/nfsd/lockd.c | 13 +++++++++++--
fs/nfsd/nfsfh.c | 12 ++++--------
fs/nfsd/trace.h | 2 +-
fs/nfsd/vfs.c | 12 +-----------
fs/nfsd/vfs.h | 2 +-
5 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
index 46a7f9b813e5..edc9f75dc75c 100644
--- a/fs/nfsd/lockd.c
+++ b/fs/nfsd/lockd.c
@@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
fh.fh_export = NULL;
+ /*
+ * Allow BYPASS_GSS as some client implementations use AUTH_SYS
+ * for NLM even when GSS is used for NFS.
+ * Allow OWNER_OVERRIDE as permission might have been changed
+ * after the file was opened.
+ * Pass MAY_NLM so that authentication can be completely bypassed
+ * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
+ * for NLM requests.
+ */
access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
- access |= NFSD_MAY_LOCK;
+ access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
fh_put(&fh);
- /* We return nlm error codes as nlm doesn't know
+ /* We return nlm error codes as nlm doesn't know
* about nfsd, but nfsd does know about nlm..
*/
switch (nfserr) {
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 40533f7c7297..6a831cb242df 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
if (error)
goto out;
- /*
- * pseudoflavor restrictions are not enforced on NLM,
- * which clients virtually always use auth_sys for,
- * even while using RPCSEC_GSS for NFS.
- */
- if (access & NFSD_MAY_LOCK)
- goto skip_pseudoflavor_check;
+ if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
+ /* NLM is allowed to fully bypass authentication */
+ goto out;
+
if (access & NFSD_MAY_BYPASS_GSS)
may_bypass_gss = true;
/*
@@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
if (error)
goto out;
-skip_pseudoflavor_check:
/* Finally, check access permissions. */
error = nfsd_permission(cred, exp, dentry, access);
out:
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index b8470d4cbe99..3448e444d410 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
{ NFSD_MAY_READ, "READ" }, \
{ NFSD_MAY_SATTR, "SATTR" }, \
{ NFSD_MAY_TRUNC, "TRUNC" }, \
- { NFSD_MAY_LOCK, "LOCK" }, \
+ { NFSD_MAY_NLM, "NLM" }, \
{ NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
{ NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
{ NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 51f5a0b181f9..2610638f4301 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
(acc & NFSD_MAY_EXEC)? " exec" : "",
(acc & NFSD_MAY_SATTR)? " sattr" : "",
(acc & NFSD_MAY_TRUNC)? " trunc" : "",
- (acc & NFSD_MAY_LOCK)? " lock" : "",
+ (acc & NFSD_MAY_NLM)? " nlm" : "",
(acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
inode->i_mode,
IS_IMMUTABLE(inode)? " immut" : "",
@@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
return nfserr_perm;
- if (acc & NFSD_MAY_LOCK) {
- /* If we cannot rely on authentication in NLM requests,
- * just allow locks, otherwise require read permission, or
- * ownership
- */
- if (exp->ex_flags & NFSEXP_NOAUTHNLM)
- return 0;
- else
- acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
- }
/*
* The file owner always gets access permission for accesses that
* would normally be checked at open time. This is to make
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 854fb95dfdca..f9b09b842856 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -20,7 +20,7 @@
#define NFSD_MAY_READ 0x004 /* == MAY_READ */
#define NFSD_MAY_SATTR 0x008
#define NFSD_MAY_TRUNC 0x010
-#define NFSD_MAY_LOCK 0x020
+#define NFSD_MAY_NLM 0x020 /* request is from lockd */
#define NFSD_MAY_MASK 0x03f
/* extra hints to permission and open routines: */
base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
--
2.46.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2024-10-17 21:42 [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK NeilBrown
@ 2024-10-18 13:39 ` cel
2025-03-11 15:28 ` Olga Kornievskaia
1 sibling, 0 replies; 15+ messages in thread
From: cel @ 2024-10-18 13:39 UTC (permalink / raw)
To: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey, NeilBrown
Cc: Chuck Lever, linux-nfs
From: Chuck Lever <chuck.lever@oracle.com>
On Fri, 18 Oct 2024 08:42:31 +1100, NeilBrown wrote:
> NFSD_MAY_LOCK means a few different things.
> - it means that GSS is not required.
> - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> - it means that OWNER_OVERRIDE is allowed.
>
> None of these are specific to locking, they are specific to the NLM
> protocol.
> So:
> - rename to NFSD_MAY_NLM
> - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> so that NFSD_MAY_NLM doesn't need to imply these.
> - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> into fh_verify where other special-case tests on the MAY flags
> happen. nfsd_permission() can be called from other places than
> fh_verify(), but none of these will have NFSD_MAY_NLM.
>
> [...]
Applied to nfsd-next for v6.13, thanks!
[1/1] nfsd: refine and rename NFSD_MAY_LOCK
commit: f94d4ca4e9862261a77dcfc8b567a563452add41
--
Chuck Lever
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2024-10-17 21:42 [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK NeilBrown
2024-10-18 13:39 ` cel
@ 2025-03-11 15:28 ` Olga Kornievskaia
2025-03-11 16:35 ` Olga Kornievskaia
2025-03-11 18:25 ` Olga Kornievskaia
1 sibling, 2 replies; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-11 15:28 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
>
>
> NFSD_MAY_LOCK means a few different things.
> - it means that GSS is not required.
> - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> - it means that OWNER_OVERRIDE is allowed.
>
> None of these are specific to locking, they are specific to the NLM
> protocol.
> So:
> - rename to NFSD_MAY_NLM
> - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> so that NFSD_MAY_NLM doesn't need to imply these.
> - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> into fh_verify where other special-case tests on the MAY flags
> happen. nfsd_permission() can be called from other places than
> fh_verify(), but none of these will have NFSD_MAY_NLM.
This patch breaks NLM when used in combination with TLS. If exports
have xprtsec=tls:mtls and mount is done with tls/mtls, the server
won't give any locks and client will get "no locks available" error.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>
> No change from previous patch - the corruption in the email has been
> avoided (I hope).
>
>
> fs/nfsd/lockd.c | 13 +++++++++++--
> fs/nfsd/nfsfh.c | 12 ++++--------
> fs/nfsd/trace.h | 2 +-
> fs/nfsd/vfs.c | 12 +-----------
> fs/nfsd/vfs.h | 2 +-
> 5 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> index 46a7f9b813e5..edc9f75dc75c 100644
> --- a/fs/nfsd/lockd.c
> +++ b/fs/nfsd/lockd.c
> @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> fh.fh_export = NULL;
>
> + /*
> + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> + * for NLM even when GSS is used for NFS.
> + * Allow OWNER_OVERRIDE as permission might have been changed
> + * after the file was opened.
> + * Pass MAY_NLM so that authentication can be completely bypassed
> + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> + * for NLM requests.
> + */
> access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> - access |= NFSD_MAY_LOCK;
> + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> fh_put(&fh);
> - /* We return nlm error codes as nlm doesn't know
> + /* We return nlm error codes as nlm doesn't know
> * about nfsd, but nfsd does know about nlm..
> */
> switch (nfserr) {
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 40533f7c7297..6a831cb242df 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> if (error)
> goto out;
>
> - /*
> - * pseudoflavor restrictions are not enforced on NLM,
> - * which clients virtually always use auth_sys for,
> - * even while using RPCSEC_GSS for NFS.
> - */
> - if (access & NFSD_MAY_LOCK)
> - goto skip_pseudoflavor_check;
> + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> + /* NLM is allowed to fully bypass authentication */
> + goto out;
> +
> if (access & NFSD_MAY_BYPASS_GSS)
> may_bypass_gss = true;
> /*
> @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> if (error)
> goto out;
>
> -skip_pseudoflavor_check:
> /* Finally, check access permissions. */
> error = nfsd_permission(cred, exp, dentry, access);
> out:
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index b8470d4cbe99..3448e444d410 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> { NFSD_MAY_READ, "READ" }, \
> { NFSD_MAY_SATTR, "SATTR" }, \
> { NFSD_MAY_TRUNC, "TRUNC" }, \
> - { NFSD_MAY_LOCK, "LOCK" }, \
> + { NFSD_MAY_NLM, "NLM" }, \
> { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 51f5a0b181f9..2610638f4301 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> (acc & NFSD_MAY_EXEC)? " exec" : "",
> (acc & NFSD_MAY_SATTR)? " sattr" : "",
> (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> - (acc & NFSD_MAY_LOCK)? " lock" : "",
> + (acc & NFSD_MAY_NLM)? " nlm" : "",
> (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> inode->i_mode,
> IS_IMMUTABLE(inode)? " immut" : "",
> @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> return nfserr_perm;
>
> - if (acc & NFSD_MAY_LOCK) {
> - /* If we cannot rely on authentication in NLM requests,
> - * just allow locks, otherwise require read permission, or
> - * ownership
> - */
> - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> - return 0;
> - else
> - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> - }
> /*
> * The file owner always gets access permission for accesses that
> * would normally be checked at open time. This is to make
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 854fb95dfdca..f9b09b842856 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -20,7 +20,7 @@
> #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> #define NFSD_MAY_SATTR 0x008
> #define NFSD_MAY_TRUNC 0x010
> -#define NFSD_MAY_LOCK 0x020
> +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> #define NFSD_MAY_MASK 0x03f
>
> /* extra hints to permission and open routines: */
>
> base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> --
> 2.46.0
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-11 15:28 ` Olga Kornievskaia
@ 2025-03-11 16:35 ` Olga Kornievskaia
2025-03-11 18:25 ` Olga Kornievskaia
1 sibling, 0 replies; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-11 16:35 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> >
> >
> > NFSD_MAY_LOCK means a few different things.
> > - it means that GSS is not required.
> > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > - it means that OWNER_OVERRIDE is allowed.
> >
> > None of these are specific to locking, they are specific to the NLM
> > protocol.
> > So:
> > - rename to NFSD_MAY_NLM
> > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > so that NFSD_MAY_NLM doesn't need to imply these.
> > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > into fh_verify where other special-case tests on the MAY flags
> > happen. nfsd_permission() can be called from other places than
> > fh_verify(), but none of these will have NFSD_MAY_NLM.
>
> This patch breaks NLM when used in combination with TLS. If exports
> have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> won't give any locks and client will get "no locks available" error.
>
> >
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >
> > No change from previous patch - the corruption in the email has been
> > avoided (I hope).
> >
> >
> > fs/nfsd/lockd.c | 13 +++++++++++--
> > fs/nfsd/nfsfh.c | 12 ++++--------
> > fs/nfsd/trace.h | 2 +-
> > fs/nfsd/vfs.c | 12 +-----------
> > fs/nfsd/vfs.h | 2 +-
> > 5 files changed, 18 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > index 46a7f9b813e5..edc9f75dc75c 100644
> > --- a/fs/nfsd/lockd.c
> > +++ b/fs/nfsd/lockd.c
> > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > fh.fh_export = NULL;
> >
> > + /*
> > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > + * for NLM even when GSS is used for NFS.
> > + * Allow OWNER_OVERRIDE as permission might have been changed
> > + * after the file was opened.
> > + * Pass MAY_NLM so that authentication can be completely bypassed
> > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > + * for NLM requests.
> > + */
> > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > - access |= NFSD_MAY_LOCK;
> > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > fh_put(&fh);
> > - /* We return nlm error codes as nlm doesn't know
> > + /* We return nlm error codes as nlm doesn't know
> > * about nfsd, but nfsd does know about nlm..
> > */
> > switch (nfserr) {
> > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > index 40533f7c7297..6a831cb242df 100644
> > --- a/fs/nfsd/nfsfh.c
> > +++ b/fs/nfsd/nfsfh.c
> > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > if (error)
> > goto out;
> >
> > - /*
> > - * pseudoflavor restrictions are not enforced on NLM,
> > - * which clients virtually always use auth_sys for,
> > - * even while using RPCSEC_GSS for NFS.
> > - */
> > - if (access & NFSD_MAY_LOCK)
> > - goto skip_pseudoflavor_check;
> > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > + /* NLM is allowed to fully bypass authentication */
> > + goto out;
> > +
I think it's not appropriate to add (exp->ex_flags & NFSEXP_NOAUTHNLM) here.
> > if (access & NFSD_MAY_BYPASS_GSS)
> > may_bypass_gss = true;
> > /*
> > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > if (error)
> > goto out;
> >
> > -skip_pseudoflavor_check:
> > /* Finally, check access permissions. */
> > error = nfsd_permission(cred, exp, dentry, access);
> > out:
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index b8470d4cbe99..3448e444d410 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > { NFSD_MAY_READ, "READ" }, \
> > { NFSD_MAY_SATTR, "SATTR" }, \
> > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > - { NFSD_MAY_LOCK, "LOCK" }, \
> > + { NFSD_MAY_NLM, "NLM" }, \
> > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 51f5a0b181f9..2610638f4301 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > inode->i_mode,
> > IS_IMMUTABLE(inode)? " immut" : "",
> > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > return nfserr_perm;
> >
> > - if (acc & NFSD_MAY_LOCK) {
> > - /* If we cannot rely on authentication in NLM requests,
> > - * just allow locks, otherwise require read permission, or
> > - * ownership
> > - */
> > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > - return 0;
> > - else
> > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > - }
> > /*
> > * The file owner always gets access permission for accesses that
> > * would normally be checked at open time. This is to make
> > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > index 854fb95dfdca..f9b09b842856 100644
> > --- a/fs/nfsd/vfs.h
> > +++ b/fs/nfsd/vfs.h
> > @@ -20,7 +20,7 @@
> > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > #define NFSD_MAY_SATTR 0x008
> > #define NFSD_MAY_TRUNC 0x010
> > -#define NFSD_MAY_LOCK 0x020
> > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > #define NFSD_MAY_MASK 0x03f
> >
> > /* extra hints to permission and open routines: */
> >
> > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > --
> > 2.46.0
> >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-11 15:28 ` Olga Kornievskaia
2025-03-11 16:35 ` Olga Kornievskaia
@ 2025-03-11 18:25 ` Olga Kornievskaia
2025-03-11 21:42 ` NeilBrown
1 sibling, 1 reply; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-11 18:25 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> >
> >
> > NFSD_MAY_LOCK means a few different things.
> > - it means that GSS is not required.
> > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > - it means that OWNER_OVERRIDE is allowed.
> >
> > None of these are specific to locking, they are specific to the NLM
> > protocol.
> > So:
> > - rename to NFSD_MAY_NLM
> > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > so that NFSD_MAY_NLM doesn't need to imply these.
> > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > into fh_verify where other special-case tests on the MAY flags
> > happen. nfsd_permission() can be called from other places than
> > fh_verify(), but none of these will have NFSD_MAY_NLM.
>
> This patch breaks NLM when used in combination with TLS.
I was too quick to link this to TLS. It's presence of security policy
so sec=krb* causes the same problems.
> If exports
> have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> won't give any locks and client will get "no locks available" error.
>
> >
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >
> > No change from previous patch - the corruption in the email has been
> > avoided (I hope).
> >
> >
> > fs/nfsd/lockd.c | 13 +++++++++++--
> > fs/nfsd/nfsfh.c | 12 ++++--------
> > fs/nfsd/trace.h | 2 +-
> > fs/nfsd/vfs.c | 12 +-----------
> > fs/nfsd/vfs.h | 2 +-
> > 5 files changed, 18 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > index 46a7f9b813e5..edc9f75dc75c 100644
> > --- a/fs/nfsd/lockd.c
> > +++ b/fs/nfsd/lockd.c
> > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > fh.fh_export = NULL;
> >
> > + /*
> > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > + * for NLM even when GSS is used for NFS.
> > + * Allow OWNER_OVERRIDE as permission might have been changed
> > + * after the file was opened.
> > + * Pass MAY_NLM so that authentication can be completely bypassed
> > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > + * for NLM requests.
> > + */
> > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > - access |= NFSD_MAY_LOCK;
> > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > fh_put(&fh);
> > - /* We return nlm error codes as nlm doesn't know
> > + /* We return nlm error codes as nlm doesn't know
> > * about nfsd, but nfsd does know about nlm..
> > */
> > switch (nfserr) {
> > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > index 40533f7c7297..6a831cb242df 100644
> > --- a/fs/nfsd/nfsfh.c
> > +++ b/fs/nfsd/nfsfh.c
> > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > if (error)
> > goto out;
> >
> > - /*
> > - * pseudoflavor restrictions are not enforced on NLM,
> > - * which clients virtually always use auth_sys for,
> > - * even while using RPCSEC_GSS for NFS.
> > - */
> > - if (access & NFSD_MAY_LOCK)
> > - goto skip_pseudoflavor_check;
> > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
I think this should either be an OR or the fact that "nlm but only
with insecurelock export option and not other" is the only way to
bypass checking is wrong. I think it's just a check for NLM that
stays.
> > + /* NLM is allowed to fully bypass authentication */
> > + goto out;
> > +
> > if (access & NFSD_MAY_BYPASS_GSS)
> > may_bypass_gss = true;
> > /*
> > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > if (error)
> > goto out;
> >
> > -skip_pseudoflavor_check:
> > /* Finally, check access permissions. */
> > error = nfsd_permission(cred, exp, dentry, access);
> > out:
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index b8470d4cbe99..3448e444d410 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > { NFSD_MAY_READ, "READ" }, \
> > { NFSD_MAY_SATTR, "SATTR" }, \
> > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > - { NFSD_MAY_LOCK, "LOCK" }, \
> > + { NFSD_MAY_NLM, "NLM" }, \
> > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 51f5a0b181f9..2610638f4301 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > inode->i_mode,
> > IS_IMMUTABLE(inode)? " immut" : "",
> > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > return nfserr_perm;
> >
> > - if (acc & NFSD_MAY_LOCK) {
> > - /* If we cannot rely on authentication in NLM requests,
> > - * just allow locks, otherwise require read permission, or
> > - * ownership
> > - */
> > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > - return 0;
> > - else
> > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > - }
> > /*
> > * The file owner always gets access permission for accesses that
> > * would normally be checked at open time. This is to make
> > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > index 854fb95dfdca..f9b09b842856 100644
> > --- a/fs/nfsd/vfs.h
> > +++ b/fs/nfsd/vfs.h
> > @@ -20,7 +20,7 @@
> > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > #define NFSD_MAY_SATTR 0x008
> > #define NFSD_MAY_TRUNC 0x010
> > -#define NFSD_MAY_LOCK 0x020
> > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > #define NFSD_MAY_MASK 0x03f
> >
> > /* extra hints to permission and open routines: */
> >
> > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > --
> > 2.46.0
> >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-11 18:25 ` Olga Kornievskaia
@ 2025-03-11 21:42 ` NeilBrown
2025-03-12 13:22 ` Olga Kornievskaia
0 siblings, 1 reply; 15+ messages in thread
From: NeilBrown @ 2025-03-11 21:42 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> >
> > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > >
> > >
> > > NFSD_MAY_LOCK means a few different things.
> > > - it means that GSS is not required.
> > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > - it means that OWNER_OVERRIDE is allowed.
> > >
> > > None of these are specific to locking, they are specific to the NLM
> > > protocol.
> > > So:
> > > - rename to NFSD_MAY_NLM
> > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > into fh_verify where other special-case tests on the MAY flags
> > > happen. nfsd_permission() can be called from other places than
> > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> >
> > This patch breaks NLM when used in combination with TLS.
>
> I was too quick to link this to TLS. It's presence of security policy
> so sec=krb* causes the same problems.
>
> > If exports
> > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > won't give any locks and client will get "no locks available" error.
> >
> > >
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > ---
> > >
> > > No change from previous patch - the corruption in the email has been
> > > avoided (I hope).
> > >
> > >
> > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > fs/nfsd/trace.h | 2 +-
> > > fs/nfsd/vfs.c | 12 +-----------
> > > fs/nfsd/vfs.h | 2 +-
> > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > --- a/fs/nfsd/lockd.c
> > > +++ b/fs/nfsd/lockd.c
> > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > fh.fh_export = NULL;
> > >
> > > + /*
> > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > + * for NLM even when GSS is used for NFS.
> > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > + * after the file was opened.
> > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > + * for NLM requests.
> > > + */
> > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > - access |= NFSD_MAY_LOCK;
> > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > fh_put(&fh);
> > > - /* We return nlm error codes as nlm doesn't know
> > > + /* We return nlm error codes as nlm doesn't know
> > > * about nfsd, but nfsd does know about nlm..
> > > */
> > > switch (nfserr) {
> > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > index 40533f7c7297..6a831cb242df 100644
> > > --- a/fs/nfsd/nfsfh.c
> > > +++ b/fs/nfsd/nfsfh.c
> > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > if (error)
> > > goto out;
> > >
> > > - /*
> > > - * pseudoflavor restrictions are not enforced on NLM,
> > > - * which clients virtually always use auth_sys for,
> > > - * even while using RPCSEC_GSS for NFS.
> > > - */
> > > - if (access & NFSD_MAY_LOCK)
> > > - goto skip_pseudoflavor_check;
> > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
>
> I think this should either be an OR or the fact that "nlm but only
> with insecurelock export option and not other" is the only way to
> bypass checking is wrong. I think it's just a check for NLM that
> stays.
I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
to make it work.
I assume the NLM request is arriving with AUTH_SYS authentication?
So check_nfsd_access() is being called with may_bypass_gss and this:
if (may_bypass_gss && (
rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
for (f = exp->ex_flavors; f < end; f++) {
if (f->pseudoflavor >= RPC_AUTH_DES)
return 0;
}
}
in check_nfsd_access() should succeed.
Can you add some tracing and see what is happening in here?
Maybe the "goto denied" earlier in the function is being reached. I
don't fully understand the TLS code yet - maybe it needs some test on
may_bypass_gss.
Thanks,
NeilBrown
>
> > > + /* NLM is allowed to fully bypass authentication */
> > > + goto out;
> > > +
> > > if (access & NFSD_MAY_BYPASS_GSS)
> > > may_bypass_gss = true;
> > > /*
> > > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > > if (error)
> > > goto out;
> > >
> > > -skip_pseudoflavor_check:
> > > /* Finally, check access permissions. */
> > > error = nfsd_permission(cred, exp, dentry, access);
> > > out:
> > > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > > index b8470d4cbe99..3448e444d410 100644
> > > --- a/fs/nfsd/trace.h
> > > +++ b/fs/nfsd/trace.h
> > > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > > { NFSD_MAY_READ, "READ" }, \
> > > { NFSD_MAY_SATTR, "SATTR" }, \
> > > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > > - { NFSD_MAY_LOCK, "LOCK" }, \
> > > + { NFSD_MAY_NLM, "NLM" }, \
> > > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > index 51f5a0b181f9..2610638f4301 100644
> > > --- a/fs/nfsd/vfs.c
> > > +++ b/fs/nfsd/vfs.c
> > > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > > inode->i_mode,
> > > IS_IMMUTABLE(inode)? " immut" : "",
> > > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > > return nfserr_perm;
> > >
> > > - if (acc & NFSD_MAY_LOCK) {
> > > - /* If we cannot rely on authentication in NLM requests,
> > > - * just allow locks, otherwise require read permission, or
> > > - * ownership
> > > - */
> > > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > > - return 0;
> > > - else
> > > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > > - }
> > > /*
> > > * The file owner always gets access permission for accesses that
> > > * would normally be checked at open time. This is to make
> > > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > > index 854fb95dfdca..f9b09b842856 100644
> > > --- a/fs/nfsd/vfs.h
> > > +++ b/fs/nfsd/vfs.h
> > > @@ -20,7 +20,7 @@
> > > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > > #define NFSD_MAY_SATTR 0x008
> > > #define NFSD_MAY_TRUNC 0x010
> > > -#define NFSD_MAY_LOCK 0x020
> > > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > > #define NFSD_MAY_MASK 0x03f
> > >
> > > /* extra hints to permission and open routines: */
> > >
> > > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > > --
> > > 2.46.0
> > >
> > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-11 21:42 ` NeilBrown
@ 2025-03-12 13:22 ` Olga Kornievskaia
2025-03-12 22:26 ` NeilBrown
2025-03-12 22:51 ` Olga Kornievskaia
0 siblings, 2 replies; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-12 13:22 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
>
> On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > >
> > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > >
> > > >
> > > > NFSD_MAY_LOCK means a few different things.
> > > > - it means that GSS is not required.
> > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > - it means that OWNER_OVERRIDE is allowed.
> > > >
> > > > None of these are specific to locking, they are specific to the NLM
> > > > protocol.
> > > > So:
> > > > - rename to NFSD_MAY_NLM
> > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > into fh_verify where other special-case tests on the MAY flags
> > > > happen. nfsd_permission() can be called from other places than
> > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > >
> > > This patch breaks NLM when used in combination with TLS.
> >
> > I was too quick to link this to TLS. It's presence of security policy
> > so sec=krb* causes the same problems.
> >
> > > If exports
> > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > won't give any locks and client will get "no locks available" error.
> > >
> > > >
> > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > ---
> > > >
> > > > No change from previous patch - the corruption in the email has been
> > > > avoided (I hope).
> > > >
> > > >
> > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > fs/nfsd/trace.h | 2 +-
> > > > fs/nfsd/vfs.c | 12 +-----------
> > > > fs/nfsd/vfs.h | 2 +-
> > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > --- a/fs/nfsd/lockd.c
> > > > +++ b/fs/nfsd/lockd.c
> > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > fh.fh_export = NULL;
> > > >
> > > > + /*
> > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > + * for NLM even when GSS is used for NFS.
> > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > + * after the file was opened.
> > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > + * for NLM requests.
> > > > + */
> > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > - access |= NFSD_MAY_LOCK;
> > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > fh_put(&fh);
> > > > - /* We return nlm error codes as nlm doesn't know
> > > > + /* We return nlm error codes as nlm doesn't know
> > > > * about nfsd, but nfsd does know about nlm..
> > > > */
> > > > switch (nfserr) {
> > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > index 40533f7c7297..6a831cb242df 100644
> > > > --- a/fs/nfsd/nfsfh.c
> > > > +++ b/fs/nfsd/nfsfh.c
> > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > if (error)
> > > > goto out;
> > > >
> > > > - /*
> > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > - * which clients virtually always use auth_sys for,
> > > > - * even while using RPCSEC_GSS for NFS.
> > > > - */
> > > > - if (access & NFSD_MAY_LOCK)
> > > > - goto skip_pseudoflavor_check;
> > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> >
> > I think this should either be an OR or the fact that "nlm but only
> > with insecurelock export option and not other" is the only way to
> > bypass checking is wrong. I think it's just a check for NLM that
> > stays.
>
> I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> to make it work.
>
> I assume the NLM request is arriving with AUTH_SYS authentication?
It does.
Just to give you a practical example. exports have
(rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
does an flock() on the file. What's more I have just now hit Kasan's
out-of-bounds warning on that. I'll have to see if that exists on 6.14
(as I'm debugging the matter on the commit of the patch itself and
thus on 6.12-rc now).
I will layout more reasoning but what allowed NLM to work was this
- /*
- * pseudoflavor restrictions are not enforced on NLM,
- * which clients virtually always use auth_sys for,
- * even while using RPCSEC_GSS for NFS.
- */
- if (access & NFSD_MAY_LOCK)
- goto skip_pseudoflavor_check;
but I don't know why the replacement doesn't work.
> So check_nfsd_access() is being called with may_bypass_gss and this:
>
> if (may_bypass_gss && (
> rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> for (f = exp->ex_flavors; f < end; f++) {
> if (f->pseudoflavor >= RPC_AUTH_DES)
> return 0;
> }
> }
>
> in check_nfsd_access() should succeed.
> Can you add some tracing and see what is happening in here?
> Maybe the "goto denied" earlier in the function is being reached. I
> don't fully understand the TLS code yet - maybe it needs some test on
> may_bypass_gss.
>
> Thanks,
> NeilBrown
>
>
> >
> > > > + /* NLM is allowed to fully bypass authentication */
> > > > + goto out;
> > > > +
> > > > if (access & NFSD_MAY_BYPASS_GSS)
> > > > may_bypass_gss = true;
> > > > /*
> > > > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > if (error)
> > > > goto out;
> > > >
> > > > -skip_pseudoflavor_check:
> > > > /* Finally, check access permissions. */
> > > > error = nfsd_permission(cred, exp, dentry, access);
> > > > out:
> > > > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > > > index b8470d4cbe99..3448e444d410 100644
> > > > --- a/fs/nfsd/trace.h
> > > > +++ b/fs/nfsd/trace.h
> > > > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > > > { NFSD_MAY_READ, "READ" }, \
> > > > { NFSD_MAY_SATTR, "SATTR" }, \
> > > > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > > > - { NFSD_MAY_LOCK, "LOCK" }, \
> > > > + { NFSD_MAY_NLM, "NLM" }, \
> > > > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > > > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > > > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > index 51f5a0b181f9..2610638f4301 100644
> > > > --- a/fs/nfsd/vfs.c
> > > > +++ b/fs/nfsd/vfs.c
> > > > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > > > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > > > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > > > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > > > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > > > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > > > inode->i_mode,
> > > > IS_IMMUTABLE(inode)? " immut" : "",
> > > > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > > > return nfserr_perm;
> > > >
> > > > - if (acc & NFSD_MAY_LOCK) {
> > > > - /* If we cannot rely on authentication in NLM requests,
> > > > - * just allow locks, otherwise require read permission, or
> > > > - * ownership
> > > > - */
> > > > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > > > - return 0;
> > > > - else
> > > > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > > > - }
> > > > /*
> > > > * The file owner always gets access permission for accesses that
> > > > * would normally be checked at open time. This is to make
> > > > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > > > index 854fb95dfdca..f9b09b842856 100644
> > > > --- a/fs/nfsd/vfs.h
> > > > +++ b/fs/nfsd/vfs.h
> > > > @@ -20,7 +20,7 @@
> > > > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > > > #define NFSD_MAY_SATTR 0x008
> > > > #define NFSD_MAY_TRUNC 0x010
> > > > -#define NFSD_MAY_LOCK 0x020
> > > > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > > > #define NFSD_MAY_MASK 0x03f
> > > >
> > > > /* extra hints to permission and open routines: */
> > > >
> > > > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > > > --
> > > > 2.46.0
> > > >
> > > >
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-12 13:22 ` Olga Kornievskaia
@ 2025-03-12 22:26 ` NeilBrown
2025-03-12 22:54 ` Olga Kornievskaia
2025-03-12 22:51 ` Olga Kornievskaia
1 sibling, 1 reply; 15+ messages in thread
From: NeilBrown @ 2025-03-12 22:26 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
> On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> >
> > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > >
> > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > >
> > > > >
> > > > > NFSD_MAY_LOCK means a few different things.
> > > > > - it means that GSS is not required.
> > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > >
> > > > > None of these are specific to locking, they are specific to the NLM
> > > > > protocol.
> > > > > So:
> > > > > - rename to NFSD_MAY_NLM
> > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > happen. nfsd_permission() can be called from other places than
> > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > >
> > > > This patch breaks NLM when used in combination with TLS.
> > >
> > > I was too quick to link this to TLS. It's presence of security policy
> > > so sec=krb* causes the same problems.
> > >
> > > > If exports
> > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > won't give any locks and client will get "no locks available" error.
> > > >
> > > > >
> > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > ---
> > > > >
> > > > > No change from previous patch - the corruption in the email has been
> > > > > avoided (I hope).
> > > > >
> > > > >
> > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > fs/nfsd/trace.h | 2 +-
> > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > fs/nfsd/vfs.h | 2 +-
> > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > --- a/fs/nfsd/lockd.c
> > > > > +++ b/fs/nfsd/lockd.c
> > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > fh.fh_export = NULL;
> > > > >
> > > > > + /*
> > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > + * for NLM even when GSS is used for NFS.
> > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > + * after the file was opened.
> > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > + * for NLM requests.
> > > > > + */
> > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > - access |= NFSD_MAY_LOCK;
> > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > fh_put(&fh);
> > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > * about nfsd, but nfsd does know about nlm..
> > > > > */
> > > > > switch (nfserr) {
> > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > --- a/fs/nfsd/nfsfh.c
> > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > if (error)
> > > > > goto out;
> > > > >
> > > > > - /*
> > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > - * which clients virtually always use auth_sys for,
> > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > - */
> > > > > - if (access & NFSD_MAY_LOCK)
> > > > > - goto skip_pseudoflavor_check;
> > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > >
> > > I think this should either be an OR or the fact that "nlm but only
> > > with insecurelock export option and not other" is the only way to
> > > bypass checking is wrong. I think it's just a check for NLM that
> > > stays.
> >
> > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > to make it work.
> >
> > I assume the NLM request is arriving with AUTH_SYS authentication?
>
> It does.
>
> Just to give you a practical example. exports have
> (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> does an flock() on the file. What's more I have just now hit Kasan's
> out-of-bounds warning on that. I'll have to see if that exists on 6.14
> (as I'm debugging the matter on the commit of the patch itself and
> thus on 6.12-rc now).
>
> I will layout more reasoning but what allowed NLM to work was this
> - /*
> - * pseudoflavor restrictions are not enforced on NLM,
> - * which clients virtually always use auth_sys for,
> - * even while using RPCSEC_GSS for NFS.
> - */
> - if (access & NFSD_MAY_LOCK)
> - goto skip_pseudoflavor_check;
>
> but I don't know why the replacement doesn't work.
Can you see if this fixes it?
Maybe we need to bypass tls as well as gss
Thanks,
NeilBrown
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1124,7 +1124,8 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
test_bit(XPT_PEER_AUTH, &xprt->xpt_flags))
goto ok;
}
- goto denied;
+ if (!may_bypass_gss)
+ goto denied;
ok:
/* legacy gss-only clients are always OK: */
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-12 13:22 ` Olga Kornievskaia
2025-03-12 22:26 ` NeilBrown
@ 2025-03-12 22:51 ` Olga Kornievskaia
2025-03-12 22:59 ` Olga Kornievskaia
2025-03-13 0:09 ` NeilBrown
1 sibling, 2 replies; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-12 22:51 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Wed, Mar 12, 2025 at 9:22 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> >
> > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > >
> > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > >
> > > > >
> > > > > NFSD_MAY_LOCK means a few different things.
> > > > > - it means that GSS is not required.
> > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > >
> > > > > None of these are specific to locking, they are specific to the NLM
> > > > > protocol.
> > > > > So:
> > > > > - rename to NFSD_MAY_NLM
> > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > happen. nfsd_permission() can be called from other places than
> > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > >
> > > > This patch breaks NLM when used in combination with TLS.
> > >
> > > I was too quick to link this to TLS. It's presence of security policy
> > > so sec=krb* causes the same problems.
> > >
> > > > If exports
> > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > won't give any locks and client will get "no locks available" error.
> > > >
> > > > >
> > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > ---
> > > > >
> > > > > No change from previous patch - the corruption in the email has been
> > > > > avoided (I hope).
> > > > >
> > > > >
> > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > fs/nfsd/trace.h | 2 +-
> > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > fs/nfsd/vfs.h | 2 +-
> > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > --- a/fs/nfsd/lockd.c
> > > > > +++ b/fs/nfsd/lockd.c
> > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > fh.fh_export = NULL;
> > > > >
> > > > > + /*
> > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > + * for NLM even when GSS is used for NFS.
> > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > + * after the file was opened.
> > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > + * for NLM requests.
> > > > > + */
> > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > - access |= NFSD_MAY_LOCK;
> > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > fh_put(&fh);
> > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > * about nfsd, but nfsd does know about nlm..
> > > > > */
> > > > > switch (nfserr) {
> > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > --- a/fs/nfsd/nfsfh.c
> > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > if (error)
> > > > > goto out;
> > > > >
> > > > > - /*
> > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > - * which clients virtually always use auth_sys for,
> > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > - */
> > > > > - if (access & NFSD_MAY_LOCK)
> > > > > - goto skip_pseudoflavor_check;
> > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > >
> > > I think this should either be an OR or the fact that "nlm but only
> > > with insecurelock export option and not other" is the only way to
> > > bypass checking is wrong. I think it's just a check for NLM that
> > > stays.
> >
> > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > to make it work.
> >
> > I assume the NLM request is arriving with AUTH_SYS authentication?
>
> It does.
>
> Just to give you a practical example. exports have
> (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> does an flock() on the file. What's more I have just now hit Kasan's
> out-of-bounds warning on that. I'll have to see if that exists on 6.14
> (as I'm debugging the matter on the commit of the patch itself and
> thus on 6.12-rc now).
>
> I will layout more reasoning but what allowed NLM to work was this
> - /*
> - * pseudoflavor restrictions are not enforced on NLM,
> - * which clients virtually always use auth_sys for,
> - * even while using RPCSEC_GSS for NFS.
> - */
> - if (access & NFSD_MAY_LOCK)
> - goto skip_pseudoflavor_check;
>
> but I don't know why the replacement doesn't work.
As I mentioned the patch removed the skip_pseudoflavor check (that for
NLM) would have bypassed calling check_nfsd_access(). Instead, the
problem is that even though may_bypass_gss is set to true it call into
nfsd4_spo_must_allow(rqstp) which now wrongly assumes there is
compound state (struct nfsd4_compound_state *cstate = &resp->cstate;)
... (but this is NLM). So it proceed to deference it if
(!cstate->minorversion) causing the KASAN to do the out-of-bound error
that I mentioned. It most of the time now cause a crash. But on the
off non-deterministic times when it completes it fails.
I really don't think calling into check_nfsd_access() is appropriate for NLM.
>
> > So check_nfsd_access() is being called with may_bypass_gss and this:
> >
> > if (may_bypass_gss && (
> > rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > for (f = exp->ex_flavors; f < end; f++) {
> > if (f->pseudoflavor >= RPC_AUTH_DES)
> > return 0;
> > }
> > }
> >
> > in check_nfsd_access() should succeed.
> > Can you add some tracing and see what is happening in here?
> > Maybe the "goto denied" earlier in the function is being reached. I
> > don't fully understand the TLS code yet - maybe it needs some test on
> > may_bypass_gss.
> >
> > Thanks,
> > NeilBrown
> >
> >
> > >
> > > > > + /* NLM is allowed to fully bypass authentication */
> > > > > + goto out;
> > > > > +
> > > > > if (access & NFSD_MAY_BYPASS_GSS)
> > > > > may_bypass_gss = true;
> > > > > /*
> > > > > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > if (error)
> > > > > goto out;
> > > > >
> > > > > -skip_pseudoflavor_check:
> > > > > /* Finally, check access permissions. */
> > > > > error = nfsd_permission(cred, exp, dentry, access);
> > > > > out:
> > > > > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > > > > index b8470d4cbe99..3448e444d410 100644
> > > > > --- a/fs/nfsd/trace.h
> > > > > +++ b/fs/nfsd/trace.h
> > > > > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > > > > { NFSD_MAY_READ, "READ" }, \
> > > > > { NFSD_MAY_SATTR, "SATTR" }, \
> > > > > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > > > > - { NFSD_MAY_LOCK, "LOCK" }, \
> > > > > + { NFSD_MAY_NLM, "NLM" }, \
> > > > > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > > > > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > > > > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > > index 51f5a0b181f9..2610638f4301 100644
> > > > > --- a/fs/nfsd/vfs.c
> > > > > +++ b/fs/nfsd/vfs.c
> > > > > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > > > > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > > > > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > > > > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > > > > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > > > > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > > > > inode->i_mode,
> > > > > IS_IMMUTABLE(inode)? " immut" : "",
> > > > > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > > > > return nfserr_perm;
> > > > >
> > > > > - if (acc & NFSD_MAY_LOCK) {
> > > > > - /* If we cannot rely on authentication in NLM requests,
> > > > > - * just allow locks, otherwise require read permission, or
> > > > > - * ownership
> > > > > - */
> > > > > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > > > > - return 0;
> > > > > - else
> > > > > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > > > > - }
> > > > > /*
> > > > > * The file owner always gets access permission for accesses that
> > > > > * would normally be checked at open time. This is to make
> > > > > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > > > > index 854fb95dfdca..f9b09b842856 100644
> > > > > --- a/fs/nfsd/vfs.h
> > > > > +++ b/fs/nfsd/vfs.h
> > > > > @@ -20,7 +20,7 @@
> > > > > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > > > > #define NFSD_MAY_SATTR 0x008
> > > > > #define NFSD_MAY_TRUNC 0x010
> > > > > -#define NFSD_MAY_LOCK 0x020
> > > > > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > > > > #define NFSD_MAY_MASK 0x03f
> > > > >
> > > > > /* extra hints to permission and open routines: */
> > > > >
> > > > > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > > > > --
> > > > > 2.46.0
> > > > >
> > > > >
> > >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-12 22:26 ` NeilBrown
@ 2025-03-12 22:54 ` Olga Kornievskaia
2025-03-13 0:06 ` NeilBrown
0 siblings, 1 reply; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-12 22:54 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Wed, Mar 12, 2025 at 6:27 PM NeilBrown <neilb@suse.de> wrote:
>
> On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
> > On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > >
> > > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > > >
> > > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > > >
> > > > > >
> > > > > > NFSD_MAY_LOCK means a few different things.
> > > > > > - it means that GSS is not required.
> > > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > > >
> > > > > > None of these are specific to locking, they are specific to the NLM
> > > > > > protocol.
> > > > > > So:
> > > > > > - rename to NFSD_MAY_NLM
> > > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > > happen. nfsd_permission() can be called from other places than
> > > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > > >
> > > > > This patch breaks NLM when used in combination with TLS.
> > > >
> > > > I was too quick to link this to TLS. It's presence of security policy
> > > > so sec=krb* causes the same problems.
> > > >
> > > > > If exports
> > > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > > won't give any locks and client will get "no locks available" error.
> > > > >
> > > > > >
> > > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > > ---
> > > > > >
> > > > > > No change from previous patch - the corruption in the email has been
> > > > > > avoided (I hope).
> > > > > >
> > > > > >
> > > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > > fs/nfsd/trace.h | 2 +-
> > > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > > fs/nfsd/vfs.h | 2 +-
> > > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > > --- a/fs/nfsd/lockd.c
> > > > > > +++ b/fs/nfsd/lockd.c
> > > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > > fh.fh_export = NULL;
> > > > > >
> > > > > > + /*
> > > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > > + * for NLM even when GSS is used for NFS.
> > > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > > + * after the file was opened.
> > > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > > + * for NLM requests.
> > > > > > + */
> > > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > > - access |= NFSD_MAY_LOCK;
> > > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > > fh_put(&fh);
> > > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > > * about nfsd, but nfsd does know about nlm..
> > > > > > */
> > > > > > switch (nfserr) {
> > > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > > --- a/fs/nfsd/nfsfh.c
> > > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > if (error)
> > > > > > goto out;
> > > > > >
> > > > > > - /*
> > > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > > - * which clients virtually always use auth_sys for,
> > > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > > - */
> > > > > > - if (access & NFSD_MAY_LOCK)
> > > > > > - goto skip_pseudoflavor_check;
> > > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > > >
> > > > I think this should either be an OR or the fact that "nlm but only
> > > > with insecurelock export option and not other" is the only way to
> > > > bypass checking is wrong. I think it's just a check for NLM that
> > > > stays.
> > >
> > > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > > to make it work.
> > >
> > > I assume the NLM request is arriving with AUTH_SYS authentication?
> >
> > It does.
> >
> > Just to give you a practical example. exports have
> > (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> > does an flock() on the file. What's more I have just now hit Kasan's
> > out-of-bounds warning on that. I'll have to see if that exists on 6.14
> > (as I'm debugging the matter on the commit of the patch itself and
> > thus on 6.12-rc now).
> >
> > I will layout more reasoning but what allowed NLM to work was this
> > - /*
> > - * pseudoflavor restrictions are not enforced on NLM,
> > - * which clients virtually always use auth_sys for,
> > - * even while using RPCSEC_GSS for NFS.
> > - */
> > - if (access & NFSD_MAY_LOCK)
> > - goto skip_pseudoflavor_check;
> >
> > but I don't know why the replacement doesn't work.
>
> Can you see if this fixes it?
> Maybe we need to bypass tls as well as gss
>
> Thanks,
> NeilBrown
>
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1124,7 +1124,8 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
> test_bit(XPT_PEER_AUTH, &xprt->xpt_flags))
> goto ok;
> }
> - goto denied;
> + if (!may_bypass_gss)
> + goto denied;
>
I don't think this would help in any way as NLM does pass may_bypass_gss...
> ok:
> /* legacy gss-only clients are always OK: */
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-12 22:51 ` Olga Kornievskaia
@ 2025-03-12 22:59 ` Olga Kornievskaia
2025-03-13 0:09 ` NeilBrown
1 sibling, 0 replies; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-12 22:59 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Wed, Mar 12, 2025 at 6:51 PM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Wed, Mar 12, 2025 at 9:22 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> >
> > On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > >
> > > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > > >
> > > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > > >
> > > > > >
> > > > > > NFSD_MAY_LOCK means a few different things.
> > > > > > - it means that GSS is not required.
> > > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > > >
> > > > > > None of these are specific to locking, they are specific to the NLM
> > > > > > protocol.
> > > > > > So:
> > > > > > - rename to NFSD_MAY_NLM
> > > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > > happen. nfsd_permission() can be called from other places than
> > > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > > >
> > > > > This patch breaks NLM when used in combination with TLS.
> > > >
> > > > I was too quick to link this to TLS. It's presence of security policy
> > > > so sec=krb* causes the same problems.
> > > >
> > > > > If exports
> > > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > > won't give any locks and client will get "no locks available" error.
> > > > >
> > > > > >
> > > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > > ---
> > > > > >
> > > > > > No change from previous patch - the corruption in the email has been
> > > > > > avoided (I hope).
> > > > > >
> > > > > >
> > > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > > fs/nfsd/trace.h | 2 +-
> > > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > > fs/nfsd/vfs.h | 2 +-
> > > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > > --- a/fs/nfsd/lockd.c
> > > > > > +++ b/fs/nfsd/lockd.c
> > > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > > fh.fh_export = NULL;
> > > > > >
> > > > > > + /*
> > > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > > + * for NLM even when GSS is used for NFS.
> > > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > > + * after the file was opened.
> > > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > > + * for NLM requests.
> > > > > > + */
> > > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > > - access |= NFSD_MAY_LOCK;
> > > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > > fh_put(&fh);
> > > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > > * about nfsd, but nfsd does know about nlm..
> > > > > > */
> > > > > > switch (nfserr) {
> > > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > > --- a/fs/nfsd/nfsfh.c
> > > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > if (error)
> > > > > > goto out;
> > > > > >
> > > > > > - /*
> > > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > > - * which clients virtually always use auth_sys for,
> > > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > > - */
> > > > > > - if (access & NFSD_MAY_LOCK)
> > > > > > - goto skip_pseudoflavor_check;
> > > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > > >
> > > > I think this should either be an OR or the fact that "nlm but only
> > > > with insecurelock export option and not other" is the only way to
> > > > bypass checking is wrong. I think it's just a check for NLM that
> > > > stays.
> > >
> > > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > > to make it work.
> > >
> > > I assume the NLM request is arriving with AUTH_SYS authentication?
> >
> > It does.
> >
> > Just to give you a practical example. exports have
> > (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> > does an flock() on the file. What's more I have just now hit Kasan's
> > out-of-bounds warning on that. I'll have to see if that exists on 6.14
> > (as I'm debugging the matter on the commit of the patch itself and
> > thus on 6.12-rc now).
> >
> > I will layout more reasoning but what allowed NLM to work was this
> > - /*
> > - * pseudoflavor restrictions are not enforced on NLM,
> > - * which clients virtually always use auth_sys for,
> > - * even while using RPCSEC_GSS for NFS.
> > - */
> > - if (access & NFSD_MAY_LOCK)
> > - goto skip_pseudoflavor_check;
> >
> > but I don't know why the replacement doesn't work.
>
> As I mentioned the patch removed the skip_pseudoflavor check (that for
> NLM) would have bypassed calling check_nfsd_access(). Instead, the
> problem is that even though may_bypass_gss is set to true it call into
> nfsd4_spo_must_allow(rqstp) which now wrongly assumes there is
> compound state (struct nfsd4_compound_state *cstate = &resp->cstate;)
> ... (but this is NLM). So it proceed to deference it if
> (!cstate->minorversion) causing the KASAN to do the out-of-bound error
> that I mentioned. It most of the time now cause a crash. But on the
> off non-deterministic times when it completes it fails.
/export *(rw,no_root_squash,sec=krb5:krb5i:krb5p)
sudo mount -o vers=3,sec=krb5 192.168.1.184:/export /mnt
then do flock -x /mnt/foobar .... results in...
[ 60.809906] ==================================================================
[ 60.810393] BUG: KASAN: slab-out-of-bounds in
nfsd4_spo_must_allow+0x2f0/0x370 [nfsd]
[ 60.810990] Read of size 4 at addr ffff0000aa307f88 by task lockd/5339
[ 60.811461]
[ 60.811576] CPU: 2 UID: 0 PID: 5339 Comm: lockd Kdump: loaded Not
tainted 6.14.0-rc1+ #38
[ 60.811580] Hardware name: VMware, Inc. VMware20,1/VBSA, BIOS
VMW201.00V.24006586.BA64.2406042154 06/04/2024
[ 60.811582] Call trace:
[ 60.811584] show_stack+0x34/0x98 (C)
[ 60.811592] dump_stack_lvl+0x80/0xa8
[ 60.811594] print_address_description.constprop.0+0x90/0x330
[ 60.811597] print_report+0x108/0x1f8
[ 60.811599] kasan_report+0xc8/0x120
[ 60.811603] __asan_report_load4_noabort+0x20/0x30
[ 60.811606] nfsd4_spo_must_allow+0x2f0/0x370 [nfsd]
[ 60.811630] check_nfsd_access+0x22c/0x3a8 [nfsd]
[ 60.811655] __fh_verify+0x81c/0x980 [nfsd]
[ 60.811679] fh_verify+0xb0/0x198 [nfsd]
[ 60.811702] nfsd_open+0x7c/0xd8 [nfsd]
[ 60.811726] nlm_fopen+0x14c/0x240 [nfsd]
[ 60.811749] nlm_do_fopen+0xb0/0x160 [lockd]
[ 60.811759] nlm_lookup_file+0x388/0x5e0 [lockd]
[ 60.811766] nlm4svc_retrieve_args+0x1fc/0x530 [lockd]
[ 60.811773] __nlm4svc_proc_lock+0x18c/0x328 [lockd]
[ 60.811780] nlm4svc_proc_lock+0x48/0x60 [lockd]
[ 60.811787] nlmsvc_dispatch+0xb4/0x200 [lockd]
[ 60.811794] svc_process_common+0xa24/0x18d8 [sunrpc]
[ 60.811831] svc_process+0x3d8/0x7f8 [sunrpc]
[ 60.811859] svc_handle_xprt+0x5d4/0xd70 [sunrpc]
[ 60.811888] svc_recv+0x2b8/0x690 [sunrpc]
[ 60.811916] lockd+0x154/0x298 [lockd]
[ 60.811923] kthread+0x2e8/0x388
[ 60.811929] ret_from_fork+0x10/0x20
[ 60.811931]
[ 60.819753] Allocated by task 5325:
[ 60.819963] kasan_save_stack+0x3c/0x70
[ 60.820194] kasan_save_track+0x20/0x40
[ 60.820423] kasan_save_alloc_info+0x40/0x58
[ 60.820672] __kasan_kmalloc+0xd4/0xd8
[ 60.820891] __kmalloc_node_noprof+0x17c/0x560
[ 60.821153] svc_prepare_thread+0x1cc/0x5c0 [sunrpc]
[ 60.821477] svc_start_kthreads+0x4f0/0x5c8 [sunrpc]
[ 60.821794] svc_set_num_threads+0xa8/0x100 [sunrpc]
[ 60.822113] lockd_up+0x1b8/0x5c8 [lockd]
[ 60.822361] nfsd_startup_net+0x364/0x508 [nfsd]
[ 60.822662] nfsd_svc+0x178/0x310 [nfsd]
[ 60.822919] nfsd_nl_threads_set_doit+0x3ac/0x900 [nfsd]
[ 60.823256] genl_family_rcv_msg_doit+0x1c8/0x2a0
[ 60.823535] genl_family_rcv_msg+0x360/0x4b8
[ 60.823790] genl_rcv_msg+0xb8/0x168
[ 60.824000] netlink_rcv_skb+0x1bc/0x370
[ 60.824233] genl_rcv+0x40/0x60
[ 60.824423] netlink_unicast+0x5a8/0x820
[ 60.824656] netlink_sendmsg+0x644/0xa60
[ 60.824892] __sock_sendmsg+0xd0/0x180
[ 60.825120] ____sys_sendmsg+0x4b4/0x650
[ 60.825348] ___sys_sendmsg+0x128/0x1b0
[ 60.825562] __sys_sendmsg+0x110/0x198
[ 60.825773] __arm64_sys_sendmsg+0x78/0xb0
[ 60.826003] invoke_syscall.constprop.0+0xdc/0x1e8
[ 60.826276] do_el0_svc+0x154/0x1d0
[ 60.826481] el0_svc+0x40/0xe8
[ 60.826651] el0t_64_sync_handler+0x10c/0x138
[ 60.826884] el0t_64_sync+0x1ac/0x1b0
[ 60.827082]
[ 60.827164] The buggy address belongs to the object at ffff0000aa307c00
[ 60.827164] which belongs to the cache kmalloc-512 of size 512
[ 60.827840] The buggy address is located 464 bytes to the right of
[ 60.827840] allocated 440-byte region [ffff0000aa307c00, ffff0000aa307db8)
[ 60.828549]
[ 60.828641] The buggy address belongs to the physical page:
[ 60.828964] page: refcount:0 mapcount:0 mapping:0000000000000000
index:0x0 pfn:0x12a304
[ 60.829414] head: order:2 mapcount:0 entire_mapcount:0
nr_pages_mapped:0 pincount:0
[ 60.829841] flags: 0x2fffff00000040(head|node=0|zone=2|lastcpupid=0xfffff)
[ 60.830248] page_type: f5(slab)
[ 60.830426] raw: 002fffff00000040 ffff000080002c80 dead000000000100
dead000000000122
[ 60.830860] raw: 0000000000000000 0000000000100010 00000000f5000000
0000000000000000
[ 60.831287] head: 002fffff00000040 ffff000080002c80
dead000000000100 dead000000000122
[ 60.831725] head: 0000000000000000 0000000000100010
00000000f5000000 0000000000000000
[ 60.832158] head: 002fffff00000002 fffffdffc2a8c101
ffffffffffffffff 0000000000000000
[ 60.832589] head: 0000000000000004 0000000000000000
00000000ffffffff 0000000000000000
[ 60.833023] page dumped because: kasan: bad access detected
[ 60.833325]
[ 60.833410] Memory state around the buggy address:
[ 60.833667] ffff0000aa307e80: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[ 60.834063] ffff0000aa307f00: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[ 60.834452] >ffff0000aa307f80: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[ 60.834818] ^
[ 60.834996] ffff0000aa308000: fa fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 60.835398] ffff0000aa308080: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 60.835805] ==================================================================
[ 60.836388] Disabling lock debugging due to kernel taint
[ 60.836709] Unable to handle kernel paging request at virtual
address dfff80a081b0d020
[ 60.837066] KASAN: probably user-memory-access in range
[0x000005040d868100-0x000005040d868107]
[ 60.837469] Mem abort info:
[ 60.837583] ESR = 0x0000000096000004
[ 60.837760] EC = 0x25: DABT (current EL), IL = 32 bits
[ 60.838007] SET = 0, FnV = 0
[ 60.838146] EA = 0, S1PTW = 0
[ 60.838289] FSC = 0x04: level 0 translation fault
[ 60.838503] Data abort info:
[ 60.838628] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 60.838865] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 60.839074] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 60.839296] [dfff80a081b0d020] address between user and kernel address ranges
[ 60.839609] Internal error: Oops: 0000000096000004 [#1] SMP
[ 60.839858] Modules linked in: rpcsec_gss_krb5 nfsv3 nfs netfs
rpcrdma rdma_cm iw_cm ib_cm ib_core nfsd nfs_acl lockd grace
nfs_localio overlay isofs uinput snd_seq_dummy snd_hrtimer qrtr rfkill
vfat fat snd_hda_codec_generic uvcvideo videobuf2_vmalloc
videobuf2_memops uvc snd_hda_intel videobuf2_v4l2 videobuf2_common
videodev snd_intel_dspcfg snd_hda_codec e1000e snd_hda_core mc
snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore sg
auth_rpcgss sunrpc dm_multipath dm_mod loop nfnetlink vsock_loopback
vmw_vsock_virtio_transport_common vmw_vsock_vmci_transport vmw_vmci
vsock xfs ghash_ce nvme sha2_ce vmwgfx nvme_core sha256_arm64 sha1_ce
nvme_auth sr_mod cdrom drm_ttm_helper ttm fuse
[ 60.842700] CPU: 2 UID: 0 PID: 5339 Comm: lockd Kdump: loaded
Tainted: G B 6.14.0-rc1+ #38
[ 60.843266] Tainted: [B]=BAD_PAGE
[ 60.843427] Hardware name: VMware, Inc. VMware20,1/VBSA, BIOS
VMW201.00V.24006586.BA64.2406042154 06/04/2024
[ 60.844156] pstate: 41400005 (nZcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 60.844511] pc : nfsd4_spo_must_allow+0x1a4/0x370 [nfsd]
[ 60.844825] lr : nfsd4_spo_must_allow+0x334/0x370 [nfsd]
[ 60.845129] sp : ffff80008b2e7410
[ 60.845291] x29: ffff80008b2e7410 x28: 0000000000000003 x27: 000000000000a534
[ 60.845641] x26: ffff00009b7e3c00 x25: dfff800000000000 x24: 0000000000000000
[ 60.845988] x23: ffff0000aa307c28 x22: ffff0000aa307f7c x21: 0000000000000000
[ 60.846304] x20: 0000000000000001 x19: 000005040d868107 x18: 1fffe00011f1183c
[ 60.846612] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[ 60.846932] x14: 0000000000000000 x13: 0000000000000001 x12: ffff700010b4c349
[ 60.847244] x11: 1ffff00010b4c348 x10: ffff700010b4c348 x9 : dfff800000000000
[ 60.847551] x8 : ffff80008b2e7400 x7 : 0000000000000000 x6 : 0000000000000001
[ 60.847907] x5 : ffff6000136fc787 x4 : ffff0000a1452c80 x3 : 0000000000000020
[ 60.848241] x2 : 000000a081b0d020 x1 : ffff0000a1624000 x0 : 0000000000000000
[ 60.848579] Call trace:
[ 60.848699] nfsd4_spo_must_allow+0x1a4/0x370 [nfsd] (P)
[ 60.849081] check_nfsd_access+0x22c/0x3a8 [nfsd]
[ 60.849340] __fh_verify+0x81c/0x980 [nfsd]
[ 60.849610] fh_verify+0xb0/0x198 [nfsd]
[ 60.849826] nfsd_open+0x7c/0xd8 [nfsd]
[ 60.850057] nlm_fopen+0x14c/0x240 [nfsd]
[ 60.850323] nlm_do_fopen+0xb0/0x160 [lockd]
[ 60.850546] nlm_lookup_file+0x388/0x5e0 [lockd]
[ 60.850776] nlm4svc_retrieve_args+0x1fc/0x530 [lockd]
[ 60.851031] __nlm4svc_proc_lock+0x18c/0x328 [lockd]
[ 60.851290] nlm4svc_proc_lock+0x48/0x60 [lockd]
[ 60.851534] nlmsvc_dispatch+0xb4/0x200 [lockd]
[ 60.851816] svc_process_common+0xa24/0x18d8 [sunrpc]
[ 60.852108] svc_process+0x3d8/0x7f8 [sunrpc]
[ 60.852349] svc_handle_xprt+0x5d4/0xd70 [sunrpc]
[ 60.852614] svc_recv+0x2b8/0x690 [sunrpc]
[ 60.852846] lockd+0x154/0x298 [lockd]
[ 60.853037] kthread+0x2e8/0x388
[ 60.853209] ret_from_fork+0x10/0x20
[ 60.853397] Code: f9401f53 11000694 8b180273 d343fe62 (38f96842)
[ 60.853700] SMP: stopping secondary CPUs
[ 60.854775] Starting crashdump kernel...
[ 60.854968] Bye!
>
> I really don't think calling into check_nfsd_access() is appropriate for NLM.
>
> >
> > > So check_nfsd_access() is being called with may_bypass_gss and this:
> > >
> > > if (may_bypass_gss && (
> > > rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > > rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > > for (f = exp->ex_flavors; f < end; f++) {
> > > if (f->pseudoflavor >= RPC_AUTH_DES)
> > > return 0;
> > > }
> > > }
> > >
> > > in check_nfsd_access() should succeed.
> > > Can you add some tracing and see what is happening in here?
> > > Maybe the "goto denied" earlier in the function is being reached. I
> > > don't fully understand the TLS code yet - maybe it needs some test on
> > > may_bypass_gss.
> > >
> > > Thanks,
> > > NeilBrown
> > >
> > >
> > > >
> > > > > > + /* NLM is allowed to fully bypass authentication */
> > > > > > + goto out;
> > > > > > +
> > > > > > if (access & NFSD_MAY_BYPASS_GSS)
> > > > > > may_bypass_gss = true;
> > > > > > /*
> > > > > > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > if (error)
> > > > > > goto out;
> > > > > >
> > > > > > -skip_pseudoflavor_check:
> > > > > > /* Finally, check access permissions. */
> > > > > > error = nfsd_permission(cred, exp, dentry, access);
> > > > > > out:
> > > > > > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > > > > > index b8470d4cbe99..3448e444d410 100644
> > > > > > --- a/fs/nfsd/trace.h
> > > > > > +++ b/fs/nfsd/trace.h
> > > > > > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > > > > > { NFSD_MAY_READ, "READ" }, \
> > > > > > { NFSD_MAY_SATTR, "SATTR" }, \
> > > > > > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > > > > > - { NFSD_MAY_LOCK, "LOCK" }, \
> > > > > > + { NFSD_MAY_NLM, "NLM" }, \
> > > > > > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > > > > > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > > > > > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > > > index 51f5a0b181f9..2610638f4301 100644
> > > > > > --- a/fs/nfsd/vfs.c
> > > > > > +++ b/fs/nfsd/vfs.c
> > > > > > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > > > > > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > > > > > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > > > > > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > > > > > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > > > > > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > > > > > inode->i_mode,
> > > > > > IS_IMMUTABLE(inode)? " immut" : "",
> > > > > > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > > > > > return nfserr_perm;
> > > > > >
> > > > > > - if (acc & NFSD_MAY_LOCK) {
> > > > > > - /* If we cannot rely on authentication in NLM requests,
> > > > > > - * just allow locks, otherwise require read permission, or
> > > > > > - * ownership
> > > > > > - */
> > > > > > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > > > > > - return 0;
> > > > > > - else
> > > > > > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > > > > > - }
> > > > > > /*
> > > > > > * The file owner always gets access permission for accesses that
> > > > > > * would normally be checked at open time. This is to make
> > > > > > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > > > > > index 854fb95dfdca..f9b09b842856 100644
> > > > > > --- a/fs/nfsd/vfs.h
> > > > > > +++ b/fs/nfsd/vfs.h
> > > > > > @@ -20,7 +20,7 @@
> > > > > > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > > > > > #define NFSD_MAY_SATTR 0x008
> > > > > > #define NFSD_MAY_TRUNC 0x010
> > > > > > -#define NFSD_MAY_LOCK 0x020
> > > > > > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > > > > > #define NFSD_MAY_MASK 0x03f
> > > > > >
> > > > > > /* extra hints to permission and open routines: */
> > > > > >
> > > > > > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > > > > > --
> > > > > > 2.46.0
> > > > > >
> > > > > >
> > > >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-12 22:54 ` Olga Kornievskaia
@ 2025-03-13 0:06 ` NeilBrown
0 siblings, 0 replies; 15+ messages in thread
From: NeilBrown @ 2025-03-13 0:06 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
> On Wed, Mar 12, 2025 at 6:27 PM NeilBrown <neilb@suse.de> wrote:
> >
> > On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
> > > On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > >
> > > > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > > > >
> > > > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > > > >
> > > > > > >
> > > > > > > NFSD_MAY_LOCK means a few different things.
> > > > > > > - it means that GSS is not required.
> > > > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > > > >
> > > > > > > None of these are specific to locking, they are specific to the NLM
> > > > > > > protocol.
> > > > > > > So:
> > > > > > > - rename to NFSD_MAY_NLM
> > > > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > > > happen. nfsd_permission() can be called from other places than
> > > > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > > > >
> > > > > > This patch breaks NLM when used in combination with TLS.
> > > > >
> > > > > I was too quick to link this to TLS. It's presence of security policy
> > > > > so sec=krb* causes the same problems.
> > > > >
> > > > > > If exports
> > > > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > > > won't give any locks and client will get "no locks available" error.
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > > > ---
> > > > > > >
> > > > > > > No change from previous patch - the corruption in the email has been
> > > > > > > avoided (I hope).
> > > > > > >
> > > > > > >
> > > > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > > > fs/nfsd/trace.h | 2 +-
> > > > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > > > fs/nfsd/vfs.h | 2 +-
> > > > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > > > >
> > > > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > > > --- a/fs/nfsd/lockd.c
> > > > > > > +++ b/fs/nfsd/lockd.c
> > > > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > > > fh.fh_export = NULL;
> > > > > > >
> > > > > > > + /*
> > > > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > > > + * for NLM even when GSS is used for NFS.
> > > > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > > > + * after the file was opened.
> > > > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > > > + * for NLM requests.
> > > > > > > + */
> > > > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > > > - access |= NFSD_MAY_LOCK;
> > > > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > > > fh_put(&fh);
> > > > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > > > * about nfsd, but nfsd does know about nlm..
> > > > > > > */
> > > > > > > switch (nfserr) {
> > > > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > > > --- a/fs/nfsd/nfsfh.c
> > > > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > > if (error)
> > > > > > > goto out;
> > > > > > >
> > > > > > > - /*
> > > > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > > > - * which clients virtually always use auth_sys for,
> > > > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > > > - */
> > > > > > > - if (access & NFSD_MAY_LOCK)
> > > > > > > - goto skip_pseudoflavor_check;
> > > > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > > > >
> > > > > I think this should either be an OR or the fact that "nlm but only
> > > > > with insecurelock export option and not other" is the only way to
> > > > > bypass checking is wrong. I think it's just a check for NLM that
> > > > > stays.
> > > >
> > > > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > > > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > > > to make it work.
> > > >
> > > > I assume the NLM request is arriving with AUTH_SYS authentication?
> > >
> > > It does.
> > >
> > > Just to give you a practical example. exports have
> > > (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> > > does an flock() on the file. What's more I have just now hit Kasan's
> > > out-of-bounds warning on that. I'll have to see if that exists on 6.14
> > > (as I'm debugging the matter on the commit of the patch itself and
> > > thus on 6.12-rc now).
> > >
> > > I will layout more reasoning but what allowed NLM to work was this
> > > - /*
> > > - * pseudoflavor restrictions are not enforced on NLM,
> > > - * which clients virtually always use auth_sys for,
> > > - * even while using RPCSEC_GSS for NFS.
> > > - */
> > > - if (access & NFSD_MAY_LOCK)
> > > - goto skip_pseudoflavor_check;
> > >
> > > but I don't know why the replacement doesn't work.
> >
> > Can you see if this fixes it?
> > Maybe we need to bypass tls as well as gss
> >
> > Thanks,
> > NeilBrown
> >
> > --- a/fs/nfsd/export.c
> > +++ b/fs/nfsd/export.c
> > @@ -1124,7 +1124,8 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp,
> > test_bit(XPT_PEER_AUTH, &xprt->xpt_flags))
> > goto ok;
> > }
> > - goto denied;
> > + if (!may_bypass_gss)
> > + goto denied;
> >
>
> I don't think this would help in any way as NLM does pass may_bypass_gss...
Yes, so for NLM it won't "goto denied" but will fall through and pass
the other checks.
NeilBrown
>
> > ok:
> > /* legacy gss-only clients are always OK: */
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-12 22:51 ` Olga Kornievskaia
2025-03-12 22:59 ` Olga Kornievskaia
@ 2025-03-13 0:09 ` NeilBrown
2025-03-17 17:48 ` Olga Kornievskaia
1 sibling, 1 reply; 15+ messages in thread
From: NeilBrown @ 2025-03-13 0:09 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
> On Wed, Mar 12, 2025 at 9:22 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> >
> > On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > >
> > > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > > >
> > > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > > >
> > > > > >
> > > > > > NFSD_MAY_LOCK means a few different things.
> > > > > > - it means that GSS is not required.
> > > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > > >
> > > > > > None of these are specific to locking, they are specific to the NLM
> > > > > > protocol.
> > > > > > So:
> > > > > > - rename to NFSD_MAY_NLM
> > > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > > happen. nfsd_permission() can be called from other places than
> > > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > > >
> > > > > This patch breaks NLM when used in combination with TLS.
> > > >
> > > > I was too quick to link this to TLS. It's presence of security policy
> > > > so sec=krb* causes the same problems.
> > > >
> > > > > If exports
> > > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > > won't give any locks and client will get "no locks available" error.
> > > > >
> > > > > >
> > > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > > ---
> > > > > >
> > > > > > No change from previous patch - the corruption in the email has been
> > > > > > avoided (I hope).
> > > > > >
> > > > > >
> > > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > > fs/nfsd/trace.h | 2 +-
> > > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > > fs/nfsd/vfs.h | 2 +-
> > > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > > --- a/fs/nfsd/lockd.c
> > > > > > +++ b/fs/nfsd/lockd.c
> > > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > > fh.fh_export = NULL;
> > > > > >
> > > > > > + /*
> > > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > > + * for NLM even when GSS is used for NFS.
> > > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > > + * after the file was opened.
> > > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > > + * for NLM requests.
> > > > > > + */
> > > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > > - access |= NFSD_MAY_LOCK;
> > > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > > fh_put(&fh);
> > > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > > * about nfsd, but nfsd does know about nlm..
> > > > > > */
> > > > > > switch (nfserr) {
> > > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > > --- a/fs/nfsd/nfsfh.c
> > > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > if (error)
> > > > > > goto out;
> > > > > >
> > > > > > - /*
> > > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > > - * which clients virtually always use auth_sys for,
> > > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > > - */
> > > > > > - if (access & NFSD_MAY_LOCK)
> > > > > > - goto skip_pseudoflavor_check;
> > > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > > >
> > > > I think this should either be an OR or the fact that "nlm but only
> > > > with insecurelock export option and not other" is the only way to
> > > > bypass checking is wrong. I think it's just a check for NLM that
> > > > stays.
> > >
> > > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > > to make it work.
> > >
> > > I assume the NLM request is arriving with AUTH_SYS authentication?
> >
> > It does.
> >
> > Just to give you a practical example. exports have
> > (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> > does an flock() on the file. What's more I have just now hit Kasan's
> > out-of-bounds warning on that. I'll have to see if that exists on 6.14
> > (as I'm debugging the matter on the commit of the patch itself and
> > thus on 6.12-rc now).
> >
> > I will layout more reasoning but what allowed NLM to work was this
> > - /*
> > - * pseudoflavor restrictions are not enforced on NLM,
> > - * which clients virtually always use auth_sys for,
> > - * even while using RPCSEC_GSS for NFS.
> > - */
> > - if (access & NFSD_MAY_LOCK)
> > - goto skip_pseudoflavor_check;
> >
> > but I don't know why the replacement doesn't work.
>
> As I mentioned the patch removed the skip_pseudoflavor check (that for
> NLM) would have bypassed calling check_nfsd_access(). Instead, the
> problem is that even though may_bypass_gss is set to true it call into
> nfsd4_spo_must_allow(rqstp) which now wrongly assumes there is
> compound state (struct nfsd4_compound_state *cstate = &resp->cstate;)
> ... (but this is NLM). So it proceed to deference it if
> (!cstate->minorversion) causing the KASAN to do the out-of-bound error
> that I mentioned. It most of the time now cause a crash. But on the
> off non-deterministic times when it completes it fails.
>
> I really don't think calling into check_nfsd_access() is appropriate for NLM.
Why not? What is there is inherently inappropriate for NLM?
I agree that calling nfsd4_spo_must_allow(rqstp) isn't appropriate for
NLM, but it isn't appropriate for v2 or v3 either. We should add
version checks either before it is called or before the minorversion
check that it already has.
NeilBrown
>
> >
> > > So check_nfsd_access() is being called with may_bypass_gss and this:
> > >
> > > if (may_bypass_gss && (
> > > rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > > rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > > for (f = exp->ex_flavors; f < end; f++) {
> > > if (f->pseudoflavor >= RPC_AUTH_DES)
> > > return 0;
> > > }
> > > }
> > >
> > > in check_nfsd_access() should succeed.
> > > Can you add some tracing and see what is happening in here?
> > > Maybe the "goto denied" earlier in the function is being reached. I
> > > don't fully understand the TLS code yet - maybe it needs some test on
> > > may_bypass_gss.
> > >
> > > Thanks,
> > > NeilBrown
> > >
> > >
> > > >
> > > > > > + /* NLM is allowed to fully bypass authentication */
> > > > > > + goto out;
> > > > > > +
> > > > > > if (access & NFSD_MAY_BYPASS_GSS)
> > > > > > may_bypass_gss = true;
> > > > > > /*
> > > > > > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > if (error)
> > > > > > goto out;
> > > > > >
> > > > > > -skip_pseudoflavor_check:
> > > > > > /* Finally, check access permissions. */
> > > > > > error = nfsd_permission(cred, exp, dentry, access);
> > > > > > out:
> > > > > > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > > > > > index b8470d4cbe99..3448e444d410 100644
> > > > > > --- a/fs/nfsd/trace.h
> > > > > > +++ b/fs/nfsd/trace.h
> > > > > > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > > > > > { NFSD_MAY_READ, "READ" }, \
> > > > > > { NFSD_MAY_SATTR, "SATTR" }, \
> > > > > > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > > > > > - { NFSD_MAY_LOCK, "LOCK" }, \
> > > > > > + { NFSD_MAY_NLM, "NLM" }, \
> > > > > > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > > > > > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > > > > > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > > > index 51f5a0b181f9..2610638f4301 100644
> > > > > > --- a/fs/nfsd/vfs.c
> > > > > > +++ b/fs/nfsd/vfs.c
> > > > > > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > > > > > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > > > > > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > > > > > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > > > > > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > > > > > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > > > > > inode->i_mode,
> > > > > > IS_IMMUTABLE(inode)? " immut" : "",
> > > > > > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > > > > > return nfserr_perm;
> > > > > >
> > > > > > - if (acc & NFSD_MAY_LOCK) {
> > > > > > - /* If we cannot rely on authentication in NLM requests,
> > > > > > - * just allow locks, otherwise require read permission, or
> > > > > > - * ownership
> > > > > > - */
> > > > > > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > > > > > - return 0;
> > > > > > - else
> > > > > > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > > > > > - }
> > > > > > /*
> > > > > > * The file owner always gets access permission for accesses that
> > > > > > * would normally be checked at open time. This is to make
> > > > > > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > > > > > index 854fb95dfdca..f9b09b842856 100644
> > > > > > --- a/fs/nfsd/vfs.h
> > > > > > +++ b/fs/nfsd/vfs.h
> > > > > > @@ -20,7 +20,7 @@
> > > > > > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > > > > > #define NFSD_MAY_SATTR 0x008
> > > > > > #define NFSD_MAY_TRUNC 0x010
> > > > > > -#define NFSD_MAY_LOCK 0x020
> > > > > > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > > > > > #define NFSD_MAY_MASK 0x03f
> > > > > >
> > > > > > /* extra hints to permission and open routines: */
> > > > > >
> > > > > > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > > > > > --
> > > > > > 2.46.0
> > > > > >
> > > > > >
> > > >
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-13 0:09 ` NeilBrown
@ 2025-03-17 17:48 ` Olga Kornievskaia
2025-03-17 18:02 ` Chuck Lever
0 siblings, 1 reply; 15+ messages in thread
From: Olga Kornievskaia @ 2025-03-17 17:48 UTC (permalink / raw)
To: NeilBrown
Cc: Chuck Lever, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
linux-nfs
On Wed, Mar 12, 2025 at 8:09 PM NeilBrown <neilb@suse.de> wrote:
>
> On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
> > On Wed, Mar 12, 2025 at 9:22 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > >
> > > On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > >
> > > > On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
> > > > > On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
> > > > > >
> > > > > > On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
> > > > > > >
> > > > > > >
> > > > > > > NFSD_MAY_LOCK means a few different things.
> > > > > > > - it means that GSS is not required.
> > > > > > > - it means that with NFSEXP_NOAUTHNLM, authentication is not required
> > > > > > > - it means that OWNER_OVERRIDE is allowed.
> > > > > > >
> > > > > > > None of these are specific to locking, they are specific to the NLM
> > > > > > > protocol.
> > > > > > > So:
> > > > > > > - rename to NFSD_MAY_NLM
> > > > > > > - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
> > > > > > > so that NFSD_MAY_NLM doesn't need to imply these.
> > > > > > > - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
> > > > > > > into fh_verify where other special-case tests on the MAY flags
> > > > > > > happen. nfsd_permission() can be called from other places than
> > > > > > > fh_verify(), but none of these will have NFSD_MAY_NLM.
> > > > > >
> > > > > > This patch breaks NLM when used in combination with TLS.
> > > > >
> > > > > I was too quick to link this to TLS. It's presence of security policy
> > > > > so sec=krb* causes the same problems.
> > > > >
> > > > > > If exports
> > > > > > have xprtsec=tls:mtls and mount is done with tls/mtls, the server
> > > > > > won't give any locks and client will get "no locks available" error.
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > > > > > ---
> > > > > > >
> > > > > > > No change from previous patch - the corruption in the email has been
> > > > > > > avoided (I hope).
> > > > > > >
> > > > > > >
> > > > > > > fs/nfsd/lockd.c | 13 +++++++++++--
> > > > > > > fs/nfsd/nfsfh.c | 12 ++++--------
> > > > > > > fs/nfsd/trace.h | 2 +-
> > > > > > > fs/nfsd/vfs.c | 12 +-----------
> > > > > > > fs/nfsd/vfs.h | 2 +-
> > > > > > > 5 files changed, 18 insertions(+), 23 deletions(-)
> > > > > > >
> > > > > > > diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
> > > > > > > index 46a7f9b813e5..edc9f75dc75c 100644
> > > > > > > --- a/fs/nfsd/lockd.c
> > > > > > > +++ b/fs/nfsd/lockd.c
> > > > > > > @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
> > > > > > > memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
> > > > > > > fh.fh_export = NULL;
> > > > > > >
> > > > > > > + /*
> > > > > > > + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
> > > > > > > + * for NLM even when GSS is used for NFS.
> > > > > > > + * Allow OWNER_OVERRIDE as permission might have been changed
> > > > > > > + * after the file was opened.
> > > > > > > + * Pass MAY_NLM so that authentication can be completely bypassed
> > > > > > > + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
> > > > > > > + * for NLM requests.
> > > > > > > + */
> > > > > > > access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> > > > > > > - access |= NFSD_MAY_LOCK;
> > > > > > > + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
> > > > > > > nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
> > > > > > > fh_put(&fh);
> > > > > > > - /* We return nlm error codes as nlm doesn't know
> > > > > > > + /* We return nlm error codes as nlm doesn't know
> > > > > > > * about nfsd, but nfsd does know about nlm..
> > > > > > > */
> > > > > > > switch (nfserr) {
> > > > > > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > > > > > index 40533f7c7297..6a831cb242df 100644
> > > > > > > --- a/fs/nfsd/nfsfh.c
> > > > > > > +++ b/fs/nfsd/nfsfh.c
> > > > > > > @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > > if (error)
> > > > > > > goto out;
> > > > > > >
> > > > > > > - /*
> > > > > > > - * pseudoflavor restrictions are not enforced on NLM,
> > > > > > > - * which clients virtually always use auth_sys for,
> > > > > > > - * even while using RPCSEC_GSS for NFS.
> > > > > > > - */
> > > > > > > - if (access & NFSD_MAY_LOCK)
> > > > > > > - goto skip_pseudoflavor_check;
> > > > > > > + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
> > > > >
> > > > > I think this should either be an OR or the fact that "nlm but only
> > > > > with insecurelock export option and not other" is the only way to
> > > > > bypass checking is wrong. I think it's just a check for NLM that
> > > > > stays.
> > > >
> > > > I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
> > > > For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
> > > > to make it work.
> > > >
> > > > I assume the NLM request is arriving with AUTH_SYS authentication?
> > >
> > > It does.
> > >
> > > Just to give you a practical example. exports have
> > > (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
> > > does an flock() on the file. What's more I have just now hit Kasan's
> > > out-of-bounds warning on that. I'll have to see if that exists on 6.14
> > > (as I'm debugging the matter on the commit of the patch itself and
> > > thus on 6.12-rc now).
> > >
> > > I will layout more reasoning but what allowed NLM to work was this
> > > - /*
> > > - * pseudoflavor restrictions are not enforced on NLM,
> > > - * which clients virtually always use auth_sys for,
> > > - * even while using RPCSEC_GSS for NFS.
> > > - */
> > > - if (access & NFSD_MAY_LOCK)
> > > - goto skip_pseudoflavor_check;
> > >
> > > but I don't know why the replacement doesn't work.
> >
> > As I mentioned the patch removed the skip_pseudoflavor check (that for
> > NLM) would have bypassed calling check_nfsd_access(). Instead, the
> > problem is that even though may_bypass_gss is set to true it call into
> > nfsd4_spo_must_allow(rqstp) which now wrongly assumes there is
> > compound state (struct nfsd4_compound_state *cstate = &resp->cstate;)
> > ... (but this is NLM). So it proceed to deference it if
> > (!cstate->minorversion) causing the KASAN to do the out-of-bound error
> > that I mentioned. It most of the time now cause a crash. But on the
> > off non-deterministic times when it completes it fails.
> >
> > I really don't think calling into check_nfsd_access() is appropriate for NLM.
>
> Why not? What is there is inherently inappropriate for NLM?
I spoke too soon. It's not about calling into check_nfsd_acces()
(though it's a problem for v3 because there isn't a compound state!).
The real problem is "access" content that's being passed into the
nfsd_permission() function.
I don't fully understand the logic. But before this patch, "access"
(acc) was (re)set to "NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE" before
calling into inode_permission():
@@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct
svc_export *exp,
if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
return nfserr_perm;
- if (acc & NFSD_MAY_LOCK) {
- /* If we cannot rely on authentication in NLM requests,
- * just allow locks, otherwise require read permission, or
- * ownership
- */
- if (exp->ex_flags & NFSEXP_NOAUTHNLM)
- return 0;
- else
- acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
- }
/*
* The file owner always gets access permission for accesses that
* would normally be checked at open time. This is to make
now it's doesn't get "reset" and passes all of what was set in nlm_fopen()
access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
- access |= NFSD_MAY_LOCK;
+ access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
which ends up being "write" instead of a read and inode_permission returned -13.
Here's my proposed fix for one the problems in the patch.
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 4021b047eb18..eb139962ac4c 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2600,6 +2600,9 @@ nfsd_permission(struct svc_cred *cred, struct
svc_export *exp,
uid_eq(inode->i_uid, current_fsuid()))
return 0;
+ if (acc & NFSD_MAY_NLM)
+ acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
+
/* This assumes NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC} */
err = inode_permission(&nop_mnt_idmap, inode,
acc & (MAY_READ | MAY_WRITE | MAY_EXEC));
Now the 2nd problem. You mentioned checking for version before calling
nfsd4_spo_must_allow for v3. So something like this perhaps but not
sure if that's right?
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 0363720280d4..0106da76da89 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1148,8 +1148,9 @@ __be32 check_nfsd_access(struct svc_export *exp,
struct svc_rqst *rqstp,
* don't support it
*/
- if (nfsd4_spo_must_allow(rqstp))
- return nfs_ok;
+ if (rqstp->rq_prog == 100003)
+ 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
In the end, I question, why not revert the original patch instead?
> I agree that calling nfsd4_spo_must_allow(rqstp) isn't appropriate for
> NLM, but it isn't appropriate for v2 or v3 either. We should add
> version checks either before it is called or before the minorversion
> check that it already has.
>
> NeilBrown
>
>
> >
> > >
> > > > So check_nfsd_access() is being called with may_bypass_gss and this:
> > > >
> > > > if (may_bypass_gss && (
> > > > rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
> > > > rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
> > > > for (f = exp->ex_flavors; f < end; f++) {
> > > > if (f->pseudoflavor >= RPC_AUTH_DES)
> > > > return 0;
> > > > }
> > > > }
> > > >
> > > > in check_nfsd_access() should succeed.
> > > > Can you add some tracing and see what is happening in here?
> > > > Maybe the "goto denied" earlier in the function is being reached. I
> > > > don't fully understand the TLS code yet - maybe it needs some test on
> > > > may_bypass_gss.
> > > >
> > > > Thanks,
> > > > NeilBrown
> > > >
> > > >
> > > > >
> > > > > > > + /* NLM is allowed to fully bypass authentication */
> > > > > > > + goto out;
> > > > > > > +
> > > > > > > if (access & NFSD_MAY_BYPASS_GSS)
> > > > > > > may_bypass_gss = true;
> > > > > > > /*
> > > > > > > @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
> > > > > > > if (error)
> > > > > > > goto out;
> > > > > > >
> > > > > > > -skip_pseudoflavor_check:
> > > > > > > /* Finally, check access permissions. */
> > > > > > > error = nfsd_permission(cred, exp, dentry, access);
> > > > > > > out:
> > > > > > > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > > > > > > index b8470d4cbe99..3448e444d410 100644
> > > > > > > --- a/fs/nfsd/trace.h
> > > > > > > +++ b/fs/nfsd/trace.h
> > > > > > > @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
> > > > > > > { NFSD_MAY_READ, "READ" }, \
> > > > > > > { NFSD_MAY_SATTR, "SATTR" }, \
> > > > > > > { NFSD_MAY_TRUNC, "TRUNC" }, \
> > > > > > > - { NFSD_MAY_LOCK, "LOCK" }, \
> > > > > > > + { NFSD_MAY_NLM, "NLM" }, \
> > > > > > > { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
> > > > > > > { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
> > > > > > > { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
> > > > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > > > > index 51f5a0b181f9..2610638f4301 100644
> > > > > > > --- a/fs/nfsd/vfs.c
> > > > > > > +++ b/fs/nfsd/vfs.c
> > > > > > > @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > > > (acc & NFSD_MAY_EXEC)? " exec" : "",
> > > > > > > (acc & NFSD_MAY_SATTR)? " sattr" : "",
> > > > > > > (acc & NFSD_MAY_TRUNC)? " trunc" : "",
> > > > > > > - (acc & NFSD_MAY_LOCK)? " lock" : "",
> > > > > > > + (acc & NFSD_MAY_NLM)? " nlm" : "",
> > > > > > > (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
> > > > > > > inode->i_mode,
> > > > > > > IS_IMMUTABLE(inode)? " immut" : "",
> > > > > > > @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
> > > > > > > if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> > > > > > > return nfserr_perm;
> > > > > > >
> > > > > > > - if (acc & NFSD_MAY_LOCK) {
> > > > > > > - /* If we cannot rely on authentication in NLM requests,
> > > > > > > - * just allow locks, otherwise require read permission, or
> > > > > > > - * ownership
> > > > > > > - */
> > > > > > > - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> > > > > > > - return 0;
> > > > > > > - else
> > > > > > > - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> > > > > > > - }
> > > > > > > /*
> > > > > > > * The file owner always gets access permission for accesses that
> > > > > > > * would normally be checked at open time. This is to make
> > > > > > > diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> > > > > > > index 854fb95dfdca..f9b09b842856 100644
> > > > > > > --- a/fs/nfsd/vfs.h
> > > > > > > +++ b/fs/nfsd/vfs.h
> > > > > > > @@ -20,7 +20,7 @@
> > > > > > > #define NFSD_MAY_READ 0x004 /* == MAY_READ */
> > > > > > > #define NFSD_MAY_SATTR 0x008
> > > > > > > #define NFSD_MAY_TRUNC 0x010
> > > > > > > -#define NFSD_MAY_LOCK 0x020
> > > > > > > +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
> > > > > > > #define NFSD_MAY_MASK 0x03f
> > > > > > >
> > > > > > > /* extra hints to permission and open routines: */
> > > > > > >
> > > > > > > base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
> > > > > > > --
> > > > > > > 2.46.0
> > > > > > >
> > > > > > >
> > > > >
> >
>
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK
2025-03-17 17:48 ` Olga Kornievskaia
@ 2025-03-17 18:02 ` Chuck Lever
0 siblings, 0 replies; 15+ messages in thread
From: Chuck Lever @ 2025-03-17 18:02 UTC (permalink / raw)
To: Olga Kornievskaia, NeilBrown
Cc: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs
On 3/17/25 1:48 PM, Olga Kornievskaia wrote:
> On Wed, Mar 12, 2025 at 8:09 PM NeilBrown <neilb@suse.de> wrote:
>>
>> On Thu, 13 Mar 2025, Olga Kornievskaia wrote:
>>> On Wed, Mar 12, 2025 at 9:22 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>
>>>> On Tue, Mar 11, 2025 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
>>>>>
>>>>> On Wed, 12 Mar 2025, Olga Kornievskaia wrote:
>>>>>> On Tue, Mar 11, 2025 at 11:28 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>>>>>>>
>>>>>>> On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@suse.de> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> NFSD_MAY_LOCK means a few different things.
>>>>>>>> - it means that GSS is not required.
>>>>>>>> - it means that with NFSEXP_NOAUTHNLM, authentication is not required
>>>>>>>> - it means that OWNER_OVERRIDE is allowed.
>>>>>>>>
>>>>>>>> None of these are specific to locking, they are specific to the NLM
>>>>>>>> protocol.
>>>>>>>> So:
>>>>>>>> - rename to NFSD_MAY_NLM
>>>>>>>> - set NFSD_MAY_OWNER_OVERRIDE and NFSD_MAY_BYPASS_GSS in nlm_fopen()
>>>>>>>> so that NFSD_MAY_NLM doesn't need to imply these.
>>>>>>>> - move the test on NFSEXP_NOAUTHNLM out of nfsd_permission() and
>>>>>>>> into fh_verify where other special-case tests on the MAY flags
>>>>>>>> happen. nfsd_permission() can be called from other places than
>>>>>>>> fh_verify(), but none of these will have NFSD_MAY_NLM.
>>>>>>>
>>>>>>> This patch breaks NLM when used in combination with TLS.
>>>>>>
>>>>>> I was too quick to link this to TLS. It's presence of security policy
>>>>>> so sec=krb* causes the same problems.
>>>>>>
>>>>>>> If exports
>>>>>>> have xprtsec=tls:mtls and mount is done with tls/mtls, the server
>>>>>>> won't give any locks and client will get "no locks available" error.
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: NeilBrown <neilb@suse.de>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> No change from previous patch - the corruption in the email has been
>>>>>>>> avoided (I hope).
>>>>>>>>
>>>>>>>>
>>>>>>>> fs/nfsd/lockd.c | 13 +++++++++++--
>>>>>>>> fs/nfsd/nfsfh.c | 12 ++++--------
>>>>>>>> fs/nfsd/trace.h | 2 +-
>>>>>>>> fs/nfsd/vfs.c | 12 +-----------
>>>>>>>> fs/nfsd/vfs.h | 2 +-
>>>>>>>> 5 files changed, 18 insertions(+), 23 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
>>>>>>>> index 46a7f9b813e5..edc9f75dc75c 100644
>>>>>>>> --- a/fs/nfsd/lockd.c
>>>>>>>> +++ b/fs/nfsd/lockd.c
>>>>>>>> @@ -38,11 +38,20 @@ nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f, struct file **filp,
>>>>>>>> memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
>>>>>>>> fh.fh_export = NULL;
>>>>>>>>
>>>>>>>> + /*
>>>>>>>> + * Allow BYPASS_GSS as some client implementations use AUTH_SYS
>>>>>>>> + * for NLM even when GSS is used for NFS.
>>>>>>>> + * Allow OWNER_OVERRIDE as permission might have been changed
>>>>>>>> + * after the file was opened.
>>>>>>>> + * Pass MAY_NLM so that authentication can be completely bypassed
>>>>>>>> + * if NFSEXP_NOAUTHNLM is set. Some older clients use AUTH_NULL
>>>>>>>> + * for NLM requests.
>>>>>>>> + */
>>>>>>>> access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
>>>>>>>> - access |= NFSD_MAY_LOCK;
>>>>>>>> + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
>>>>>>>> nfserr = nfsd_open(rqstp, &fh, S_IFREG, access, filp);
>>>>>>>> fh_put(&fh);
>>>>>>>> - /* We return nlm error codes as nlm doesn't know
>>>>>>>> + /* We return nlm error codes as nlm doesn't know
>>>>>>>> * about nfsd, but nfsd does know about nlm..
>>>>>>>> */
>>>>>>>> switch (nfserr) {
>>>>>>>> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
>>>>>>>> index 40533f7c7297..6a831cb242df 100644
>>>>>>>> --- a/fs/nfsd/nfsfh.c
>>>>>>>> +++ b/fs/nfsd/nfsfh.c
>>>>>>>> @@ -363,13 +363,10 @@ __fh_verify(struct svc_rqst *rqstp,
>>>>>>>> if (error)
>>>>>>>> goto out;
>>>>>>>>
>>>>>>>> - /*
>>>>>>>> - * pseudoflavor restrictions are not enforced on NLM,
>>>>>>>> - * which clients virtually always use auth_sys for,
>>>>>>>> - * even while using RPCSEC_GSS for NFS.
>>>>>>>> - */
>>>>>>>> - if (access & NFSD_MAY_LOCK)
>>>>>>>> - goto skip_pseudoflavor_check;
>>>>>>>> + if ((access & NFSD_MAY_NLM) && (exp->ex_flags & NFSEXP_NOAUTHNLM))
>>>>>>
>>>>>> I think this should either be an OR or the fact that "nlm but only
>>>>>> with insecurelock export option and not other" is the only way to
>>>>>> bypass checking is wrong. I think it's just a check for NLM that
>>>>>> stays.
>>>>>
>>>>> I don't think that NLM gets a complete bypass unless no_auth_nlm is set.
>>>>> For the case you are describing, I think NFSD_MAY_BYPASS_GSS is supposed
>>>>> to make it work.
>>>>>
>>>>> I assume the NLM request is arriving with AUTH_SYS authentication?
>>>>
>>>> It does.
>>>>
>>>> Just to give you a practical example. exports have
>>>> (rw,...,sec=krb5:krb5i:krb5p). Client does mount with sec=krb5. Then
>>>> does an flock() on the file. What's more I have just now hit Kasan's
>>>> out-of-bounds warning on that. I'll have to see if that exists on 6.14
>>>> (as I'm debugging the matter on the commit of the patch itself and
>>>> thus on 6.12-rc now).
>>>>
>>>> I will layout more reasoning but what allowed NLM to work was this
>>>> - /*
>>>> - * pseudoflavor restrictions are not enforced on NLM,
>>>> - * which clients virtually always use auth_sys for,
>>>> - * even while using RPCSEC_GSS for NFS.
>>>> - */
>>>> - if (access & NFSD_MAY_LOCK)
>>>> - goto skip_pseudoflavor_check;
>>>>
>>>> but I don't know why the replacement doesn't work.
>>>
>>> As I mentioned the patch removed the skip_pseudoflavor check (that for
>>> NLM) would have bypassed calling check_nfsd_access(). Instead, the
>>> problem is that even though may_bypass_gss is set to true it call into
>>> nfsd4_spo_must_allow(rqstp) which now wrongly assumes there is
>>> compound state (struct nfsd4_compound_state *cstate = &resp->cstate;)
>>> ... (but this is NLM). So it proceed to deference it if
>>> (!cstate->minorversion) causing the KASAN to do the out-of-bound error
>>> that I mentioned. It most of the time now cause a crash. But on the
>>> off non-deterministic times when it completes it fails.
>>>
>>> I really don't think calling into check_nfsd_access() is appropriate for NLM.
>>
>> Why not? What is there is inherently inappropriate for NLM?
>
> I spoke too soon. It's not about calling into check_nfsd_acces()
> (though it's a problem for v3 because there isn't a compound state!).
> The real problem is "access" content that's being passed into the
> nfsd_permission() function.
>
> I don't fully understand the logic. But before this patch, "access"
> (acc) was (re)set to "NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE" before
> calling into inode_permission():
> @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct
> svc_export *exp,
> if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
> return nfserr_perm;
>
> - if (acc & NFSD_MAY_LOCK) {
> - /* If we cannot rely on authentication in NLM requests,
> - * just allow locks, otherwise require read permission, or
> - * ownership
> - */
> - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
> - return 0;
> - else
> - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> - }
> /*
> * The file owner always gets access permission for accesses that
> * would normally be checked at open time. This is to make
>
> now it's doesn't get "reset" and passes all of what was set in nlm_fopen()
>
> access = (mode == O_WRONLY) ? NFSD_MAY_WRITE : NFSD_MAY_READ;
> - access |= NFSD_MAY_LOCK;
> + access |= NFSD_MAY_NLM | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_BYPASS_GSS;
>
> which ends up being "write" instead of a read and inode_permission returned -13.
>
> Here's my proposed fix for one the problems in the patch.
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 4021b047eb18..eb139962ac4c 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2600,6 +2600,9 @@ nfsd_permission(struct svc_cred *cred, struct
> svc_export *exp,
> uid_eq(inode->i_uid, current_fsuid()))
> return 0;
>
> + if (acc & NFSD_MAY_NLM)
> + acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
> +
> /* This assumes NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC} */
> err = inode_permission(&nop_mnt_idmap, inode,
> acc & (MAY_READ | MAY_WRITE | MAY_EXEC));
>
> Now the 2nd problem. You mentioned checking for version before calling
> nfsd4_spo_must_allow for v3. So something like this perhaps but not
> sure if that's right?
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index 0363720280d4..0106da76da89 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1148,8 +1148,9 @@ __be32 check_nfsd_access(struct svc_export *exp,
> struct svc_rqst *rqstp,
> * don't support it
> */
>
> - if (nfsd4_spo_must_allow(rqstp))
> - return nfs_ok;
> + if (rqstp->rq_prog == 100003)
<shudder>
I guess we're stuck with this because this function is used for both
NFS and NLM file handles.
Let's use a symbolic constant here, at least.
> + 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
>
>
> In the end, I question, why not revert the original patch instead?
>
>> I agree that calling nfsd4_spo_must_allow(rqstp) isn't appropriate for
>> NLM, but it isn't appropriate for v2 or v3 either. We should add
>> version checks either before it is called or before the minorversion
>> check that it already has.
>>
>> NeilBrown
>>
>>
>>>
>>>>
>>>>> So check_nfsd_access() is being called with may_bypass_gss and this:
>>>>>
>>>>> if (may_bypass_gss && (
>>>>> rqstp->rq_cred.cr_flavor == RPC_AUTH_NULL ||
>>>>> rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX)) {
>>>>> for (f = exp->ex_flavors; f < end; f++) {
>>>>> if (f->pseudoflavor >= RPC_AUTH_DES)
>>>>> return 0;
>>>>> }
>>>>> }
>>>>>
>>>>> in check_nfsd_access() should succeed.
>>>>> Can you add some tracing and see what is happening in here?
>>>>> Maybe the "goto denied" earlier in the function is being reached. I
>>>>> don't fully understand the TLS code yet - maybe it needs some test on
>>>>> may_bypass_gss.
>>>>>
>>>>> Thanks,
>>>>> NeilBrown
>>>>>
>>>>>
>>>>>>
>>>>>>>> + /* NLM is allowed to fully bypass authentication */
>>>>>>>> + goto out;
>>>>>>>> +
>>>>>>>> if (access & NFSD_MAY_BYPASS_GSS)
>>>>>>>> may_bypass_gss = true;
>>>>>>>> /*
>>>>>>>> @@ -385,7 +382,6 @@ __fh_verify(struct svc_rqst *rqstp,
>>>>>>>> if (error)
>>>>>>>> goto out;
>>>>>>>>
>>>>>>>> -skip_pseudoflavor_check:
>>>>>>>> /* Finally, check access permissions. */
>>>>>>>> error = nfsd_permission(cred, exp, dentry, access);
>>>>>>>> out:
>>>>>>>> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
>>>>>>>> index b8470d4cbe99..3448e444d410 100644
>>>>>>>> --- a/fs/nfsd/trace.h
>>>>>>>> +++ b/fs/nfsd/trace.h
>>>>>>>> @@ -79,7 +79,7 @@ DEFINE_NFSD_XDR_ERR_EVENT(cant_encode);
>>>>>>>> { NFSD_MAY_READ, "READ" }, \
>>>>>>>> { NFSD_MAY_SATTR, "SATTR" }, \
>>>>>>>> { NFSD_MAY_TRUNC, "TRUNC" }, \
>>>>>>>> - { NFSD_MAY_LOCK, "LOCK" }, \
>>>>>>>> + { NFSD_MAY_NLM, "NLM" }, \
>>>>>>>> { NFSD_MAY_OWNER_OVERRIDE, "OWNER_OVERRIDE" }, \
>>>>>>>> { NFSD_MAY_LOCAL_ACCESS, "LOCAL_ACCESS" }, \
>>>>>>>> { NFSD_MAY_BYPASS_GSS_ON_ROOT, "BYPASS_GSS_ON_ROOT" }, \
>>>>>>>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>>>>>>>> index 51f5a0b181f9..2610638f4301 100644
>>>>>>>> --- a/fs/nfsd/vfs.c
>>>>>>>> +++ b/fs/nfsd/vfs.c
>>>>>>>> @@ -2509,7 +2509,7 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
>>>>>>>> (acc & NFSD_MAY_EXEC)? " exec" : "",
>>>>>>>> (acc & NFSD_MAY_SATTR)? " sattr" : "",
>>>>>>>> (acc & NFSD_MAY_TRUNC)? " trunc" : "",
>>>>>>>> - (acc & NFSD_MAY_LOCK)? " lock" : "",
>>>>>>>> + (acc & NFSD_MAY_NLM)? " nlm" : "",
>>>>>>>> (acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
>>>>>>>> inode->i_mode,
>>>>>>>> IS_IMMUTABLE(inode)? " immut" : "",
>>>>>>>> @@ -2534,16 +2534,6 @@ nfsd_permission(struct svc_cred *cred, struct svc_export *exp,
>>>>>>>> if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
>>>>>>>> return nfserr_perm;
>>>>>>>>
>>>>>>>> - if (acc & NFSD_MAY_LOCK) {
>>>>>>>> - /* If we cannot rely on authentication in NLM requests,
>>>>>>>> - * just allow locks, otherwise require read permission, or
>>>>>>>> - * ownership
>>>>>>>> - */
>>>>>>>> - if (exp->ex_flags & NFSEXP_NOAUTHNLM)
>>>>>>>> - return 0;
>>>>>>>> - else
>>>>>>>> - acc = NFSD_MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
>>>>>>>> - }
>>>>>>>> /*
>>>>>>>> * The file owner always gets access permission for accesses that
>>>>>>>> * would normally be checked at open time. This is to make
>>>>>>>> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
>>>>>>>> index 854fb95dfdca..f9b09b842856 100644
>>>>>>>> --- a/fs/nfsd/vfs.h
>>>>>>>> +++ b/fs/nfsd/vfs.h
>>>>>>>> @@ -20,7 +20,7 @@
>>>>>>>> #define NFSD_MAY_READ 0x004 /* == MAY_READ */
>>>>>>>> #define NFSD_MAY_SATTR 0x008
>>>>>>>> #define NFSD_MAY_TRUNC 0x010
>>>>>>>> -#define NFSD_MAY_LOCK 0x020
>>>>>>>> +#define NFSD_MAY_NLM 0x020 /* request is from lockd */
>>>>>>>> #define NFSD_MAY_MASK 0x03f
>>>>>>>>
>>>>>>>> /* extra hints to permission and open routines: */
>>>>>>>>
>>>>>>>> base-commit: c4e418a53fe30d8e1da68f5aabca352b682fd331
>>>>>>>> --
>>>>>>>> 2.46.0
>>>>>>>>
>>>>>>>>
>>>>>>
>>>
>>
--
Chuck Lever
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-03-17 18:02 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-17 21:42 [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK NeilBrown
2024-10-18 13:39 ` cel
2025-03-11 15:28 ` Olga Kornievskaia
2025-03-11 16:35 ` Olga Kornievskaia
2025-03-11 18:25 ` Olga Kornievskaia
2025-03-11 21:42 ` NeilBrown
2025-03-12 13:22 ` Olga Kornievskaia
2025-03-12 22:26 ` NeilBrown
2025-03-12 22:54 ` Olga Kornievskaia
2025-03-13 0:06 ` NeilBrown
2025-03-12 22:51 ` Olga Kornievskaia
2025-03-12 22:59 ` Olga Kornievskaia
2025-03-13 0:09 ` NeilBrown
2025-03-17 17:48 ` Olga Kornievskaia
2025-03-17 18:02 ` Chuck Lever
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox