Re: [PATCH 2/2] fuse: wait for aborted connection before releasing last fuse_dev
Bernd Schubert <[email protected]>
| Newsgroups | org.kernel.vger.io-uring,dev.linux.lists.fuse-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 5/17/26 14:59, Berkant Koc wrote: > [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > From: Berkant Koc <[email protected]> > > fuse_dev_release() on the last fuse_dev of a connection calls > fuse_abort_conn(fc) and then immediately fuse_conn_put(fc). For io-uring > backed connections fuse_abort_conn() reaches fuse_uring_abort(), which > runs fuse_uring_teardown_all_queues() synchronously once and then > schedules ring->async_teardown_work to run after > FUSE_URING_TEARDOWN_INTERVAL (HZ/20). If the synchronous pass left > queue_refs > 0 the work owns further accesses to ring->queues[*]-> > ent_avail_queue and ent_in_userspace entries. > > Meanwhile fuse_conn_put() can drop the last reference and arm > delayed_release() via call_rcu(). After the RCU grace period > delayed_release() calls fuse_uring_destruct(), which kfree()s the ring > entries on each queue->ent_released list. The previously scheduled > async_teardown_work then runs and walks per-queue lists that contain > freed entries, producing a slab-use-after-free reported by KASAN at > fuse_uring_teardown_all_queues+0xee reading ent->list.next from a > freed kmalloc-192 region. > > fuse_wait_aborted() already exists for this purpose: it waits on > fc->blocked_waitq for num_waiting to drain and then calls > fuse_uring_wait_stopped_queues(), which waits for ring->queue_refs to > reach zero. Call it between fuse_abort_conn() and fuse_conn_put() on > the last-device path so the io-uring teardown work has fully drained > before the connection can be torn down. > > Fixes: c090c8abae4b ("fuse: Add io-uring sqe commit and fetch support") > Cc: [email protected] # 6.14+ > Tested-by: Berkant Koc <[email protected]> > Signed-off-by: Berkant Koc <[email protected]> > --- > fs/fuse/dev.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 5dda7080f4a9..7d9c06654a98 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -2566,6 +2566,7 @@ int fuse_dev_release(struct inode *inode, struct file *file) > if (last) { > WARN_ON(fc->iq.fasync != NULL); > fuse_abort_conn(fc); > + fuse_wait_aborted(fc); > } > fuse_conn_put(fc); > } I might be wrong, but I don't think it is possible, Maybe Pavel or Jens could help (added to CC). Basically as long as fuse_uring_async_stop_queues() runs we do not have completed all io-uring commands via io_uring_cmd_done() and as long as we do not have completed these io-uring commands. References are taken here io_issue_sqe io_assign_file io_file_get_normal fget __io_uring_cmd_done() req->io_task_work.func = io_req_task_complete io_req_task_work_add(req) io_uring/uring_cmd.c:179 io_req_task_complete io_req_complete_defer wq_list_add_tail __io_submit_flush_completions io_free_batch_list io_put_file fput I also look it up for fixed file and for fixed file it needs io_ring_ctx_free(), which is then only one after completing all uring_cmd objects (io_uring_cmd_done). I don't mind adding fuse_wait_aborted() here, but IMHO it is a no-op and no security issue. Thanks Bernd