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/18/26 11:06, Pavel Begunkov wrote: > On 5/17/26 16:00, Bernd Schubert wrote: >> 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. > > If I understand the question right, yes, fuse io_uring cmd requests hold > a reference to the fuse file, so until you complete them the file will > not get released. Sorry, had totally messed up the phrase, can't read it myself. What I mean is that the io-uring was set up with /dev/fuse as file and as long as fuse holds non-completed 'struct io_uring_cmd *cmd' objects there is a reference on the /dev/fuse fd, which blocks the call of fuse_dev_release(). Thanks, Bernd