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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.