| Message ID | 20230909-nfsd-fixes-v1-1-2ebc659c0cf4@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfsd: fix change_info in NFSv4 RENAME replies | expand |
On Sat, Sep 09, 2023 at 07:12:30AM -0400, Jeff Layton wrote: > nfsd sends the transposed directory change info in the RENAME reply. The > source directory is in save_fh and the target is in current_fh. > > Reported-by: Zhi Li <yieli@redhat.com> > Reported-by: Benjamin Coddington <bcodding@redhat.com> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > This bug predates git, so I can't add a proper Fixes tag. I think this > is probably appropriate for stable series kernels though. > > This bug was largely papered over by the fact that we factored in the > ctime when generating a change attribute. Since this commit, however: > > 638e3e7d9493 nfsd: use the getattr operation to fetch i_version > > We stopped doing that for directory inodes and that caused this bug to > pop up. Applied to nfsd-fixes with a "Cc: stable" tag added. Is there a publicly-accessible bug report link that can be included? > --- > fs/nfsd/nfs4proc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 5ca748309c26..4199ede0583c 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -1058,8 +1058,8 @@ nfsd4_rename(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > rename->rn_tname, rename->rn_tnamelen); > if (status) > return status; > - set_change_info(&rename->rn_sinfo, &cstate->current_fh); > - set_change_info(&rename->rn_tinfo, &cstate->save_fh); > + set_change_info(&rename->rn_sinfo, &cstate->save_fh); > + set_change_info(&rename->rn_tinfo, &cstate->current_fh); > return nfs_ok; > } > > > --- > base-commit: dd1386dd3c4f4bc55456c88180f9f39697bb95c0 > change-id: 20230908-nfsd-fixes-f5bdb87e6035 > > Best regards, > -- > Jeff Layton <jlayton@kernel.org> >
On Sat, 2023-09-09 at 11:18 -0400, Chuck Lever wrote: > On Sat, Sep 09, 2023 at 07:12:30AM -0400, Jeff Layton wrote: > > nfsd sends the transposed directory change info in the RENAME reply. The > > source directory is in save_fh and the target is in current_fh. > > > > Reported-by: Zhi Li <yieli@redhat.com> > > Reported-by: Benjamin Coddington <bcodding@redhat.com> > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > This bug predates git, so I can't add a proper Fixes tag. I think this > > is probably appropriate for stable series kernels though. > > > > This bug was largely papered over by the fact that we factored in the > > ctime when generating a change attribute. Since this commit, however: > > > > 638e3e7d9493 nfsd: use the getattr operation to fetch i_version > > > > We stopped doing that for directory inodes and that caused this bug to > > pop up. > > Applied to nfsd-fixes with a "Cc: stable" tag added. > > Is there a publicly-accessible bug report link that can be included? > > Yes. I just made this one public: https://bugzilla.redhat.com/show_bug.cgi?id=2218844 It has Ben's reproducer script for this in it too. > > --- > > fs/nfsd/nfs4proc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > > index 5ca748309c26..4199ede0583c 100644 > > --- a/fs/nfsd/nfs4proc.c > > +++ b/fs/nfsd/nfs4proc.c > > @@ -1058,8 +1058,8 @@ nfsd4_rename(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, > > rename->rn_tname, rename->rn_tnamelen); > > if (status) > > return status; > > - set_change_info(&rename->rn_sinfo, &cstate->current_fh); > > - set_change_info(&rename->rn_tinfo, &cstate->save_fh); > > + set_change_info(&rename->rn_sinfo, &cstate->save_fh); > > + set_change_info(&rename->rn_tinfo, &cstate->current_fh); > > return nfs_ok; > > } > > > > > > --- > > base-commit: dd1386dd3c4f4bc55456c88180f9f39697bb95c0 > > change-id: 20230908-nfsd-fixes-f5bdb87e6035 > > > > Best regards, > > -- > > Jeff Layton <jlayton@kernel.org> > > >
> On Sep 9, 2023, at 1:23 PM, Jeff Layton <jlayton@kernel.org> wrote: > > On Sat, 2023-09-09 at 11:18 -0400, Chuck Lever wrote: >> On Sat, Sep 09, 2023 at 07:12:30AM -0400, Jeff Layton wrote: >>> nfsd sends the transposed directory change info in the RENAME reply. The >>> source directory is in save_fh and the target is in current_fh. >>> >>> Reported-by: Zhi Li <yieli@redhat.com> >>> Reported-by: Benjamin Coddington <bcodding@redhat.com> >>> Signed-off-by: Jeff Layton <jlayton@kernel.org> >>> --- >>> This bug predates git, so I can't add a proper Fixes tag. I think this >>> is probably appropriate for stable series kernels though. >>> >>> This bug was largely papered over by the fact that we factored in the >>> ctime when generating a change attribute. Since this commit, however: >>> >>> 638e3e7d9493 nfsd: use the getattr operation to fetch i_version >>> >>> We stopped doing that for directory inodes and that caused this bug to >>> pop up. >> >> Applied to nfsd-fixes with a "Cc: stable" tag added. >> >> Is there a publicly-accessible bug report link that can be included? >> >> > > Yes. I just made this one public: > > https://bugzilla.redhat.com/show_bug.cgi?id=2218844 > > It has Ben's reproducer script for this in it too. Thanks, added in my private repo, to be pushed out later. >>> --- >>> fs/nfsd/nfs4proc.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c >>> index 5ca748309c26..4199ede0583c 100644 >>> --- a/fs/nfsd/nfs4proc.c >>> +++ b/fs/nfsd/nfs4proc.c >>> @@ -1058,8 +1058,8 @@ nfsd4_rename(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, >>> rename->rn_tname, rename->rn_tnamelen); >>> if (status) >>> return status; >>> - set_change_info(&rename->rn_sinfo, &cstate->current_fh); >>> - set_change_info(&rename->rn_tinfo, &cstate->save_fh); >>> + set_change_info(&rename->rn_sinfo, &cstate->save_fh); >>> + set_change_info(&rename->rn_tinfo, &cstate->current_fh); >>> return nfs_ok; >>> } >>> >>> >>> --- >>> base-commit: dd1386dd3c4f4bc55456c88180f9f39697bb95c0 >>> change-id: 20230908-nfsd-fixes-f5bdb87e6035 >>> >>> Best regards, >>> -- >>> Jeff Layton <jlayton@kernel.org> >>> >> > > -- > Jeff Layton <jlayton@kernel.org> -- Chuck Lever
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 5ca748309c26..4199ede0583c 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1058,8 +1058,8 @@ nfsd4_rename(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, rename->rn_tname, rename->rn_tnamelen); if (status) return status; - set_change_info(&rename->rn_sinfo, &cstate->current_fh); - set_change_info(&rename->rn_tinfo, &cstate->save_fh); + set_change_info(&rename->rn_sinfo, &cstate->save_fh); + set_change_info(&rename->rn_tinfo, &cstate->current_fh); return nfs_ok; }
nfsd sends the transposed directory change info in the RENAME reply. The source directory is in save_fh and the target is in current_fh. Reported-by: Zhi Li <yieli@redhat.com> Reported-by: Benjamin Coddington <bcodding@redhat.com> Signed-off-by: Jeff Layton <jlayton@kernel.org> --- This bug predates git, so I can't add a proper Fixes tag. I think this is probably appropriate for stable series kernels though. This bug was largely papered over by the fact that we factored in the ctime when generating a change attribute. Since this commit, however: 638e3e7d9493 nfsd: use the getattr operation to fetch i_version We stopped doing that for directory inodes and that caused this bug to pop up. --- fs/nfsd/nfs4proc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: dd1386dd3c4f4bc55456c88180f9f39697bb95c0 change-id: 20230908-nfsd-fixes-f5bdb87e6035 Best regards,