* [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op @ 2025-06-10 13:41 Dai Ngo 2025-06-10 13:50 ` Chuck Lever 2025-06-10 13:50 ` Jeff Layton 0 siblings, 2 replies; 13+ messages in thread From: Dai Ngo @ 2025-06-10 13:41 UTC (permalink / raw) To: chuck.lever, jlayton, neilb, okorniev, tom; +Cc: linux-nfs 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; -- 2.43.5 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 13:41 [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op Dai Ngo @ 2025-06-10 13:50 ` Chuck Lever 2025-06-10 13:55 ` Jeff Layton 2025-06-10 13:50 ` Jeff Layton 1 sibling, 1 reply; 13+ messages in thread From: Chuck Lever @ 2025-06-10 13:50 UTC (permalink / raw) To: Dai Ngo, jlayton, neilb, okorniev, tom; +Cc: linux-nfs On 6/10/25 9:41 AM, Dai Ngo wrote: > 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; How does the client react to NFS4ERR_BAD_STATEID ? Does it retry the same request or does it reflect the failure to the application? > + goto out; > + } > stp = nfsd4_find_and_lock_existing_open(fp, open); > } else { > open->op_file = NULL; -- Chuck Lever ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 13:50 ` Chuck Lever @ 2025-06-10 13:55 ` Jeff Layton 0 siblings, 0 replies; 13+ messages in thread From: Jeff Layton @ 2025-06-10 13:55 UTC (permalink / raw) To: Chuck Lever, Dai Ngo, neilb, okorniev, tom; +Cc: linux-nfs On Tue, 2025-06-10 at 09:50 -0400, Chuck Lever wrote: > On 6/10/25 9:41 AM, Dai Ngo wrote: > > 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; > > How does the client react to NFS4ERR_BAD_STATEID ? Does it retry the > same request or does it reflect the failure to the application? > I imagine this triggers the client to go into state recovery, which won't be terribly helpful. Another reason to consider a different error code. I suggest: 15.1.1.4. NFS4ERR_INVAL (Error Code 22) The arguments for this operation are not valid for some reason, even though they do match those specified in the XDR definition for the request. This might bubble up as an application error and get noticed that way. > > > + goto out; > > + } > > stp = nfsd4_find_and_lock_existing_open(fp, open); > > } else { > > open->op_file = NULL; > -- Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 13:41 [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op Dai Ngo 2025-06-10 13:50 ` Chuck Lever @ 2025-06-10 13:50 ` Jeff Layton 2025-06-10 13:52 ` Chuck Lever 1 sibling, 1 reply; 13+ messages in thread From: Jeff Layton @ 2025-06-10 13:50 UTC (permalink / raw) To: Dai Ngo, chuck.lever, neilb, okorniev, tom; +Cc: linux-nfs On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: > 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 ? In any case, whatever we decide: Reviewed-by: Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 13:50 ` Jeff Layton @ 2025-06-10 13:52 ` Chuck Lever 2025-06-10 13:59 ` Jeff Layton 0 siblings, 1 reply; 13+ messages in thread From: Chuck Lever @ 2025-06-10 13:52 UTC (permalink / raw) To: Jeff Layton, Dai Ngo, neilb, okorniev, tom; +Cc: linux-nfs On 6/10/25 9:50 AM, Jeff Layton wrote: > On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >> 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. > > In any case, whatever we decide: > > Reviewed-by: Jeff Layton <jlayton@kernel.org> -- Chuck Lever ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 13:52 ` Chuck Lever @ 2025-06-10 13:59 ` Jeff Layton 2025-06-10 14:01 ` Chuck Lever 0 siblings, 1 reply; 13+ messages in thread From: Jeff Layton @ 2025-06-10 13:59 UTC (permalink / raw) To: Chuck Lever, Dai Ngo, neilb, okorniev, tom; +Cc: linux-nfs On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: > On 6/10/25 9:50 AM, Jeff Layton wrote: > > On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: > > > 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. > > > > > In any case, whatever we decide: > > > > Reviewed-by: Jeff Layton <jlayton@kernel.org> > -- Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 13:59 ` Jeff Layton @ 2025-06-10 14:01 ` Chuck Lever 2025-06-10 14:12 ` Dai Ngo 0 siblings, 1 reply; 13+ messages in thread From: Chuck Lever @ 2025-06-10 14:01 UTC (permalink / raw) To: Jeff Layton, Dai Ngo, neilb, okorniev, tom; +Cc: linux-nfs On 6/10/25 9:59 AM, Jeff Layton wrote: > On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: >> On 6/10/25 9:50 AM, Jeff Layton wrote: >>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >>>> 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). >>> In any case, whatever we decide: >>> >>> Reviewed-by: Jeff Layton <jlayton@kernel.org> >> > -- Chuck Lever ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 14:01 ` Chuck Lever @ 2025-06-10 14:12 ` Dai Ngo 2025-06-10 14:14 ` Chuck Lever 0 siblings, 1 reply; 13+ messages in thread From: Dai Ngo @ 2025-06-10 14:12 UTC (permalink / raw) To: Chuck Lever, Jeff Layton, neilb, okorniev, tom; +Cc: linux-nfs On 6/10/25 7:01 AM, Chuck Lever wrote: > On 6/10/25 9:59 AM, Jeff Layton wrote: >> On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: >>> On 6/10/25 9:50 AM, Jeff Layton wrote: >>>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >>>>> 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). Since the Linux client does not behave this way I can not test if this error get us into a loop. I used pynfs to force this behavior. However, here is the comment in nfs4_do_open: /* * BAD_STATEID on OPEN means that the server cancelled our * state before it received the OPEN_CONFIRM. * Recover by retrying the request as per the discussion * on Page 181 of RFC3530. */ So it guess BAD_STATEID will get the client and server into a loop. I'll change error to NFS4ERR_INVAL and add a comment in the code. -Dai > > >>>> In any case, whatever we decide: >>>> >>>> Reviewed-by: Jeff Layton <jlayton@kernel.org> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 14:12 ` Dai Ngo @ 2025-06-10 14:14 ` Chuck Lever 2025-06-10 14:53 ` Frank Filz 0 siblings, 1 reply; 13+ messages in thread From: Chuck Lever @ 2025-06-10 14:14 UTC (permalink / raw) To: Dai Ngo, Jeff Layton, neilb, okorniev, tom; +Cc: linux-nfs On 6/10/25 10:12 AM, Dai Ngo wrote: > > On 6/10/25 7:01 AM, Chuck Lever wrote: >> On 6/10/25 9:59 AM, Jeff Layton wrote: >>> On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: >>>> On 6/10/25 9:50 AM, Jeff Layton wrote: >>>>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >>>>>> 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). > > Since the Linux client does not behave this way I can not test if this > error get us into a loop. Good point! > I used pynfs to force this behavior. > > However, here is the comment in nfs4_do_open: > > /* > * BAD_STATEID on OPEN means that the server cancelled our > * state before it received the OPEN_CONFIRM. > * Recover by retrying the request as per the discussion > * on Page 181 of RFC3530. > */ > > So it guess BAD_STATEID will get the client and server into a loop. > I'll change error to NFS4ERR_INVAL and add a comment in the code. Thanks, we'll start there. If that's problematic, it can always be changed later. Maybe someone should file an errata against RFC 8881. <whistles tunelessly> >>>>> In any case, whatever we decide: >>>>> >>>>> Reviewed-by: Jeff Layton <jlayton@kernel.org> >> -- Chuck Lever ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 14:14 ` Chuck Lever @ 2025-06-10 14:53 ` Frank Filz 2025-06-10 15:05 ` Dai Ngo 0 siblings, 1 reply; 13+ messages in thread From: Frank Filz @ 2025-06-10 14:53 UTC (permalink / raw) To: 'Chuck Lever', 'Dai Ngo', 'Jeff Layton', neilb, okorniev, tom Cc: linux-nfs From: Chuck Lever [mailto:chuck.lever@oracle.com] > On 6/10/25 10:12 AM, Dai Ngo wrote: > > > > On 6/10/25 7:01 AM, Chuck Lever wrote: > >> On 6/10/25 9:59 AM, Jeff Layton wrote: > >>> On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: > >>>> On 6/10/25 9:50 AM, Jeff Layton wrote: > >>>>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: > >>>>>> 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). > > > > Since the Linux client does not behave this way I can not test if this > > error get us into a loop. > > Good point! > > > > I used pynfs to force this behavior. > > > > However, here is the comment in nfs4_do_open: > > > > /* > > * BAD_STATEID on OPEN means that the server cancelled > > our > > * state before it received the OPEN_CONFIRM. > > * Recover by retrying the request as per the > > discussion > > * on Page 181 of RFC3530. > > */ > > > > So it guess BAD_STATEID will get the client and server into a loop. > > I'll change error to NFS4ERR_INVAL and add a comment in the code. > > Thanks, we'll start there. If that's problematic, it can always be changed later. > > Maybe someone should file an errata against RFC 8881. <whistles > tunelessly> An interesting case. Ganesha doesn't handle this. It would definitely be good to see an errata for it. Also a pynfs test case. Thanks Frank ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 14:53 ` Frank Filz @ 2025-06-10 15:05 ` Dai Ngo 2025-06-10 22:03 ` Calum Mackay 0 siblings, 1 reply; 13+ messages in thread From: Dai Ngo @ 2025-06-10 15:05 UTC (permalink / raw) To: Frank Filz, 'Chuck Lever', 'Jeff Layton', neilb, okorniev, tom Cc: linux-nfs On 6/10/25 7:53 AM, Frank Filz wrote: > From: Chuck Lever [mailto:chuck.lever@oracle.com] >> On 6/10/25 10:12 AM, Dai Ngo wrote: >>> On 6/10/25 7:01 AM, Chuck Lever wrote: >>>> On 6/10/25 9:59 AM, Jeff Layton wrote: >>>>> On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: >>>>>> On 6/10/25 9:50 AM, Jeff Layton wrote: >>>>>>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >>>>>>>> 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). >>> Since the Linux client does not behave this way I can not test if this >>> error get us into a loop. >> Good point! >> >> >>> I used pynfs to force this behavior. >>> >>> However, here is the comment in nfs4_do_open: >>> >>> /* >>> * BAD_STATEID on OPEN means that the server cancelled >>> our >>> * state before it received the OPEN_CONFIRM. >>> * Recover by retrying the request as per the >>> discussion >>> * on Page 181 of RFC3530. >>> */ >>> >>> So it guess BAD_STATEID will get the client and server into a loop. >>> I'll change error to NFS4ERR_INVAL and add a comment in the code. >> Thanks, we'll start there. If that's problematic, it can always be changed later. >> >> Maybe someone should file an errata against RFC 8881. <whistles >> tunelessly> > An interesting case. Ganesha doesn't handle this. It would definitely be good to see an errata for it. Also a pynfs test case. Here is the pynfs test I used to test CLAIM_DELEG_CUR_FH: def testClaimDeleg_CurFh(t, env): """Test OPEN with CLAIM_DELEG_CUR_FH with mismatch file file handle and delegation stateid. FLAGS: writedelegations deleg CODE: DELEG32 """ sess = env.c1.new_client_session(env.testname(t)) # create file-1 with read-only access (0444) no delegatation wanted, # and leave the file opened filename1 = b"file-1" res = open_create_file(sess, filename1, open_create = OPEN4_CREATE, attrs={FATTR4_MODE: 0o444}, access = OPEN4_SHARE_ACCESS_BOTH, want_deleg = False) check(res) deleg = res.resarray[-2].delegation if (_got_deleg(deleg)): fail("Not expect to get delegation") fh = res.resarray[-1].object stateid = res.resarray[-2].stateid print("----- CREATED ", filename1) # create file-2 with access RW and delegation wanted filename2 = b"file-2" res = open_create_file(sess, filename2, open_create = OPEN4_CREATE, access = OPEN4_SHARE_ACCESS_BOTH, want_deleg = True) check(res) print("----- CREATED ", filename2) wfh = res.resarray[-1].object wdeleg = res.resarray[-2].delegation if (not _got_deleg(wdeleg)): fail("Could not get WRITE delegation") wdelegstateid = wdeleg.write.stateid # OPEN for WRITE with CLAIM_DELEG_CUR_FH using the file handle # of filename1 and the delegation stateid granted for filename2. # Since the file handle and the delegation stateid do not belong # to the same file, expect server to return NFS4ERR_BAD_STATEID. claim = open_claim4(CLAIM_DELEG_CUR_FH, oc_delegate_stateid=wdelegstateid) owner = open_owner4(0, b"My Open Owner 2") how = openflag4(OPEN4_NOCREATE) open_op = op.open(0, OPEN4_SHARE_ACCESS_WRITE, OPEN4_SHARE_DENY_NONE, owner, how, claim) res = sess.compound([op.putfh(fh), open_op]) check(res, NFS4ERR_BAD_STATEID) # close file-1 res = close_file(sess, fh, stateid) check(res) # return the write delegation res = sess.compound([op.putfh(wfh), op.delegreturn(wdelegstateid)]) check(res) > > Thanks > > Frank > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 15:05 ` Dai Ngo @ 2025-06-10 22:03 ` Calum Mackay 2025-06-11 0:35 ` Dai Ngo 0 siblings, 1 reply; 13+ messages in thread From: Calum Mackay @ 2025-06-10 22:03 UTC (permalink / raw) To: Dai Ngo, Frank Filz, 'Chuck Lever', 'Jeff Layton', neilb, okorniev, tom Cc: Calum Mackay, linux-nfs On 10/06/2025 4:05 pm, Dai Ngo wrote: > > On 6/10/25 7:53 AM, Frank Filz wrote: >> From: Chuck Lever [mailto:chuck.lever@oracle.com] >>> On 6/10/25 10:12 AM, Dai Ngo wrote: >>>> On 6/10/25 7:01 AM, Chuck Lever wrote: >>>>> On 6/10/25 9:59 AM, Jeff Layton wrote: >>>>>> On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: >>>>>>> On 6/10/25 9:50 AM, Jeff Layton wrote: >>>>>>>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >>>>>>>>> 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). >>>> Since the Linux client does not behave this way I can not test if this >>>> error get us into a loop. >>> Good point! >>> >>> >>>> I used pynfs to force this behavior. >>>> >>>> However, here is the comment in nfs4_do_open: >>>> >>>> /* >>>> * BAD_STATEID on OPEN means that the server cancelled >>>> our >>>> * state before it received the OPEN_CONFIRM. >>>> * Recover by retrying the request as per the >>>> discussion >>>> * on Page 181 of RFC3530. >>>> */ >>>> >>>> So it guess BAD_STATEID will get the client and server into a loop. >>>> I'll change error to NFS4ERR_INVAL and add a comment in the code. >>> Thanks, we'll start there. If that's problematic, it can always be >>> changed later. >>> >>> Maybe someone should file an errata against RFC 8881. <whistles >>> tunelessly> >> An interesting case. Ganesha doesn't handle this. It would definitely >> be good to see an errata for it. Also a pynfs test case. > > Here is the pynfs test I used to test CLAIM_DELEG_CUR_FH: Thanks Dai; would you like to submit that as a pynfs patch, please? ta, c. > > def testClaimDeleg_CurFh(t, env): > """Test OPEN with CLAIM_DELEG_CUR_FH with mismatch file file handle > and delegation stateid. > > FLAGS: writedelegations deleg > CODE: DELEG32 > """ > > sess = env.c1.new_client_session(env.testname(t)) > > # create file-1 with read-only access (0444) no delegatation wanted, > # and leave the file opened > filename1 = b"file-1" > res = open_create_file(sess, filename1, open_create = OPEN4_CREATE, > attrs={FATTR4_MODE: 0o444}, > access = OPEN4_SHARE_ACCESS_BOTH, want_deleg = False) > check(res) > deleg = res.resarray[-2].delegation > if (_got_deleg(deleg)): > fail("Not expect to get delegation") > fh = res.resarray[-1].object > stateid = res.resarray[-2].stateid > print("----- CREATED ", filename1) > > # create file-2 with access RW and delegation wanted > filename2 = b"file-2" > res = open_create_file(sess, filename2, open_create = OPEN4_CREATE, > access = OPEN4_SHARE_ACCESS_BOTH, want_deleg = True) > check(res) > print("----- CREATED ", filename2) > > wfh = res.resarray[-1].object > wdeleg = res.resarray[-2].delegation > if (not _got_deleg(wdeleg)): > fail("Could not get WRITE delegation") > wdelegstateid = wdeleg.write.stateid > > # OPEN for WRITE with CLAIM_DELEG_CUR_FH using the file handle > # of filename1 and the delegation stateid granted for filename2. > # Since the file handle and the delegation stateid do not belong > # to the same file, expect server to return NFS4ERR_BAD_STATEID. > > claim = open_claim4(CLAIM_DELEG_CUR_FH, > oc_delegate_stateid=wdelegstateid) > owner = open_owner4(0, b"My Open Owner 2") > how = openflag4(OPEN4_NOCREATE) > open_op = op.open(0, OPEN4_SHARE_ACCESS_WRITE, OPEN4_SHARE_DENY_NONE, > owner, how, claim) > res = sess.compound([op.putfh(fh), open_op]) > check(res, NFS4ERR_BAD_STATEID) > > # close file-1 > res = close_file(sess, fh, stateid) > check(res) > > # return the write delegation > res = sess.compound([op.putfh(wfh), op.delegreturn(wdelegstateid)]) > check(res) > >> >> Thanks >> >> Frank >> > -- Calum Mackay Linux Kernel Engineering Oracle Linux and Virtualisation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op 2025-06-10 22:03 ` Calum Mackay @ 2025-06-11 0:35 ` Dai Ngo 0 siblings, 0 replies; 13+ messages in thread From: Dai Ngo @ 2025-06-11 0:35 UTC (permalink / raw) To: Calum Mackay, Frank Filz, 'Chuck Lever', 'Jeff Layton', neilb, okorniev, tom Cc: linux-nfs On 6/10/25 3:03 PM, Calum Mackay wrote: > On 10/06/2025 4:05 pm, Dai Ngo wrote: >> >> On 6/10/25 7:53 AM, Frank Filz wrote: >>> From: Chuck Lever [mailto:chuck.lever@oracle.com] >>>> On 6/10/25 10:12 AM, Dai Ngo wrote: >>>>> On 6/10/25 7:01 AM, Chuck Lever wrote: >>>>>> On 6/10/25 9:59 AM, Jeff Layton wrote: >>>>>>> On Tue, 2025-06-10 at 09:52 -0400, Chuck Lever wrote: >>>>>>>> On 6/10/25 9:50 AM, Jeff Layton wrote: >>>>>>>>> On Tue, 2025-06-10 at 06:41 -0700, Dai Ngo wrote: >>>>>>>>>> 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). >>>>> Since the Linux client does not behave this way I can not test if >>>>> this >>>>> error get us into a loop. >>>> Good point! >>>> >>>> >>>>> I used pynfs to force this behavior. >>>>> >>>>> However, here is the comment in nfs4_do_open: >>>>> >>>>> /* >>>>> * BAD_STATEID on OPEN means that the server >>>>> cancelled >>>>> our >>>>> * state before it received the OPEN_CONFIRM. >>>>> * Recover by retrying the request as per the >>>>> discussion >>>>> * on Page 181 of RFC3530. >>>>> */ >>>>> >>>>> So it guess BAD_STATEID will get the client and server into a loop. >>>>> I'll change error to NFS4ERR_INVAL and add a comment in the code. >>>> Thanks, we'll start there. If that's problematic, it can always be >>>> changed later. >>>> >>>> Maybe someone should file an errata against RFC 8881. <whistles >>>> tunelessly> >>> An interesting case. Ganesha doesn't handle this. It would >>> definitely be good to see an errata for it. Also a pynfs test case. >> >> Here is the pynfs test I used to test CLAIM_DELEG_CUR_FH: > > Thanks Dai; would you like to submit that as a pynfs patch, please? Yes, I will change the expected error code to NFS4ERR_INVAL, as we discussed, and submit the patch. -Dai > > ta, > c. > > >> >> def testClaimDeleg_CurFh(t, env): >> """Test OPEN with CLAIM_DELEG_CUR_FH with mismatch file file handle >> and delegation stateid. >> >> FLAGS: writedelegations deleg >> CODE: DELEG32 >> """ >> >> sess = env.c1.new_client_session(env.testname(t)) >> >> # create file-1 with read-only access (0444) no delegatation >> wanted, >> # and leave the file opened >> filename1 = b"file-1" >> res = open_create_file(sess, filename1, open_create = >> OPEN4_CREATE, attrs={FATTR4_MODE: 0o444}, >> access = OPEN4_SHARE_ACCESS_BOTH, want_deleg = False) >> check(res) >> deleg = res.resarray[-2].delegation >> if (_got_deleg(deleg)): >> fail("Not expect to get delegation") >> fh = res.resarray[-1].object >> stateid = res.resarray[-2].stateid >> print("----- CREATED ", filename1) >> >> # create file-2 with access RW and delegation wanted >> filename2 = b"file-2" >> res = open_create_file(sess, filename2, open_create = OPEN4_CREATE, >> access = OPEN4_SHARE_ACCESS_BOTH, want_deleg = True) >> check(res) >> print("----- CREATED ", filename2) >> >> wfh = res.resarray[-1].object >> wdeleg = res.resarray[-2].delegation >> if (not _got_deleg(wdeleg)): >> fail("Could not get WRITE delegation") >> wdelegstateid = wdeleg.write.stateid >> >> # OPEN for WRITE with CLAIM_DELEG_CUR_FH using the file handle >> # of filename1 and the delegation stateid granted for filename2. >> # Since the file handle and the delegation stateid do not belong >> # to the same file, expect server to return NFS4ERR_BAD_STATEID. >> >> claim = open_claim4(CLAIM_DELEG_CUR_FH, >> oc_delegate_stateid=wdelegstateid) >> owner = open_owner4(0, b"My Open Owner 2") >> how = openflag4(OPEN4_NOCREATE) >> open_op = op.open(0, OPEN4_SHARE_ACCESS_WRITE, >> OPEN4_SHARE_DENY_NONE, >> owner, how, claim) >> res = sess.compound([op.putfh(fh), open_op]) >> check(res, NFS4ERR_BAD_STATEID) >> >> # close file-1 >> res = close_file(sess, fh, stateid) >> check(res) >> >> # return the write delegation >> res = sess.compound([op.putfh(wfh), op.delegreturn(wdelegstateid)]) >> check(res) >> >>> >>> Thanks >>> >>> Frank >>> >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-11 0:35 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-10 13:41 [PATCH 1/1] NFSD: detect mismatch of file handle and delegation stateid in OPEN op Dai Ngo 2025-06-10 13:50 ` Chuck Lever 2025-06-10 13:55 ` Jeff Layton 2025-06-10 13:50 ` Jeff Layton 2025-06-10 13:52 ` Chuck Lever 2025-06-10 13:59 ` Jeff Layton 2025-06-10 14:01 ` Chuck Lever 2025-06-10 14:12 ` Dai Ngo 2025-06-10 14:14 ` Chuck Lever 2025-06-10 14:53 ` Frank Filz 2025-06-10 15:05 ` Dai Ngo 2025-06-10 22:03 ` Calum Mackay 2025-06-11 0:35 ` Dai Ngo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.