[PATCH 0/2] fuse: fix request lifetime races in the resend path

[PATCH 0/2] fuse: fix request lifetime races in the resend path

[Jun Yang]

Two fixes for races introduced together with FUSE_NOTIFY_RESEND
(760eac73f9f6, v6.9), where fuse_chan_resend() moves in-flight requests
from fpq->processing back onto fiq->pending.

Both concern the same invariant. FR_PENDING means "queued on fiq->pending,
protected by fiq->lock", and it is what fuse_remove_pending_req() relies on
to unlink a request and drop the queue's reference. A request on
fiq->pending can therefore be released without going through
fuse_request_end(), so anything that sets FR_PENDING, or that leaves a
request linked elsewhere while FR_PENDING is set, has to be done under
fiq->lock.

Patch 1 sets FR_PENDING under fiq->lock. fuse_chan_resend() currently
publishes the bit while the requests are reachable only through a
stack-local list, so a concurrent waiter can unlink and release a request
that fuse_chan_resend() is still iterating over.

Patch 2 re-checks FR_SENT under fiq->lock in fuse_dev_queue_interrupt().
Its callers sample FR_SENT unlocked and fuse_chan_resend() clears it under
fiq->lock, so a request can end up queued on fiq->pending and linked on
fiq->interrupts at the same time.

Dependency between the two patches
==================================

They are independent and neither supersedes the other, so please apply them
together rather than picking one. Patch 2 does not affect the unlocked
FR_PENDING publish that patch 1 fixes. Patch 1 cannot catch an intr_entry
that is linked after its locked walk has already run, because that link
happens once fuse_chan_resend() has released fiq->lock. Patch 1 on its own
also makes the condition patch 2 fixes easier to hit, since requests that
would previously have been torn out of the resend list now survive to be
re-queued.

Both were found by code audit and confirmed on v7.2-rc6 (075b74841bd0),
where the series was built and tested; the resend and interrupt paths were
verified to still be exercised with the series applied.

A KASAN reproducer for this issue is available if requested.

Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Signed-off-by: Jun Yang <junvyyang@tencent.com>

Jun Yang (2):
  fuse: set FR_PENDING under fiq->lock in fuse_chan_resend()
  fuse: don't queue an interrupt for a request that is back on
    fiq->pending

 fs/fuse/dev.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

-- 
2.43.7

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

[Jun Yang]

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.

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);
 	}
-- 
2.43.7

[PATCH 2/2] fuse: don't queue an interrupt for a request that is back on fiq->pending

[Jun Yang]

fuse_dev_queue_interrupt() links a request onto fiq->interrupts based on
an FR_SENT observation its callers make without fiq->lock: in
request_wait_answer(), in fuse_dev_do_read() after setting FR_SENT, and in
fuse_dev_do_write() on an interrupt reply with -EAGAIN.

fuse_chan_resend() invalidates that observation: under fiq->lock it clears
FR_SENT, sets FR_PENDING and splices the request back onto fiq->pending. A
caller that sampled FR_SENT just before that happens links the request onto
fiq->interrupts just after, so the request ends up queued on fiq->pending
*and* on fiq->interrupts.

That combination is a problem, because a request on fiq->pending can be
released without ever going through fuse_request_end(). A waiter whose wait
is interrupted calls fuse_remove_pending_req(), which sees FR_PENDING,
unlinks the request from fiq->pending and drops the queue's reference;
fuse_chan_send() then drops the last one. Unlike fuse_request_end(), that
path has no FR_INTERRUPTED cleanup, so the request can be released while
still linked on fiq->interrupts, and the next fuse_dev_do_read() walks it
in fuse_read_interrupt().

