[PATCH 3/5] 9p/virtio: keep a request until the device returns it

[Lianjie Wang]

When the RFLUSH of a pending request arrives, p9_virtio_cancelled()
drops the virtio transport's reference to it. However,
p9_virtio_zc_request() has already dropped that reference if a fatal
signal ended its wait for room in the ring, since the request then never
reached the device. Such a request is still flushed, so the reference is
dropped twice. This can happen when concurrent requests fill the ring
and a waiting process is killed:

  refcount_t: underflow; use-after-free.
  WARNING: lib/refcount.c:37 at refcount_warn_saturate+0x84/0xd0, CPU#3: dd/293
  Call Trace:
   p9_req_put+0xe5/0x190
   p9_client_zc_rpc+0x52e/0x970
   p9_client_read_once+0x237/0x6c0
   p9_client_read+0x9d/0x140

Additionally, a misbehaving device can return the request after the
RFLUSH, so req_done() writes into the freed request:

  BUG: KASAN: slab-use-after-free in req_done+0x1d9/0x280
  Write of size 4 at addr ffff88800392d758 by task swapper/1/0
  Call Trace:
   req_done+0x1d9/0x280
   vring_interrupt+0x32a/0x540
  The buggy address belongs to the object at ffff88800392d708
   which belongs to the cache p9_req_t of size 136
  The buggy address is located 80 bytes inside of
   freed 136-byte region [ffff88800392d708, ffff88800392d790)

Fix this by dropping the reference in req_done() for a buffer without a
reply as well, since the device can return a flushed request without
one. p9_virtio_cancelled() is no longer needed.

This patch is assisted by Claude Opus 5.5.

Fixes: 728356dedeff ("9p: Add refcount to p9_req_t")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Lianjie Wang <lian@xxxxxxxxxxxxxxxxxx>
---
 net/9p/trans_virtio.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 53046668c16b1..68c3457bec401 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -145,6 +145,9 @@ static void req_done(struct virtqueue *vq)
 		if (len) {
 			req->rc.size = len;
 			p9_client_cb(chan->client, req, REQ_STATUS_RCVD);
+		} else {
+			/* No reply, so put the ref that p9_client_cb() puts */
+			p9_req_put(chan->client, req);
 		}
 	}
 	spin_unlock_irqrestore(&chan->lock, flags);
@@ -195,13 +198,6 @@ static int p9_virtio_cancel(struct p9_client *client, struct p9_req_t *req)
 	return 1;
 }
 
-/* Reply won't come, so drop req ref */
-static int p9_virtio_cancelled(struct p9_client *client, struct p9_req_t *req)
-{
-	p9_req_put(client, req);
-	return 0;
-}
-
 /**
  * pack_sg_list_p - Just like pack_sg_list. Instead of taking a buffer,
  * this takes a list of pages.
@@ -806,7 +802,6 @@ static struct p9_trans_module p9_virtio_trans = {
 	.request = p9_virtio_request,
 	.zc_request = p9_virtio_zc_request,
 	.cancel = p9_virtio_cancel,
-	.cancelled = p9_virtio_cancelled,
 	/*
 	 * We leave one entry for input and one entry for response
 	 * headers. We also skip one more entry to accommodate, address
-- 
2.56.0