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: [email protected]
Signed-off-by: Nirbhay Kumar <[email protected]>
---
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;
