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

Jun Yang posted 2 patches 1 month, 4 weeks ago
fs/fuse/dev.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
[PATCH 0/2] fuse: fix request lifetime races in the resend path
Posted by Jun Yang 1 month, 4 weeks ago
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
Re: [PATCH 0/2] fuse: fix request lifetime races in the resend path
Posted by Tang Yizhou 1 month, 2 weeks ago
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(-)
>