Thread (13 messages) 13 messages, 5 authors, 2025-06-11

Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op

From: Chuck Lever <chuck.lever@oracle.com>
Date: 2025-06-10 14:01:55

On 6/10/25 9:59 AM, Jeff Layton wrote:
On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote:
quoted
On 6/10/25 9:50 AM, Jeff Layton wrote:
quoted
On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote:
quoted
When the client sends an OPEN with claim type CLAIM_DELEG_CUR_FH or
CLAIM_DELEGATION_CUR, the delegation stateid and the file handle
must belongs to the same file, otherwise return NFS4ERR_BAD_STATEID.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfsd/nfs4state.c | 5 +++++
 1 file changed, 5 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 59a693f22452..be2ee641a22d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6318,6 +6318,11 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 		status = nfs4_check_deleg(cl, open, &dp);
 		if (status)
 			goto out;
+		if (dp && nfsd4_is_deleg_cur(open) &&
+				(dp->dl_stid.sc_file != fp)) {
+			status = nfserr_bad_stateid;
+			goto out;
+		}
 		stp = nfsd4_find_and_lock_existing_open(fp, open);
 	} else {
 		open->op_file = NULL;
This seems like a good idea. I wonder if BAD_STATEID is the right error
here. It is a valid stateid, after all, it just doesn't match the
current_fh. Maybe this should be nfserr_inval ?
I agree, NFS4ERR_BAD_STATEID /might/ cause a loop, so that needs to be
tested. BAD_STATEID is mandated by the spec, so if we choose to return
a different status code here, it needs a comment explaining why.
Oh, I didn't realize that error was mandated, but you're right.
RFC8881, section 8.2.4:

- If the selected table entry does not match the current filehandle,
return NFS4ERR_BAD_STATEID.

I guess we're stuck with reporting that unless we want to amend the
spec.
It is spec-mandated behavior, but we are always free to ignore the
spec. I'm OK with NFS4ERR_INVAL if it results in better behavior
(as long as there is a comment explaining why we deviate from the
mandate).

quoted
quoted
In any case, whatever we decide:

Reviewed-by: Jeff Layton <jlayton@kernel.org>

-- 
Chuck Lever
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help