Re: [PATCH 2/2] fuse: wait for aborted connection before releasing last fuse_dev

[Bernd Schubert <bschubert@ddn.com>]

On 5/17/26 14:59, Berkant Koc wrote:
> [You don't often get email from me@berkoc.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> From: Berkant Koc <me@berkoc.com>
> 
> 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: stable@vger.kernel.org # 6.14+
> Tested-by: Berkant Koc <me@berkoc.com>
> Signed-off-by: Berkant Koc <me@berkoc.com>
> ---
>  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