Skip to contentSkip to conversation

NFSD: return NFS4ERR_EXIST for a guarded OPEN of a non-regular object

PATCH In review Chuck Lever 2 participants 2 messages Sep 6 – Sep 7

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Chuck Lever

author maintainer
#0 lore raw

PATCH 1/1 NFSD: return NFS4ERR_EXIST for a guarded OPEN of a non-regular object

Commit db8b567ffa98 ("nfsd: (almost) always open file in nfsd4_create_file()") moved the regular-file type check ahead of the createmode logic. A GUARDED4, EXCLUSIVE4, or EXCLUSIVE4_1 OPEN that finds a directory or symbolic link under the requested name now fails with NFS4ERR_ISDIR or NFS4ERR_SYMLINK instead of NFS4ERR_EXIST.

RFC 8881 Section 18.16.3 requires NFS4ERR_EXIST when a guarded create finds any object under that name, or when an exclusive create's verifier does not match. The type-specific errors in Section 18.16.4 apply to opening an existing file, which only UNCHECKED4 does.

Fixes db8b567ffa98 ("nfsd: (almost) always open file in nfsd4_create_file()")
Signed-off-by Chuck Lever <[email protected]>
---
 fs/nfsd/nfs4proc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Neil, I'll apply this to nfsd-testing. It should be in time to get into nfsd-next before v7.4.

fs/nfsd/nfs4proc.c +11 -1
index bb74eef43938..3a82af381a8d 100644
@@ -430,7 +430,17 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 				       oflags,
 				       open->op_iattr.ia_mode);
 	if (IS_ERR(open->op_filp)) {
-		status = nfserrno(PTR_ERR(open->op_filp));
+		int hosterr = PTR_ERR(open->op_filp);
+
+		if (open->op_createmode != NFS4_CREATE_UNCHECKED) {
+			switch (hosterr) {
+			case -EISDIR:
+			case -ELOOP:
+			case -EFTYPE:
+				hosterr = -EEXIST;
+			}
+		}
+		status = nfserrno(hosterr);
 		open->op_filp = NULL;
 		if (status == nfserr_noent && create_status)
 			status = create_status;
Signature · 1 line
2.54.0
#1 lore raw
Quoted 31 lines · Chuck Lever

Commit db8b567ffa98 ("nfsd: (almost) always open file in nfsd4_create_file()") moved the regular-file type check ahead of the createmode logic. A GUARDED4, EXCLUSIVE4, or EXCLUSIVE4_1 OPEN that finds a directory or symbolic link under the requested name now fails with NFS4ERR_ISDIR or NFS4ERR_SYMLINK instead of NFS4ERR_EXIST.

RFC 8881 Section 18.16.3 requires NFS4ERR_EXIST when a guarded create finds any object under that name, or when an exclusive create's verifier does not match. The type-specific errors in Section 18.16.4 apply to opening an existing file, which only UNCHECKED4 does.

Fixes db8b567ffa98 ("nfsd: (almost) always open file in nfsd4_create_file()")
Signed-off-by Chuck Lever <[email protected]>
---
 fs/nfsd/nfs4proc.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Neil, I'll apply this to nfsd-testing. It should be in time to get into nfsd-next before v7.4.

fs/nfsd/nfs4proc.c +1 -1
index bb74eef43938..3a82af381a8d 100644
@@ -430,7 +430,17 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
 				       oflags,
 				       open->op_iattr.ia_mode);
 	if (IS_ERR(open->op_filp)) {
-		status = nfserrno(PTR_ERR(open->op_filp));
+		int hosterr = PTR_ERR(open->op_filp);

fs/nfsd/nfs4proc.c nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, · line 443

Quoted 10 lines
fs/nfsd/nfs4proc.c +10 -0
+
+		if (open->op_createmode != NFS4_CREATE_UNCHECKED) {
+			switch (hosterr) {
+			case -EISDIR:
+			case -ELOOP:
+			case -EFTYPE:
+				hosterr = -EEXIST;
+			}
+		}
+		status = nfserrno(hosterr);

Thanks but I don't think this is the best place to fix it. I think it would be best to put the fix in do_lookup_open() so that it's behaviour more closely matches vfs_lookup_open().

I'll send an alternate patch.

Thanks,
NeilBrown

Quoted 7 lines
fs/nfsd/nfs4proc.c +0 -0
 		open->op_filp = NULL;
 		if (status == nfserr_noent && create_status)
 			status = create_status;
Signature · 1 line
2.54.0