[PATCH] fuse: clear stale intr_entry in fuse_remove_pending_req()

[Baokun Li <libaokun@linux.alibaba.com>]

Commit f8fce75fedf7 ("fuse: clear intr_entry in fuse_resend and
fuse_remove_pending_req") removes stale interrupt entries in
fuse_chan_resend() when requests are moved back to fiq->pending.
However, that cleanup only covers interrupt entries that are already
linked at scan time.  It can race with a concurrent queue_interrupt()
from the request holder:

  CPU 0 (holder thread)           CPU 1 (resend)
  ---------------------           --------------

  req in processing (FR_SENT=1)

  signal arrives
  set_bit(FR_INTERRUPTED)
  test_bit(FR_SENT) -> true
  queue_interrupt():
    spins on fiq->lock ...
                                  fuse_chan_resend():
                                    set_bit(FR_PENDING)
                                    clear_bit(FR_SENT)
                                    spin_lock(&fiq->lock)
                                    cleanup scan:
                                      intr_entry not linked yet
                                      -> list_del_init is a no-op
                                    list_splice -> fiq->pending
                                    spin_unlock(&fiq->lock)
    ... acquires fiq->lock
    list_empty(&req->intr_entry) -> true
    FR_FINISHED not set
    -> intr_entry added to fiq->interrupts
       AFTER the cleanup already ran

  fatal signal arrives
  fuse_remove_pending_req():
    test_bit(FR_PENDING) -> true
    list_del(&req->list)
    __fuse_put_request
  fuse_put_request (refcount -> 0)
  -> req freed, intr_entry dangling
     on fiq->interrupts

fuse_dev_queue_interrupt() only checks list_empty() and FR_FINISHED
before linking intr_entry -- it does not check FR_PENDING, so a
request already spliced back to fiq->pending can still be added to
fiq->interrupts.  The lock contention itself produces the bad
ordering: while the resend holds fiq->lock to scan the queued
requests, the holder spins in queue_interrupt() and links intr_entry
right after the scan finishes.

The dangling entry then causes the same use-after-free that the
above commit describes: fuse_read_interrupt() writes to the freed
slab object via list_del_init() and leaks req->in.h.unique to
userspace.  Once the freed memory is reused, INIT_LIST_HEAD() turns
the entry into a self-loop and list_empty(&fiq->interrupts) returns
false forever, so the daemon reads the same phantom FUSE_INTERRUPT
in an infinite loop and never consumes fiq->pending.

Close the race in fuse_remove_pending_req(), which is the common
bail-out path for both the legacy and the io_uring transport: after
the request is removed from the pending queue, also unlink intr_entry
under fiq->lock before the reference is dropped.  fiq->lock must be
taken explicitly since the lock argument is the ring queue lock in
the io_uring case, while fiq->interrupts is always protected by
fiq->lock.  This runs on the holder thread after any
queue_interrupt() it issued, and no other path can re-link the entry
once the request is off the queues (re-queueing an interrupt from
FUSE_INTERRUPT's -EAGAIN reply requires finding the request in the
processing queue first).

Fixes: 760eac73f9f6 ("fuse: Introduce a new notification type for resend pending requests")
Cc: stable@vger.kernel.org # 6.9
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 fs/fuse/dev.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 5763a7cd3b37..87891162985c 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -678,6 +678,8 @@ static int queue_interrupt(struct fuse_req *req)
 
 bool fuse_remove_pending_req(struct fuse_req *req, spinlock_t *lock)
 {
+	struct fuse_iqueue *fiq = &req->chan->iq;
+
 	spin_lock(lock);
 	if (test_bit(FR_PENDING, &req->flags)) {
 		/*
@@ -686,6 +688,18 @@ bool fuse_remove_pending_req(struct fuse_req *req, spinlock_t *lock)
 		 */
 		list_del(&req->list);
 		spin_unlock(lock);
+
+		/*
+		 * Remove stale intr_entry queued by queue_interrupt() before
+		 * the request was requeued, which would otherwise dangle on
+		 * fiq->interrupts once the request is freed.
+		 */
+		if (test_bit(FR_INTERRUPTED, &req->flags)) {
+			spin_lock(&fiq->lock);
+			list_del_init(&req->intr_entry);
+			spin_unlock(&fiq->lock);
+		}
+
 		__fuse_put_request(req);
 		req->out.h.error = -EINTR;
 		return true;
-- 
2.43.7