On 1/24/25 5:42 AM, Amir Goldstein wrote:
On Thu, Jan 23, 2025 at 10:07 PM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
On 1/23/25 3:43 PM, Jeff Layton wrote:
On Thu, 2025-01-23 at 14:52 -0500, cel@xxxxxxxxxx wrote:
From: Chuck Lever <chuck.lever@xxxxxxxxxx>
RFC 8881 Section 18.25.4 paragraph 5 tells us that the server
should return NFS4ERR_FILE_OPEN only if the target object is an
opened file. This suggests that returning this status when removing
a directory will confuse NFS clients.
This is a version-specific issue; nfsd_proc_remove/rmdir() and
nfsd3_proc_remove/rmdir() already return nfserr_access as
appropriate.
Unfortunately there is no quick way for nfsd4_remove() to determine
whether the target object is a file or not, so the check is done in
to nfsd_unlink() for now.
Reported-by: Trond Myklebust <trondmy@xxxxxxxxxxxxxxx>
Fixes: 466e16f0920f ("nfsd: check for EBUSY from vfs_rmdir/vfs_unink.")
Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---
fs/nfsd/vfs.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 2d8e27c225f9..3ead7fb3bf04 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1931,9 +1931,17 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
return err;
}
-/*
- * Unlink a file or directory
- * N.B. After this call fhp needs an fh_put
+/**
+ * nfsd_unlink - remove a directory entry
+ * @rqstp: RPC transaction context
+ * @fhp: the file handle of the parent directory to be modified
+ * @type: enforced file type of the object to be removed
+ * @fname: the name of directory entry to be removed
+ * @flen: length of @fname in octets
+ *
+ * After this call fhp needs an fh_put.
+ *
+ * Returns a generic NFS status code in network byte-order.
*/
__be32
nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
@@ -2007,10 +2015,14 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
fh_drop_write(fhp);
out_nfserr:
if (host_err == -EBUSY) {
- /* name is mounted-on. There is no perfect
- * error status.
+ /*
+ * See RFC 8881 Section 18.25.4 para 4: NFSv4 REMOVE
+ * distinguishes between reg file and dir.
*/
- err = nfserr_file_open;
+ if (type != S_IFDIR)
Should that be "if (type == S_ISREG)" instead? What if the inode is a
named pipe or device file? I'm not sure you can ever get EBUSY with
those, but in case you can, what's the right error in those cases?
Another way to put this:
If a client can acquire an OPEN state ID for those types of objects,
then NFS4ERR_FILE_OPEN is the correct status code in those cases.
But in practice, it depends on what existing client implementations
expect.
Check out nfsd_unlink()'s callers to see what they pass as the type
parameter. Unfortunately we have to compare against S_IFDIR here.
Not exactly. nfsd4_remove() is the only caller that needs to get
nfserr_file_open and this caller calls with type = 0, so type here
is going to be the actual type of the inode and (type == S_ISREG)
would be correct. No?
--
Chuck Lever
