Re: [PATCH 1/2] fuse: set FR_PENDING under fiq->lock in fuse_chan_resend()

[Tang Yizhou <tangyeechou@gmail.com>]

On 4/8/26 5:17 pm, Jun Yang wrote:
> FR_PENDING means "queued on fiq->pending, protected by fiq->lock". It is
> the sole predicate fuse_remove_pending_req() uses to unlink a request and
> drop the queue's reference.
> 
> fuse_chan_resend() breaks that invariant. It splices every fpq->processing
> list onto a stack-local to_queue and drops fch->lock, then sets FR_PENDING
> on each request while holding no lock at all. From that moment the request
> advertises "I am on fiq->pending" while it is in fact reachable only
> through the caller's stack. A waiter whose wait is interrupted takes
> fiq->lock, sees FR_PENDING, unlinks the request from to_queue and drops its
> reference, and fuse_chan_send() then drops the last one -- so the request
> can be released while fuse_chan_resend() is still iterating over it.
> fiq->lock serialises nothing here, because the request is not on an
> fiq-protected list.
> 
> fuse_chan_resend() then walks that same list, and on the !fiq->connected
> path it drops fiq->lock and walks it with a non-safe list_for_each_entry().
> 
> Publish FR_PENDING under fiq->lock, immediately before the splice that
> actually puts the requests on fiq->pending, and fold the intr_entry cleanup
> into the same locked walk. A waiter that arrives while the requests are
> still on the stack now sees FR_PENDING clear, so fuse_remove_pending_req()
> returns false and it falls through to wait_event(FR_FINISHED) -- the same
> handling a request already handed to userspace gets. The !fiq->connected
> path no longer needs to clear the bit, because it was never set.

Hi,

I understand what you mean, because I found a similar issue during stability
testing.

I’m not sure whether others can understand such a lengthy textual description.
People usually prefer to see a sequence diagram to illustrate the issue.

> 
> Confirmed on v7.2-rc6 (075b74841bd0).
> 
> Fixes: 760eac73f9f6 ("fuse: Introduce a new notification type for resend pending requests")
> Cc: stable@kernel.org
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Assisted-by: tencentos-corvus-ai:kimi-k3
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> ---
> A KASAN reproducer for this issue is available if requested.
> 
>  fs/fuse/dev.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 5763a7cd3b37..e62c7ed8bcf4 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1781,26 +1781,22 @@ void fuse_chan_resend(struct fuse_chan *fch)
>  	}
>  	spin_unlock(&fch->lock);
>  
> -	list_for_each_entry_safe(req, next, &to_queue, list) {
> -		set_bit(FR_PENDING, &req->flags);
> -		clear_bit(FR_SENT, &req->flags);
> -		/* mark the request as resend request */
> -		req->in.h.unique |= FUSE_UNIQUE_RESEND;
> -	}
> -
>  	spin_lock(&fiq->lock);
>  	if (!fiq->connected) {
>  		spin_unlock(&fiq->lock);
> -		list_for_each_entry(req, &to_queue, list)
> -			clear_bit(FR_PENDING, &req->flags);
>  		fuse_dev_end_requests(&to_queue);
>  		return;
>  	}
> -	/*
> -	 * Remove interrupt entries for resent requests to prevent stale
> -	 * intr_entry on fiq->interrupts after the request is re-queued.
> -	 */
> -	list_for_each_entry(req, &to_queue, list) {
> +	list_for_each_entry_safe(req, next, &to_queue, list) {
> +		/* must be set under fiq->lock, see fuse_remove_pending_req() */
> +		set_bit(FR_PENDING, &req->flags);
> +		clear_bit(FR_SENT, &req->flags);
> +		/* mark the request as resend request */
> +		req->in.h.unique |= FUSE_UNIQUE_RESEND;
> +		/*
> +		 * Remove interrupt entries for resent requests to prevent stale
> +		 * intr_entry on fiq->interrupts after the request is re-queued.
> +		 */
>  		if (test_bit(FR_INTERRUPTED, &req->flags))
>  			list_del_init(&req->intr_entry);
>  	}

The solution looks good to me.

However, I hope you can truly understand the root cause of the issue and
describe it concisely, rather than simply pasting the AI's output, which would
actually make it harder for everyone to understand.

-- 
Best Regards,
Yi