[PATCH] misc: mei: prevent reusing disconnecting clients in mei_ioctl_connect_vtag

nirbhayykumarr@proton.me posted 1 patch 3 weeks, 1 day ago
drivers/misc/mei/client.c    | 24 ++++++++++++-
drivers/misc/mei/client.h    |  1 +
drivers/misc/mei/interrupt.c | 65 +++++++++++++++++++++++-------------
drivers/misc/mei/main.c      | 24 +++----------
4 files changed, 69 insertions(+), 45 deletions(-)
[PATCH] misc: mei: prevent reusing disconnecting clients in mei_ioctl_connect_vtag
Posted by nirbhayykumarr@proton.me 3 weeks, 1 day ago
This issue was discovered using a custom multi-threaded C fuzzer
designed to stress-test MEI Virtual Tag (vtag) client lifecycles and
multiplexing over /dev/mei0. By concurrently racing rapid vtag
connections against file descriptor closures and streaming I/O, a
race condition is triggered during client teardown.

In mei_release(), closing the last file descriptor holding a virtual tag
invokes mei_cl_disconnect(). Inside __mei_cl_disconnect(),
dev->device_lock is dropped while awaiting the firmware disconnect ACK
on cl->wait.

During this lock-drop window, a concurrent IOCTL_MEI_CONNECT_CLIENT_VTAG
call on the same UUID scans dev->file_list. Because
mei_ioctl_connect_vtag() only verified pos->me_cl without checking
pos->state, it matched the tearing-down client (in
MEI_FILE_DISCONNECTING), repointed file->private_data to pos, and added a
new vtag to pos->vtag_map.

When the disconnect ACK arrived, __mei_cl_disconnect() called
mei_cl_set_disconnected(pos), setting pos->me_cl = NULL and pos->state =
MEI_FILE_DISCONNECTED. Because pos->vtag_map now contained the second
thread's tag, mei_release() skipped unlinking and freeing pos. The second
thread then attempted to reuse this disconnected client, causing packet
demuxing mismatches, continuous CSME hardware link resets, and DRM/i915
display freezes.

Fix this by:
1. Validating pos->state in mei_ioctl_connect_vtag() to ensure only
   active clients (MEI_FILE_CONNECTED or MEI_FILE_CONNECTING) are reused.
2. Setting cb->vtag during callback allocation in mei_io_cb_init() via
   mei_cl_vtag_by_fp().
3. Demuxing incoming read packets in mei_cl_irq_read_msg() by matching
   vtag against cl->rd_pending rather than blindly dequeuing the head.

Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
Cc: stable@vger.kernel.org
Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
---
 drivers/misc/mei/client.c    | 24 ++++++++++++-
 drivers/misc/mei/client.h    |  1 +
 drivers/misc/mei/interrupt.c | 65 +++++++++++++++++++++++-------------
 drivers/misc/mei/main.c      | 24 +++----------
 4 files changed, 69 insertions(+), 45 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 5f648481024..b10c483673a 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -379,7 +379,7 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
 	cb->cl = cl;
 	cb->buf_idx = 0;
 	cb->fop_type = type;
-	cb->vtag = 0;
+	cb->vtag = mei_cl_vtag_by_fp(cl, fp);
 	cb->ext_hdr = NULL;
 
 	return cb;
@@ -1313,6 +1313,28 @@ const struct file *mei_cl_fp_by_vtag(const struct mei_cl *cl, u8 vtag)
 	return ERR_PTR(-ENOENT);
 }
 
