[PATCH v5] misc: mei: fix queue cleanup and list handling during client teardown

[PATCH v5] misc: mei: fix queue cleanup and list handling during client teardown

[nirbhayykumarr]

This issue was discovered using a multi threaded C fuzzer designed to
stress test HECI client lifecycles over /dev/mei0. When closing a client
while concurrent asynchronous requests are in flight, mei_cl_unlink()
triggers an invariant warning:

  WARNING: CPU: 2 PID: 5056 at drivers/misc/mei/client.c:698 mei_cl_unlink+0xaa/0x140 [mei]
  WARN_ON(!list_empty(&cl->rd_completed) ||
          !list_empty(&cl->rd_pending) ||
          !list_empty(&cl->link));

This occurs due to two issues in queue cleanup:
1. mei_cl_free_pending() uses list_first_entry_or_null(), freeing at
   most one callback from cl->rd_pending rather than purging all pending
   callbacks. When multiple pending reads are queued, subsequent entries
   remain in cl->rd_pending.
2. In mei_cl_flush_queues(cl, fp), when closing an individual vtag file
   descriptor (fp != NULL), pending and control queues (ctrl_wr_list,
   ctrl_rd_list, rd_pending) are skipped entirely, leaving dangling
   callbacks referencing the closed file object.

Fix this by:
- Updating mei_cl_free_pending() to iterate with list_for_each_entry_safe()
  and accept fp to filter callbacks matching the closing file descriptor,
  or free all callbacks when fp is NULL.
- Updating mei_io_list_flush_cl() to support fp filtering.
- Updating mei_cl_flush_queues() to clean control and pending read queues
  for both per-file closures and final client teardown.

Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
Cc: stable@vger.kernel.org
Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
---
v5:
 - Addressed maintainer review: fixed root causes in queue cleanup
   rather than moving call sites.
 - Updated mei_cl_free_pending() to iterate with list_for_each_entry_safe()
   to purge all pending callbacks.
 - Updated mei_io_list_flush_cl() and mei_cl_flush_queues() to support
   per-file (fp) queue flushing.
v4:
 - Added the fuzzer methodology to the commit message per maintainer request.
 - Manually wrapped commit message lines to 72 characters.
v3:
 - Removed non-standard Helped-by tags.
v2:
 - Removed redundant Reported-by tag.
 - Added Fixes tag pointing to commit f35fe5f47ed0.

 drivers/misc/mei/client.c | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 26d2b2742d5..5f648481024 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -390,14 +390,16 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
  *
  * @head:  an instance of our list structure
  * @cl:    host client
+ * @fp:    file pointer (matching cb file object), may be NULL
  */
 static void mei_io_list_flush_cl(struct list_head *head,
-				 const struct mei_cl *cl)
+				 const struct mei_cl *cl,
+				 const struct file *fp)
 {
 	struct mei_cl_cb *cb, *next;

 	list_for_each_entry_safe(cb, next, head, list) {
-		if (cl == cb->cl) {
+		if (cl == cb->cl && (!fp || fp == cb->fp)) {
 			list_del_init(&cb->list);
 			if (cb->fop_type == MEI_FOP_READ)
 				mei_io_cb_free(cb);
@@ -446,16 +448,19 @@ static void mei_io_rd_list_free_fp(struct mei_cl *cl, const struct file *fp)
 }

 /**
- * mei_cl_free_pending - free pending cb
+ * mei_cl_free_pending - free pending cbs
  *
  * @cl: host client
+ * @fp: file pointer (matching cb file object), may be NULL
  */
-static void mei_cl_free_pending(struct mei_cl *cl)
+static void mei_cl_free_pending(struct mei_cl *cl, const struct file *fp)
 {
-	struct mei_cl_cb *cb;
+	struct mei_cl_cb *cb, *next;

-	cb = list_first_entry_or_null(&cl->rd_pending, struct mei_cl_cb, list);
-	mei_io_cb_free(cb);
+	list_for_each_entry_safe(cb, next, &cl->rd_pending, list) {
+		if (!fp || fp == cb->fp)
+			mei_io_cb_free(cb);
+	}
 }

 /**
@@ -565,12 +570,9 @@ int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp)
 	cl_dbg(dev, cl, "remove list entry belonging to cl\n");
 	mei_io_tx_list_free_cl(&cl->dev->write_list, cl, fp);
 	mei_io_tx_list_free_cl(&cl->dev->write_waiting_list, cl, fp);
-	/* free pending and control cb only in final flush */
-	if (!fp) {
-		mei_io_list_flush_cl(&cl->dev->ctrl_wr_list, cl);
-		mei_io_list_flush_cl(&cl->dev->ctrl_rd_list, cl);
-		mei_cl_free_pending(cl);
-	}
+	mei_io_list_flush_cl(&cl->dev->ctrl_wr_list, cl, fp);
+	mei_io_list_flush_cl(&cl->dev->ctrl_rd_list, cl, fp);
+	mei_cl_free_pending(cl, fp);
 	mei_io_rd_list_free_fp(cl, fp);

 	return 0;
@@ -790,8 +792,8 @@ static void mei_cl_set_disconnected(struct mei_cl *cl)
 	cl->state = MEI_FILE_DISCONNECTED;
 	mei_io_tx_list_free_cl(&dev->write_list, cl, NULL);
 	mei_io_tx_list_free_cl(&dev->write_waiting_list, cl, NULL);
-	mei_io_list_flush_cl(&dev->ctrl_rd_list, cl);
-	mei_io_list_flush_cl(&dev->ctrl_wr_list, cl);
+	mei_io_list_flush_cl(&dev->ctrl_rd_list, cl, NULL);
+	mei_io_list_flush_cl(&dev->ctrl_wr_list, cl, NULL);
 	mei_cl_wake_all(cl);
 	cl->rx_flow_ctrl_creds = 0;
 	cl->tx_flow_ctrl_creds = 0;
@@ -1151,8 +1153,8 @@ int mei_cl_connect(struct mei_cl *cl, struct mei_me_client *me_cl,

 	if (!mei_cl_is_connected(cl)) {
 		if (cl->state == MEI_FILE_DISCONNECT_REQUIRED) {
-			mei_io_list_flush_cl(&dev->ctrl_rd_list, cl);
-			mei_io_list_flush_cl(&dev->ctrl_wr_list, cl);
+			mei_io_list_flush_cl(&dev->ctrl_rd_list, cl, NULL);
+			mei_io_list_flush_cl(&dev->ctrl_wr_list, cl, NULL);
 			 /* ignore disconnect return valuue;
 			  * in case of failure reset will be invoked
 			  */
-- 
2.55.0

Re: [PATCH v5] misc: mei: fix queue cleanup and list handling during client teardown

[gregkh]

On Mon, Aug 31, 2026 at 11:08:10AM +0000, nirbhayykumarr@proton.me wrote:
> This issue was discovered using a multi threaded C fuzzer designed to
> stress test HECI client lifecycles over /dev/mei0. When closing a client
> while concurrent asynchronous requests are in flight, mei_cl_unlink()
> triggers an invariant warning:
> 
>   WARNING: CPU: 2 PID: 5056 at drivers/misc/mei/client.c:698 mei_cl_unlink+0xaa/0x140 [mei]
>   WARN_ON(!list_empty(&cl->rd_completed) ||
>           !list_empty(&cl->rd_pending) ||
>           !list_empty(&cl->link));
> 
> This occurs due to two issues in queue cleanup:
> 1. mei_cl_free_pending() uses list_first_entry_or_null(), freeing at
>    most one callback from cl->rd_pending rather than purging all pending
>    callbacks. When multiple pending reads are queued, subsequent entries
>    remain in cl->rd_pending.
> 2. In mei_cl_flush_queues(cl, fp), when closing an individual vtag file
>    descriptor (fp != NULL), pending and control queues (ctrl_wr_list,
>    ctrl_rd_list, rd_pending) are skipped entirely, leaving dangling
>    callbacks referencing the closed file object.
> 
> Fix this by:
> - Updating mei_cl_free_pending() to iterate with list_for_each_entry_safe()
>   and accept fp to filter callbacks matching the closing file descriptor,
>   or free all callbacks when fp is NULL.
> - Updating mei_io_list_flush_cl() to support fp filtering.
> - Updating mei_cl_flush_queues() to clean control and pending read queues
>   for both per-file closures and final client teardown.
> 
> Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
> ---
> v5:
>  - Addressed maintainer review: fixed root causes in queue cleanup
>    rather than moving call sites.
>  - Updated mei_cl_free_pending() to iterate with list_for_each_entry_safe()
>    to purge all pending callbacks.
>  - Updated mei_io_list_flush_cl() and mei_cl_flush_queues() to support
>    per-file (fp) queue flushing.
> v4:
>  - Added the fuzzer methodology to the commit message per maintainer request.
>  - Manually wrapped commit message lines to 72 characters.
> v3:
>  - Removed non-standard Helped-by tags.
> v2:
>  - Removed redundant Reported-by tag.
>  - Added Fixes tag pointing to commit f35fe5f47ed0.
> 
>  drivers/misc/mei/client.c | 36 +++++++++++++++++++-----------------
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> index 26d2b2742d5..5f648481024 100644
> --- a/drivers/misc/mei/client.c
> +++ b/drivers/misc/mei/client.c
> @@ -390,14 +390,16 @@ static struct mei_cl_cb *mei_io_cb_init(struct mei_cl *cl,
>   *
>   * @head:  an instance of our list structure
>   * @cl:    host client
> + * @fp:    file pointer (matching cb file object), may be NULL
>   */
>  static void mei_io_list_flush_cl(struct list_head *head,
> -				 const struct mei_cl *cl)
> +				 const struct mei_cl *cl,
> +				 const struct file *fp)

This is now a rough api, as you need to look up and figure out why NULL
is in some calls and others they are not.  Why is the fp not part of the
other structures here already if it is relevant for them?

thanks,

greg k-h