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@xxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx>
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
