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

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

[nirbhayykumarr]

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;

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

[gregkh]

On Fri, Sep 04, 2026 at 03:59:58AM +0000, nirbhayykumarr@proton.me wrote:
> 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;

Where is the locking here and elsewhere in this patch?  WHat prevents
the list from changing while you walk it?  And what prevents this from
changing right after you get it?

thanks,

greg k-h