Re: [PATCH v2] nfsd: refine and rename NFSD_MAY_LOCK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 17, 2024 at 5:42 PM NeilBrown <neilb@xxxxxxx> 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@xxxxxxx>
> ---
>
> 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
>
>





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux