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

nirbhayykumarr@proton.me posted 1 patch 3 weeks, 5 days ago
drivers/misc/mei/client.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
[PATCH v5] misc: mei: fix queue cleanup and list handling during client teardown
Posted by nirbhayykumarr@proton.me 3 weeks, 5 days ago
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