| Message ID | 20250306111218.13734-1-luis@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fuse: fix possible deadlock if rings are never initialized | expand |
On 3/6/25 12:12, Luis Henriques wrote: > When mounting a user-space filesystem using io_uring, the initialization > of the rings is done separately in the server side. If for some reason > (e.g. a server bug) this step is not performed it will be impossible to > unmount the filesystem if there are already requests waiting. > > This issue is easily reproduced with the libfuse passthrough_ll example, > if the queue depth is set to '0' and a request is queued before trying to > unmount the filesystem. When trying to force the unmount, fuse_abort_conn() > will try to wake up all tasks waiting in fc->blocked_waitq, but because the > rings were never initialized, fuse_uring_ready() will never return 'true'. > > Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") > Signed-off-by: Luis Henriques <luis@igalia.com> > --- > fs/fuse/dev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 7edceecedfa5..2fe565e9b403 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -77,7 +77,7 @@ void fuse_set_initialized(struct fuse_conn *fc) > static bool fuse_block_alloc(struct fuse_conn *fc, bool for_background) > { > return !fc->initialized || (for_background && fc->blocked) || > - (fc->io_uring && !fuse_uring_ready(fc)); > + (fc->io_uring && fc->connected && !fuse_uring_ready(fc)); > } > > static void fuse_drop_waiting(struct fuse_conn *fc) > Oh yes, I had missed that. Reviewed-by: Bernd Schubert <bschubert@ddn.com>
On Thu, Mar 06 2025, Bernd Schubert wrote: > On 3/6/25 12:12, Luis Henriques wrote: >> When mounting a user-space filesystem using io_uring, the initialization >> of the rings is done separately in the server side. If for some reason >> (e.g. a server bug) this step is not performed it will be impossible to >> unmount the filesystem if there are already requests waiting. >> >> This issue is easily reproduced with the libfuse passthrough_ll example, >> if the queue depth is set to '0' and a request is queued before trying to >> unmount the filesystem. When trying to force the unmount, fuse_abort_conn() >> will try to wake up all tasks waiting in fc->blocked_waitq, but because the >> rings were never initialized, fuse_uring_ready() will never return 'true'. >> >> Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") >> Signed-off-by: Luis Henriques <luis@igalia.com> >> --- >> fs/fuse/dev.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c >> index 7edceecedfa5..2fe565e9b403 100644 >> --- a/fs/fuse/dev.c >> +++ b/fs/fuse/dev.c >> @@ -77,7 +77,7 @@ void fuse_set_initialized(struct fuse_conn *fc) >> static bool fuse_block_alloc(struct fuse_conn *fc, bool for_background) >> { >> return !fc->initialized || (for_background && fc->blocked) || >> - (fc->io_uring && !fuse_uring_ready(fc)); >> + (fc->io_uring && fc->connected && !fuse_uring_ready(fc)); >> } >> >> static void fuse_drop_waiting(struct fuse_conn *fc) >> > > Oh yes, I had missed that. > > Reviewed-by: Bernd Schubert <bschubert@ddn.com> Thanks! And... by the way, Bernd: I know io_uring support in libfuse isn't ready yet, but I think there's some error handling missing in your uring branch. In particular, the return of fuse_uring_start() is never checked, and thus if the rings initialization fails, the server will not get any error. I found that out because I blindly tried the patch below, and I was surprised that the server was started just fine. Cheers,
On 3/6/25 14:16, Luis Henriques wrote: > On Thu, Mar 06 2025, Bernd Schubert wrote: > >> On 3/6/25 12:12, Luis Henriques wrote: >>> When mounting a user-space filesystem using io_uring, the initialization >>> of the rings is done separately in the server side. If for some reason >>> (e.g. a server bug) this step is not performed it will be impossible to >>> unmount the filesystem if there are already requests waiting. >>> >>> This issue is easily reproduced with the libfuse passthrough_ll example, >>> if the queue depth is set to '0' and a request is queued before trying to >>> unmount the filesystem. When trying to force the unmount, fuse_abort_conn() >>> will try to wake up all tasks waiting in fc->blocked_waitq, but because the >>> rings were never initialized, fuse_uring_ready() will never return 'true'. >>> >>> Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") >>> Signed-off-by: Luis Henriques <luis@igalia.com> >>> --- >>> fs/fuse/dev.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c >>> index 7edceecedfa5..2fe565e9b403 100644 >>> --- a/fs/fuse/dev.c >>> +++ b/fs/fuse/dev.c >>> @@ -77,7 +77,7 @@ void fuse_set_initialized(struct fuse_conn *fc) >>> static bool fuse_block_alloc(struct fuse_conn *fc, bool for_background) >>> { >>> return !fc->initialized || (for_background && fc->blocked) || >>> - (fc->io_uring && !fuse_uring_ready(fc)); >>> + (fc->io_uring && fc->connected && !fuse_uring_ready(fc)); >>> } >>> >>> static void fuse_drop_waiting(struct fuse_conn *fc) >>> >> >> Oh yes, I had missed that. >> >> Reviewed-by: Bernd Schubert <bschubert@ddn.com> > > Thanks! And... by the way, Bernd: > > I know io_uring support in libfuse isn't ready yet, but I think there's > some error handling missing in your uring branch. In particular, the > return of fuse_uring_start() is never checked, and thus if the rings > initialization fails, the server will not get any error. > > I found that out because I blindly tried the patch below, and I was > surprised that the server was started just fine. Thank you! I will work a bit on splitting the uring branch into merge-able patches later today, but probably won't finish today (too many other things to do). Thanks, Bernd
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 7edceecedfa5..2fe565e9b403 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -77,7 +77,7 @@ void fuse_set_initialized(struct fuse_conn *fc) static bool fuse_block_alloc(struct fuse_conn *fc, bool for_background) { return !fc->initialized || (for_background && fc->blocked) || - (fc->io_uring && !fuse_uring_ready(fc)); + (fc->io_uring && fc->connected && !fuse_uring_ready(fc)); } static void fuse_drop_waiting(struct fuse_conn *fc)
When mounting a user-space filesystem using io_uring, the initialization of the rings is done separately in the server side. If for some reason (e.g. a server bug) this step is not performed it will be impossible to unmount the filesystem if there are already requests waiting. This issue is easily reproduced with the libfuse passthrough_ll example, if the queue depth is set to '0' and a request is queued before trying to unmount the filesystem. When trying to force the unmount, fuse_abort_conn() will try to wake up all tasks waiting in fc->blocked_waitq, but because the rings were never initialized, fuse_uring_ready() will never return 'true'. Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") Signed-off-by: Luis Henriques <luis@igalia.com> --- fs/fuse/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)