[PATCH] fuse: fix use-after-free in request resend

Jia Zhu posted 1 patch 15 hours ago
fs/fuse/dev.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
[PATCH] fuse: fix use-after-free in request resend
Posted by Jia Zhu 15 hours ago
fuse_chan_resend() moves requests from fpq->processing to a private
to_queue before putting them back on fiq->pending.

The old ordering made FR_PENDING visible before the request was on a
list protected by fiq->lock:

  CPU0: fuse_chan_resend()      CPU1: request_wait_answer()
  ------------------------      ---------------------------
  move req to to_queue
  set FR_PENDING
                                fatal signal
                                fuse_remove_pending_req()
                                  lock fiq->lock
                                  list_del(req->list)
                                  __fuse_put_request(req)
                                    drop queue reference
                                return to fuse_chan_send()
                                fuse_put_request(req)
                                  req may be freed
  clear_bit(FR_SENT)     <----- possible use-after-free
  req->in.h.unique ...   <----- possible use-after-free
  list_splice(to_queue, pending)

fuse_remove_pending_req() treats FR_PENDING as meaning that req->list is
on a queue protected by the lock passed to it. Here req->list still belongs
to the private to_queue, so the signal path can remove and drop the queue
reference while resend still dereferences req.

Fix this by taking fiq->lock before publishing FR_PENDING, and keep the
request state updates and pending-list splice in the same critical
section.

Fixes: 760eac73f9f6 ("fuse: Introduce a new notification type for resend pending requests")
Cc: stable@vger.kernel.org
Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
---
 fs/fuse/dev.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 5763a7cd3b37f..a0ca400697611 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1760,7 +1760,7 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
 void fuse_chan_resend(struct fuse_chan *fch)
 {
 	struct fuse_dev *fud;
-	struct fuse_req *req, *next;
+	struct fuse_req *req;
 	struct fuse_iqueue *fiq = &fch->iq;
 	LIST_HEAD(to_queue);
 	unsigned int i;
@@ -1781,28 +1781,25 @@ 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.
+	 * fuse_remove_pending_req() assumes that FR_PENDING implies req->list
+	 * is protected by the supplied queue lock.  Publish the flag under
+	 * fiq->lock so the request cannot be removed while it is still on the
+	 * private to_queue list.
 	 */
 	list_for_each_entry(req, &to_queue, list) {
+		clear_bit(FR_SENT, &req->flags);
+		/* mark the request as resend request */
+		req->in.h.unique |= FUSE_UNIQUE_RESEND;
 		if (test_bit(FR_INTERRUPTED, &req->flags))
 			list_del_init(&req->intr_entry);
+		set_bit(FR_PENDING, &req->flags);
 	}
 	/* iq and pq requests are both oldest to newest */
 	list_splice(&to_queue, &fiq->pending);
-- 
2.39.5 (Apple Git-154)