Re: [PATCH 2/2] fuse: don't queue an interrupt for a request that is back on fiq->pending
Tang Yizhou <[email protected]>
| Newsgroups | dev.linux.lists.fuse-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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: [email protected]
> Reported-by: TencentOS Corvus AI <[email protected]>
> Assisted-by: tencentos-corvus-ai:kimi-k3
> Signed-off-by: Jun Yang <[email protected]>
> ---
> 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);
> /*