Re-check FR_SENT in fuse_dev_queue_interrupt() under fiq->lock, which is
the lock fuse_chan_resend() holds when it clears it. This restores the
invariant "FR_PENDING set => intr_entry not linked", both sides of it now
being taken under fiq->lock. No interrupt is lost: the request is going
back to the daemon, and fuse_dev_do_read() re-queues the interrupt once it
has set FR_SENT again.

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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index e62c7ed8bcf4..c4df1d4abd33 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -240,6 +240,11 @@ void fuse_dev_queue_forget(struct fuse_iqueue *fiq,
 void fuse_dev_queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req)
 {
 	spin_lock(&fiq->lock);
+	/* fuse_chan_resend() may have put the request back on fiq->pending */
+	if (!test_bit(FR_SENT, &req->flags)) {
+		spin_unlock(&fiq->lock);
+		return;
+	}
 	if (list_empty(&req->intr_entry)) {
 		list_add_tail(&req->intr_entry, &fiq->interrupts);
 		/*
-- 
2.43.7

Re: [PATCH 0/2] fuse: fix request lifetime races in the resend path

[Tang Yizhou]

On 4/8/26 5:17 pm, Jun Yang wrote:
> Two fixes for races introduced together with FUSE_NOTIFY_RESEND
> (760eac73f9f6, v6.9), where fuse_chan_resend() moves in-flight requests
> from fpq->processing back onto fiq->pending.
> 
> Both concern the same invariant. FR_PENDING means "queued on fiq->pending,
> protected by fiq->lock", and it is what fuse_remove_pending_req() relies on

Hi,

Currently FR_PENDING doesn't mean the request is protected by fiq->lock. It
looks like this is your solution, so you need to clearly explain why you are
doing this.

> to unlink a request and drop the queue's reference. A request on

It is unrelated to the queue's reference. I think you need to check whether the
AI's output is correct first.

> fiq->pending can therefore be released without going through
> fuse_request_end(), so anything that sets FR_PENDING, or that leaves a
> request linked elsewhere while FR_PENDING is set, has to be done under
> fiq->lock.
> 
> Patch 1 sets FR_PENDING under fiq->lock. fuse_chan_resend() currently
> publishes the bit while the requests are reachable only through a
> stack-local list, so a concurrent waiter can unlink and release a request
> that fuse_chan_resend() is still iterating over.
> 
> Patch 2 re-checks FR_SENT under fiq->lock in fuse_dev_queue_interrupt().
> Its callers sample FR_SENT unlocked and fuse_chan_resend() clears it under
> fiq->lock, so a request can end up queued on fiq->pending and linked on
> fiq->interrupts at the same time.
> 
> Dependency between the two patches
> ==================================
> 
> They are independent and neither supersedes the other, so please apply them
> together rather than picking one. Patch 2 does not affect the unlocked
> FR_PENDING publish that patch 1 fixes. Patch 1 cannot catch an intr_entry
> that is linked after its locked walk has already run, because that link
> happens once fuse_chan_resend() has released fiq->lock. Patch 1 on its own
> also makes the condition patch 2 fixes easier to hit, since requests that
> would previously have been torn out of the resend list now survive to be
> re-queued.
> 
> Both were found by code audit and confirmed on v7.2-rc6 (075b74841bd0),
> where the series was built and tested; the resend and interrupt paths were
> verified to still be exercised with the series applied.
> 
> A KASAN reproducer for this issue is available if requested.

Since you have a KASAN reproducer, please first describe how to trigger the
issue and what the symptoms are, as a prelude to the solution.

-- 
Best Regards,
Yi

> 
> Reported-by: TencentOS Corvus AI <corvus@tencent.com>
> Signed-off-by: Jun Yang <junvyyang@tencent.com>
> 
> Jun Yang (2):
>   fuse: set FR_PENDING under fiq->lock in fuse_chan_resend()
>   fuse: don't queue an interrupt for a request that is back on
>     fiq->pending
> 
>  fs/fuse/dev.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 

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

[Tang Yizhou]

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

Re: [PATCH 2/2] fuse: don't queue an interrupt for a request that is back on fiq->pending

[Tang Yizhou]

On 4/8/26 5:17 pm, Jun Yang wrote:
> fuse_dev_queue_interrupt() links a request onto fiq->interrupts based on
> an FR_SENT observation its callers make without fiq->lock: in
> request_wait_answer(), in fuse_dev_do_read() after setting FR_SENT, and in
> fuse_dev_do_write() on an interrupt reply with -EAGAIN.
> 
> fuse_chan_resend() invalidates that observation: under fiq->lock it clears
> FR_SENT, sets FR_PENDING and splices the request back onto fiq->pending. A

Right. So why did you say 'They (patch 1 and 2) are independent' in the coverletter?

-- 
Best Regards,
Yi

> caller that sampled FR_SENT just before that happens links the request onto
> fiq->interrupts just after, so the request ends up queued on fiq->pending
> *and* on fiq->interrupts.
> 
> That combination is a problem, because a request on fiq->pending can be
> released without ever going through fuse_request_end(). A waiter whose wait
> is interrupted calls fuse_remove_pending_req(), which sees FR_PENDING,
> unlinks the request from fiq->pending and drops the queue's reference;
> fuse_chan_send() then drops the last one. Unlike fuse_request_end(), that
> path has no FR_INTERRUPTED cleanup, so the request can be released while
> still linked on fiq->interrupts, and the next fuse_dev_do_read() walks it
> in fuse_read_interrupt().
> 
> Re-check FR_SENT in fuse_dev_queue_interrupt() under fiq->lock, which is
> the lock fuse_chan_resend() holds when it clears it. This restores the
> invariant "FR_PENDING set => intr_entry not linked", both sides of it now
> being taken under fiq->lock. No interrupt is lost: the request is going
> back to the daemon, and fuse_dev_do_read() re-queues the interrupt once it
> has set FR_SENT again.
> 
> 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 | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index e62c7ed8bcf4..c4df1d4abd33 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -240,6 +240,11 @@ void fuse_dev_queue_forget(struct fuse_iqueue *fiq,
>  void fuse_dev_queue_interrupt(struct fuse_iqueue *fiq, struct fuse_req *req)
>  {
>  	spin_lock(&fiq->lock);
> +	/* fuse_chan_resend() may have put the request back on fiq->pending */
> +	if (!test_bit(FR_SENT, &req->flags)) {
> +		spin_unlock(&fiq->lock);
> +		return;
> +	}
>  	if (list_empty(&req->intr_entry)) {
>  		list_add_tail(&req->intr_entry, &fiq->interrupts);
>  		/*