+/**
+ * mei_cl_vtag_by_fp - obtain the vtag by file pointer
+ *
+ * @cl: host client
+ * @fp: pointer to file structure
+ *
+ * Return: vtag value on success, otherwise 0
+ */
+u8 mei_cl_vtag_by_fp(const struct mei_cl *cl, const struct file *fp)
+{
+	struct mei_cl_vtag *cl_vtag;
+
+	if (!cl || !fp)
+		return 0;
+
+	list_for_each_entry(cl_vtag, &cl->vtag_map, list)
+		if (cl_vtag->fp == fp)
+			return cl_vtag->vtag;
+	return 0;
+}
+
 /**
  * mei_cl_reset_read_by_vtag - reset pending_read flag by given vtag
  *
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index 031114478bc..a15f1936703 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -146,6 +146,7 @@ int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp);
 
 struct mei_cl_vtag *mei_cl_vtag_alloc(struct file *fp, u8 vtag);
 const struct file *mei_cl_fp_by_vtag(const struct mei_cl *cl, u8 vtag);
+u8 mei_cl_vtag_by_fp(const struct mei_cl *cl, const struct file *fp);
 int mei_cl_vt_support_check(const struct mei_cl *cl);
 /*
  *  MEI input output function prototype
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 4262965c4f9..1e5b7728b56 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -96,7 +96,7 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 			       struct list_head *cmpl_list)
 {
 	struct mei_device *dev = cl->dev;
-	struct mei_cl_cb *cb;
+	struct mei_cl_cb *cb = NULL;
 
 	struct mei_ext_hdr_vtag *vtag_hdr = NULL;
 	struct mei_ext_hdr_gsc_f2h *gsc_f2h = NULL;
@@ -108,24 +108,11 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 	length = mei_hdr->length;
 	ext_len = 0;
 	if (mei_hdr->extended) {
+		struct mei_ext_hdr *ext = mei_ext_begin(meta);
+
 		ext_len = sizeof(*meta) + mei_slots2data(meta->size);
 		length -= ext_len;
-	}
 
-	cb = list_first_entry_or_null(&cl->rd_pending, struct mei_cl_cb, list);
-	if (!cb) {
-		if (!mei_cl_is_fixed_address(cl)) {
-			cl_err(dev, cl, "pending read cb not found\n");
-			goto discard;
-		}
-		cb = mei_cl_alloc_cb(cl, mei_cl_mtu(cl), MEI_FOP_READ, cl->fp);
-		if (!cb)
-			goto discard;
-		list_add_tail(&cb->list, &cl->rd_pending);
-	}
-
-	if (mei_hdr->extended) {
-		struct mei_ext_hdr *ext = mei_ext_begin(meta);
 		do {
 			switch (ext->type) {
 			case MEI_EXT_HDR_VTAG:
@@ -133,18 +120,16 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 				break;
 			case MEI_EXT_HDR_GSC:
 				gsc_f2h = (struct mei_ext_hdr_gsc_f2h *)ext;
-				cb->ext_hdr = (struct mei_ext_hdr *) kzalloc_obj(*gsc_f2h);
-				if (!cb->ext_hdr) {
-					cb->status = -ENOMEM;
-					goto discard;
-				}
 				break;
 			case MEI_EXT_HDR_NONE:
 				fallthrough;
 			default:
 				cl_err(dev, cl, "unknown extended header\n");
-				cb->status = -EPROTO;
-				break;
+				cb = list_first_entry_or_null(&cl->rd_pending,
+							      struct mei_cl_cb, list);
+				if (cb)
+					cb->status = -EPROTO;
+				goto discard;
 			}
 
 			ext = mei_ext_next(ext);
@@ -152,11 +137,38 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 
 		if (!vtag_hdr && !gsc_f2h) {
 			cl_dbg(dev, cl, "no vtag or gsc found in extended header.\n");
-			cb->status = -EPROTO;
+			cb = list_first_entry_or_null(&cl->rd_pending,
+						      struct mei_cl_cb, list);
+			if (cb)
+				cb->status = -EPROTO;
 			goto discard;
 		}
 	}
 
+	if (vtag_hdr) {
+		struct mei_cl_cb *pos;
+
+		list_for_each_entry(pos, &cl->rd_pending, list) {
+			if (pos->vtag == vtag_hdr->vtag) {
+				cb = pos;
+				break;
+			}
+		}
+	} else {
+		cb = list_first_entry_or_null(&cl->rd_pending, struct mei_cl_cb, list);
+	}
+
+	if (!cb) {
+		if (!mei_cl_is_fixed_address(cl)) {
+			cl_err(dev, cl, "pending read cb not found\n");
+			goto discard;
+		}
+		cb = mei_cl_alloc_cb(cl, mei_cl_mtu(cl), MEI_FOP_READ, cl->fp);
+		if (!cb)
+			goto discard;
+		list_add_tail(&cb->list, &cl->rd_pending);
+	}
+
 	if (vtag_hdr) {
 		cl_dbg(dev, cl, "vtag: %d\n", vtag_hdr->vtag);
 		if (cb->vtag && cb->vtag != vtag_hdr->vtag) {
@@ -187,6 +199,11 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 			cb->status = -EPROTO;
 			goto discard;
 		}
+		cb->ext_hdr = (struct mei_ext_hdr *) kzalloc_obj(*gsc_f2h);
+		if (!cb->ext_hdr) {
+			cb->status = -ENOMEM;
+			goto discard;
+		}
 		memcpy(cb->ext_hdr, gsc_f2h, ext_hdr_len);
 	}
 
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 4fbf0b32361..99ac29a464b 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -284,26 +284,6 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 	return rets;
 }
 
-/**
- * mei_cl_vtag_by_fp - obtain the vtag by file pointer
- *
- * @cl: host client
- * @fp: pointer to file structure
- *
- * Return: vtag value on success, otherwise 0
- */
-static u8 mei_cl_vtag_by_fp(const struct mei_cl *cl, const struct file *fp)
-{
-	struct mei_cl_vtag *cl_vtag;
-
-	if (!fp)
-		return 0;
-
-	list_for_each_entry(cl_vtag, &cl->vtag_map, list)
-		if (cl_vtag->fp == fp)
-			return cl_vtag->vtag;
-	return 0;
-}
 
 /**
  * mei_write - the write function.
@@ -564,6 +544,10 @@ static int mei_ioctl_connect_vtag(struct file *file,
 			if (!pos->me_cl)
 				continue;
 
+			if (pos->state != MEI_FILE_CONNECTED &&
+			    pos->state != MEI_FILE_CONNECTING)
+				continue;
+
 			/* only search for same UUID */
 			if (uuid_le_cmp(*mei_cl_uuid(pos), *in_client_uuid))
 				continue;