2024-05-06 16:36:11

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 1/2] knfsd: LOOKUP can return an illegal error value

From: Trond Myklebust <[email protected]>

The 'NFS error' NFSERR_OPNOTSUPP is not described by any of the official
NFS related RFCs, but appears to have snuck into some older .x files for
NFSv2.
Either way, it is not in RFC1094, RFC1813 or any of the NFSv4 RFCs, so
should not be returned by the knfsd server, and particularly not by the
"LOOKUP" operation.

Instead, let's return NFSERR_STALE, which is more appropriate if the
filesystem encodes the filehandle as FILEID_INVALID.

Cc: [email protected]
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfsfh.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index dbfa0ac13564..d41e7630eb7a 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -572,7 +572,7 @@ fh_compose(struct svc_fh *fhp, struct svc_export *exp, struct dentry *dentry,
_fh_update(fhp, exp, dentry);
if (fhp->fh_handle.fh_fileid_type == FILEID_INVALID) {
fh_put(fhp);
- return nfserr_opnotsupp;
+ return nfserr_stale;
}

return 0;
@@ -598,7 +598,7 @@ fh_update(struct svc_fh *fhp)

_fh_update(fhp, fhp->fh_export, dentry);
if (fhp->fh_handle.fh_fileid_type == FILEID_INVALID)
- return nfserr_opnotsupp;
+ return nfserr_stale;
return 0;
out_bad:
printk(KERN_ERR "fh_update: fh not verified!\n");
--
2.45.0



2024-05-06 16:36:14

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 2/2] NFS/knfsd: Remove the invalid NFS error 'NFSERR_OPNOTSUPP'

From: Trond Myklebust <[email protected]>

NFSERR_OPNOTSUPP is not described by any RFC, and should not be used.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfsd.h | 1 -
include/trace/misc/nfs.h | 2 --
include/uapi/linux/nfs.h | 1 -
3 files changed, 4 deletions(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 304e9728b929..d1e4a8f159fd 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -229,7 +229,6 @@ void nfsd_lockd_shutdown(void);
#define nfserr_nospc cpu_to_be32(NFSERR_NOSPC)
#define nfserr_rofs cpu_to_be32(NFSERR_ROFS)
#define nfserr_mlink cpu_to_be32(NFSERR_MLINK)
-#define nfserr_opnotsupp cpu_to_be32(NFSERR_OPNOTSUPP)
#define nfserr_nametoolong cpu_to_be32(NFSERR_NAMETOOLONG)
#define nfserr_notempty cpu_to_be32(NFSERR_NOTEMPTY)
#define nfserr_dquot cpu_to_be32(NFSERR_DQUOT)
diff --git a/include/trace/misc/nfs.h b/include/trace/misc/nfs.h
index 5387eb0a6a08..d919e6e2c736 100644
--- a/include/trace/misc/nfs.h
+++ b/include/trace/misc/nfs.h
@@ -28,7 +28,6 @@ TRACE_DEFINE_ENUM(NFSERR_FBIG);
TRACE_DEFINE_ENUM(NFSERR_NOSPC);
TRACE_DEFINE_ENUM(NFSERR_ROFS);
TRACE_DEFINE_ENUM(NFSERR_MLINK);
-TRACE_DEFINE_ENUM(NFSERR_OPNOTSUPP);
TRACE_DEFINE_ENUM(NFSERR_NAMETOOLONG);
TRACE_DEFINE_ENUM(NFSERR_NOTEMPTY);
TRACE_DEFINE_ENUM(NFSERR_DQUOT);
@@ -64,7 +63,6 @@ TRACE_DEFINE_ENUM(NFSERR_JUKEBOX);
{ NFSERR_NOSPC, "NOSPC" }, \
{ NFSERR_ROFS, "ROFS" }, \
{ NFSERR_MLINK, "MLINK" }, \
- { NFSERR_OPNOTSUPP, "OPNOTSUPP" }, \
{ NFSERR_NAMETOOLONG, "NAMETOOLONG" }, \
{ NFSERR_NOTEMPTY, "NOTEMPTY" }, \
{ NFSERR_DQUOT, "DQUOT" }, \
diff --git a/include/uapi/linux/nfs.h b/include/uapi/linux/nfs.h
index 946cb62d64b0..f356f2ba3814 100644
--- a/include/uapi/linux/nfs.h
+++ b/include/uapi/linux/nfs.h
@@ -61,7 +61,6 @@
NFSERR_NOSPC = 28, /* v2 v3 v4 */
NFSERR_ROFS = 30, /* v2 v3 v4 */
NFSERR_MLINK = 31, /* v3 v4 */
- NFSERR_OPNOTSUPP = 45, /* v2 v3 */
NFSERR_NAMETOOLONG = 63, /* v2 v3 v4 */
NFSERR_NOTEMPTY = 66, /* v2 v3 v4 */
NFSERR_DQUOT = 69, /* v2 v3 v4 */
--
2.45.0


2024-05-06 16:52:31

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 1/2] knfsd: LOOKUP can return an illegal error value

On Mon, May 06, 2024 at 12:30:04PM -0400, [email protected] wrote:
> From: Trond Myklebust <[email protected]>
>
> The 'NFS error' NFSERR_OPNOTSUPP is not described by any of the official
> NFS related RFCs, but appears to have snuck into some older .x files for
> NFSv2.
> Either way, it is not in RFC1094, RFC1813 or any of the NFSv4 RFCs, so
> should not be returned by the knfsd server, and particularly not by the
> "LOOKUP" operation.
>
> Instead, let's return NFSERR_STALE, which is more appropriate if the
> filesystem encodes the filehandle as FILEID_INVALID.
>
> Cc: [email protected]
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfsd/nfsfh.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Hi, both applied to nfsd-next (for v6.10). Thanks!


> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index dbfa0ac13564..d41e7630eb7a 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -572,7 +572,7 @@ fh_compose(struct svc_fh *fhp, struct svc_export *exp, struct dentry *dentry,
> _fh_update(fhp, exp, dentry);
> if (fhp->fh_handle.fh_fileid_type == FILEID_INVALID) {
> fh_put(fhp);
> - return nfserr_opnotsupp;
> + return nfserr_stale;
> }
>
> return 0;
> @@ -598,7 +598,7 @@ fh_update(struct svc_fh *fhp)
>
> _fh_update(fhp, fhp->fh_export, dentry);
> if (fhp->fh_handle.fh_fileid_type == FILEID_INVALID)
> - return nfserr_opnotsupp;
> + return nfserr_stale;
> return 0;
> out_bad:
> printk(KERN_ERR "fh_update: fh not verified!\n");
> --
> 2.45.0
>

--
Chuck Lever