* [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
@ 2024-03-18 16:32 Jan Kara
2024-03-18 16:50 ` Jeff Layton
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jan Kara @ 2024-03-18 16:32 UTC (permalink / raw)
To: Chuck Lever; +Cc: Christian Brauner, Al Viro, linux-nfs, Jeff Layton, Jan Kara
Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
having no common ancestor") added an error bail out path. However this
path does not drop the remount protection that has been acquired. Fix
the cleanup path to properly drop the remount protection.
Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/nfsd/vfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6a9464262fae..2e41eb4c3cec 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1852,7 +1852,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
trap = lock_rename(tdentry, fdentry);
if (IS_ERR(trap)) {
err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
- goto out;
+ goto out_want_write;
}
err = fh_fill_pre_attrs(ffhp);
if (err != nfs_ok)
@@ -1922,6 +1922,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
}
out_unlock:
unlock_rename(tdentry, fdentry);
+out_want_write:
fh_drop_write(ffhp);
/*
--
2.35.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
2024-03-18 16:32 [PATCH] nfsd: Fix error cleanup path in nfsd_rename() Jan Kara
@ 2024-03-18 16:50 ` Jeff Layton
2024-03-18 20:12 ` Chuck Lever
2024-03-22 2:37 ` Al Viro
2 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2024-03-18 16:50 UTC (permalink / raw)
To: Jan Kara, Chuck Lever; +Cc: Christian Brauner, Al Viro, linux-nfs
On Mon, 2024-03-18 at 17:32 +0100, Jan Kara wrote:
> Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
> having no common ancestor") added an error bail out path. However this
> path does not drop the remount protection that has been acquired. Fix
> the cleanup path to properly drop the remount protection.
>
> Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/nfsd/vfs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6a9464262fae..2e41eb4c3cec 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1852,7 +1852,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> trap = lock_rename(tdentry, fdentry);
> if (IS_ERR(trap)) {
> err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
> - goto out;
> + goto out_want_write;
> }
> err = fh_fill_pre_attrs(ffhp);
> if (err != nfs_ok)
> @@ -1922,6 +1922,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> }
> out_unlock:
> unlock_rename(tdentry, fdentry);
> +out_want_write:
> fh_drop_write(ffhp);
>
> /*
Nice catch.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
2024-03-18 16:32 [PATCH] nfsd: Fix error cleanup path in nfsd_rename() Jan Kara
2024-03-18 16:50 ` Jeff Layton
@ 2024-03-18 20:12 ` Chuck Lever
2024-03-19 13:53 ` Jan Kara
2024-03-22 2:37 ` Al Viro
2 siblings, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2024-03-18 20:12 UTC (permalink / raw)
To: Jan Kara; +Cc: Christian Brauner, Al Viro, linux-nfs, Jeff Layton
On Mon, Mar 18, 2024 at 05:32:09PM +0100, Jan Kara wrote:
> Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
> having no common ancestor") added an error bail out path. However this
> path does not drop the remount protection that has been acquired. Fix
> the cleanup path to properly drop the remount protection.
>
> Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
> Signed-off-by: Jan Kara <jack@suse.cz>
Al, Jan, let me know if you'd like me to take this through the
nfsd tree for v6.9-rc. If not:
Acked-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> fs/nfsd/vfs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 6a9464262fae..2e41eb4c3cec 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1852,7 +1852,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> trap = lock_rename(tdentry, fdentry);
> if (IS_ERR(trap)) {
> err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
> - goto out;
> + goto out_want_write;
> }
> err = fh_fill_pre_attrs(ffhp);
> if (err != nfs_ok)
> @@ -1922,6 +1922,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> }
> out_unlock:
> unlock_rename(tdentry, fdentry);
> +out_want_write:
> fh_drop_write(ffhp);
>
> /*
> --
> 2.35.3
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
2024-03-18 20:12 ` Chuck Lever
@ 2024-03-19 13:53 ` Jan Kara
2024-03-19 15:18 ` Chuck Lever
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2024-03-19 13:53 UTC (permalink / raw)
To: Chuck Lever; +Cc: Jan Kara, Christian Brauner, Al Viro, linux-nfs, Jeff Layton
On Mon 18-03-24 16:12:17, Chuck Lever wrote:
> On Mon, Mar 18, 2024 at 05:32:09PM +0100, Jan Kara wrote:
> > Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
> > having no common ancestor") added an error bail out path. However this
> > path does not drop the remount protection that has been acquired. Fix
> > the cleanup path to properly drop the remount protection.
> >
> > Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
> > Signed-off-by: Jan Kara <jack@suse.cz>
>
> Al, Jan, let me know if you'd like me to take this through the
> nfsd tree for v6.9-rc. If not:
>
> Acked-by: Chuck Lever <chuck.lever@oracle.com>
Yeah, I guess taking this through NFS tree is the best.
Honza
>
>
> > ---
> > fs/nfsd/vfs.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 6a9464262fae..2e41eb4c3cec 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -1852,7 +1852,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> > trap = lock_rename(tdentry, fdentry);
> > if (IS_ERR(trap)) {
> > err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
> > - goto out;
> > + goto out_want_write;
> > }
> > err = fh_fill_pre_attrs(ffhp);
> > if (err != nfs_ok)
> > @@ -1922,6 +1922,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> > }
> > out_unlock:
> > unlock_rename(tdentry, fdentry);
> > +out_want_write:
> > fh_drop_write(ffhp);
> >
> > /*
> > --
> > 2.35.3
> >
>
> --
> Chuck Lever
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
2024-03-19 13:53 ` Jan Kara
@ 2024-03-19 15:18 ` Chuck Lever
0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2024-03-19 15:18 UTC (permalink / raw)
To: Jan Kara; +Cc: Christian Brauner, Al Viro, linux-nfs, Jeff Layton
On Tue, Mar 19, 2024 at 02:53:54PM +0100, Jan Kara wrote:
> On Mon 18-03-24 16:12:17, Chuck Lever wrote:
> > On Mon, Mar 18, 2024 at 05:32:09PM +0100, Jan Kara wrote:
> > > Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
> > > having no common ancestor") added an error bail out path. However this
> > > path does not drop the remount protection that has been acquired. Fix
> > > the cleanup path to properly drop the remount protection.
> > >
> > > Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> >
> > Al, Jan, let me know if you'd like me to take this through the
> > nfsd tree for v6.9-rc. If not:
> >
> > Acked-by: Chuck Lever <chuck.lever@oracle.com>
>
> Yeah, I guess taking this through NFS tree is the best.
I've pushed this to my nfsd-fixes branch, and will begin testing.
> Honza
>
> >
> >
> > > ---
> > > fs/nfsd/vfs.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > index 6a9464262fae..2e41eb4c3cec 100644
> > > --- a/fs/nfsd/vfs.c
> > > +++ b/fs/nfsd/vfs.c
> > > @@ -1852,7 +1852,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> > > trap = lock_rename(tdentry, fdentry);
> > > if (IS_ERR(trap)) {
> > > err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
> > > - goto out;
> > > + goto out_want_write;
> > > }
> > > err = fh_fill_pre_attrs(ffhp);
> > > if (err != nfs_ok)
> > > @@ -1922,6 +1922,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> > > }
> > > out_unlock:
> > > unlock_rename(tdentry, fdentry);
> > > +out_want_write:
> > > fh_drop_write(ffhp);
> > >
> > > /*
> > > --
> > > 2.35.3
> > >
> >
> > --
> > Chuck Lever
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
2024-03-18 16:32 [PATCH] nfsd: Fix error cleanup path in nfsd_rename() Jan Kara
2024-03-18 16:50 ` Jeff Layton
2024-03-18 20:12 ` Chuck Lever
@ 2024-03-22 2:37 ` Al Viro
2 siblings, 0 replies; 6+ messages in thread
From: Al Viro @ 2024-03-22 2:37 UTC (permalink / raw)
To: Jan Kara; +Cc: Chuck Lever, Christian Brauner, linux-nfs, Jeff Layton
On Mon, Mar 18, 2024 at 05:32:09PM +0100, Jan Kara wrote:
> Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
> having no common ancestor") added an error bail out path. However this
> path does not drop the remount protection that has been acquired. Fix
> the cleanup path to properly drop the remount protection.
>
> Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
> Signed-off-by: Jan Kara <jack@suse.cz>
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
Sorry, my fault.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-22 2:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-18 16:32 [PATCH] nfsd: Fix error cleanup path in nfsd_rename() Jan Kara
2024-03-18 16:50 ` Jeff Layton
2024-03-18 20:12 ` Chuck Lever
2024-03-19 13:53 ` Jan Kara
2024-03-19 15:18 ` Chuck Lever
2024-03-22 2:37 ` Al Viro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox