Re: [PATCH v1 3/3] fuse: fix moving cancelled entry to ent_in_userspace list

[Bernd Schubert <bernd@bsbernd.com>]

On 5/15/26 06:55, Joanne Koong wrote:
> fuse_uring_cancel() moves entries that are available (these have no reqs
> attached) to the ent_in_userspace list. ent_list_request_expired()
> checks the first entry on ent_in_userspace and dereferences
> ent->fuse_req unconditionally, which will crash on a cancelled entry
> that was moved to this list.
> 
> Fix this by freeing the entry and dropping queue_refs directly in
> fuse_uring_cancel(). This is safe because cancel is the cancel handler
> itself - after io_uring_cmd_done(), no more cancels will be dispatched
> for this command, and teardown serializes with cancel via queue->lock.
> 
> Since cancel now decrements queue_refs, fuse_uring_abort() must no
> longer gate fuse_uring_abort_end_requests() on queue_refs > 0, as
> cancelled entries may have already dropped queue_refs while requests are
> still queued. Remove the gate so abort always flushes requests and stops
> queues.
> 
> Reported-by: Heechan Kang <gganji11@naver.com>
> Fixes: 4fea593e625c ("fuse: optimize over-io-uring request expiration check")
> Cc: stable@vger.kernel.org
> Co-developed-by: Jian Huang Li <ali@ddn.com>
> Co-developed-by: Horst Birthelmer <horst@birthelmer.de>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> ---
>  fs/fuse/dev_uring.c   | 6 ++++--
>  fs/fuse/dev_uring_i.h | 6 +++---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index d9108b5b5db8..f4ba64a1796a 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -511,8 +511,7 @@ static void fuse_uring_cancel(struct io_uring_cmd *cmd,
>  	queue = ent->queue;
>  	spin_lock(&queue->lock);
>  	if (ent->state == FRRS_AVAILABLE) {
> -		ent->state = FRRS_USERSPACE;
> -		list_move_tail(&ent->list, &queue->ent_in_userspace);
> +		list_del_init(&ent->list);
>  		need_cmd_done = true;
>  		ent->cmd = NULL;
>  	}
> @@ -521,6 +520,9 @@ static void fuse_uring_cancel(struct io_uring_cmd *cmd,
>  	if (need_cmd_done) {
>  		/* no queue lock to avoid lock order issues */
>  		io_uring_cmd_done(cmd, -ENOTCONN, issue_flags);
> +		kfree(ent);
> +		if (atomic_dec_and_test(&queue->ring->queue_refs))
> +			wake_up_all(&queue->ring->stop_waitq);
>  	}
>  }

Hmm, ok, I had done that via fuse_uring_entry_teardown(), but this way
is also fine.

While thinking about it over night, I wonder if we should abort the
connection here. Calls for fuse_uring_cancel() / IO_URING_F_CANCEL
happen when

a) The daemon dies - that is what I had written the function for
b) When one calls

With reduced rings queues we would actually need to have per queue refs
and if a single queue reaches 0, it would need to re-calculate the
queue. In general this gets complex and from my point of view, if
fuse-server wants to re-initialize queues, fuse-server should:

a) wake up the ring thread with an eventfd (libfuse already has that)
b) we need a reconfig SQE (like FUSE_IO_URING_CMD_RECONFIG) that
requests to re-configure things

Right now that is all not supported, from my point of view we should
call fuse_abort_conn() when we call into fuse_uring_cancel()


Thanks,
Bernd

>  
> diff --git a/fs/fuse/dev_uring_i.h b/fs/fuse/dev_uring_i.h
> index 368f4d0790eb..22ec67e39ee0 100644
> --- a/fs/fuse/dev_uring_i.h
> +++ b/fs/fuse/dev_uring_i.h
> @@ -150,10 +150,10 @@ static inline void fuse_uring_abort(struct fuse_chan *fch)
>  	if (ring == NULL)
>  		return;
>  
> -	if (atomic_read(&ring->queue_refs) > 0) {
> -		fuse_uring_abort_end_requests(ring);
> +	fuse_uring_abort_end_requests(ring);
> +
> +	if (atomic_read(&ring->queue_refs) > 0)
>  		fuse_uring_stop_queues(ring);
> -	}
>  }
>  
>  static inline void fuse_uring_wait_stopped_queues(struct fuse_chan *fch)