[PATCH] 9p/xen: fix use-after-free in p9_xen_request

[Yizhou Zhao]

p9_xen_request() looks up the transport private data in xen_9pfs_devs
under xen_9pfs_lock, but drops the lock before using the returned
pointer.

This means the lookup only protects the list traversal. It does not pin
the lifetime of the object. If xen_9pfs_front_remove() runs after the
read lock is dropped, it can remove the entry from the list and free the
private data and its rings while p9_xen_request() is still about to use
them.

Fix this by adding a kref to xen_9pfs_front_priv. Take a reference while
still holding xen_9pfs_lock, and drop it after the request has finished.
Make the remove path drop the list reference instead of freeing the
object directly, so the final free is deferred until any in-flight
p9_xen_request() users have released their references.

Fixes: f023f18ddf41 ("xen/9pfs: send requests to the backend")
Reported-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
Reported-by: Yuxiang Yang <yangyx22@xxxxxxxxxxxxxxxxxxxxx>
Reported-by: Ao Wang <wangao@xxxxxxxxxx>
Reported-by: Xuewei Feng <fengxw06@xxxxxxx>
Reported-by: Qi Li <qli01@xxxxxxxxxxxxxxx>
Reported-by: Ke Xu <xuke@xxxxxxxxxxxxxxx>
Assisted-by: GLM:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
---
 net/9p/trans_xen.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 47af5a1..abdc652 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -14,6 +14,7 @@
 #include <xen/interface/io/9pfs.h>

 #include <linux/module.h>
+#include <linux/kref.h>
 #include <linux/spinlock.h>
 #include <linux/fs_context.h>
 #include <net/9p/9p.h>
@@ -54,6 +55,7 @@ struct xen_9pfs_front_priv {
 	struct xenbus_device *dev;
 	char *tag;
 	struct p9_client *client;
+	struct kref refcount;

 	struct xen_9pfs_dataring *rings;
 };
@@ -114,6 +116,8 @@ static bool p9_xen_write_todo(struct xen_9pfs_dataring *ring, RING_IDX size)
 		xen_9pfs_queued(prod, cons, XEN_9PFS_RING_SIZE(ring)) >= size;
 }

+static void xen_9pfs_front_release(struct kref *ref);
+
 static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 {
 	struct xen_9pfs_front_priv *priv;
@@ -128,9 +132,12 @@ static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 		if (priv->client == client)
 			break;
 	}
-	read_unlock(&xen_9pfs_lock);
-	if (list_entry_is_head(priv, &xen_9pfs_devs, list))
+	if (list_entry_is_head(priv, &xen_9pfs_devs, list)) {
+		read_unlock(&xen_9pfs_lock);
 		return -EINVAL;
+	}
+	kref_get(&priv->refcount);
+	read_unlock(&xen_9pfs_lock);

 	num = p9_req->tc.tag % XEN_9PFS_NUM_RINGS;
 	ring = &priv->rings[num];
@@ -165,6 +172,7 @@ static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 	spin_unlock_irqrestore(&ring->lock, flags);
 	notify_remote_via_irq(ring->irq);
 	p9_req_put(client, p9_req);
+	kref_put(&priv->refcount, xen_9pfs_front_release);

 	return 0;
 }
@@ -309,6 +317,13 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
 	kfree(priv);
 }

+static void xen_9pfs_front_release(struct kref *ref)
+{
+	struct xen_9pfs_front_priv *priv =
+		container_of(ref, struct xen_9pfs_front_priv, refcount);
+	xen_9pfs_front_free(priv);
+}
+
 static void xen_9pfs_front_remove(struct xenbus_device *dev)
 {
 	struct xen_9pfs_front_priv *priv;
@@ -323,7 +338,7 @@ static void xen_9pfs_front_remove(struct xenbus_device *dev)
 	list_del(&priv->list);
 	write_unlock(&xen_9pfs_lock);

-	xen_9pfs_front_free(priv);
+	kref_put(&priv->refcount, xen_9pfs_front_release);
 }

 static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
@@ -421,6 +436,7 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
 	if (!priv)
 		return -ENOMEM;
 	priv->dev = dev;
+	kref_init(&priv->refcount);
 	priv->rings = kzalloc_objs(*priv->rings, XEN_9PFS_NUM_RINGS);
 	if (!priv->rings) {
 		kfree(priv);
--
2.43.0