[PATCH v4] misc: mei: fix race condition between client teardown and read completion

nirbhayykumarr@proton.me posted 1 patch 1 month ago
drivers/misc/mei/client.c | 2 ++
drivers/misc/mei/main.c   | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
[PATCH v4] misc: mei: fix race condition between client teardown and read completion
Posted by nirbhayykumarr@proton.me 1 month ago
This issue was discovered using a custom multi-threaded C fuzzer
designed to stress-test HECI client lifecycles over /dev/mei0. By
running one thread that streams asynchronous MKHI requests while a
second thread concurrently drives rapid open/close/reconnect cycles,
a race condition is triggered during client teardown.

In mei_release(), a host client is torn down upon close(). During this
sequence, mei_cl_disconnect() is invoked, which temporarily releases
dev->device_lock while waiting for the firmware response.

If an in-flight read request was previously submitted, an incoming
completion interrupt processed concurrently by the MEI interrupt
handler can add a completed callback into cl->rd_completed via
mei_cl_add_rd_completed().

Because mei_cl_flush_queues(cl, NULL) was invoked before
mei_cl_unlink(cl), an incoming completion callback can slip into
cl->rd_completed after the flush has completed but before the client
is unlinked from dev->file_list. When mei_cl_unlink() is subsequently
called, the invariant check at drivers/misc/mei/client.c:698 triggers:

  WARN_ON(!list_empty(&cl->rd_completed) ||
          !list_empty(&cl->rd_pending) ||
          !list_empty(&cl->link));

Call trace:
  WARNING: CPU: 2 PID: 5056 at drivers/misc/mei/client.c:698 mei_cl_unlink+0xaa/0x140 [mei]
  RIP: 0010:mei_cl_unlink+0xaa/0x140 [mei]
  Call Trace:
   <TASK>
   mei_release+0x202/0x270 [mei]
   __fput+0x105/0x2e0
   __x64_sys_close+0x90/0x140
   do_syscall_64+0xaa/0x660
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
   </TASK>

Immediately following mei_cl_unlink(), mei_release() calls kfree(cl).
If any remaining or deferred callback references the freed client, a
use-after-free occurs.

Fix this by flushing queues after unlinking the client from
dev->file_list inside mei_cl_unlink(), preventing concurrent IRQ
completions from populating the completed queue during teardown.

Note: An LLM was used solely to assist with the proper phrasing and 
formatting of this commit message. The vulnerability discovery, 
fuzzer development, and code fix are entirely original author's work.

Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
Cc: stable@vger.kernel.org
---
v4:
 - Added the fuzzer methodology to the commit message per maintainer request.
 - Documented LLM usage for Changelog formatting in the commit message.
 - 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 | 2 ++
 drivers/misc/mei/main.c   | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 643b003..38b5792 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -695,6 +695,8 @@ int mei_cl_unlink(struct mei_cl *cl)
 	cl->state = MEI_FILE_UNINITIALIZED;
 	cl->writing_state = MEI_IDLE;
 
+	mei_cl_flush_queues(cl, NULL);
+
 	WARN_ON(!list_empty(&cl->rd_completed) ||
 		!list_empty(&cl->rd_pending) ||
 		!list_empty(&cl->link));
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 4fbf0b3..9e14ab4 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -148,7 +148,6 @@ static int mei_release(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	mei_cl_flush_queues(cl, NULL);
 	cl_dbg(dev, cl, "removing\n");
 
 	mei_cl_unlink(cl);
-- 
2.55.0
RE: [PATCH v4] misc: mei: fix race condition between client teardown and read completion
Posted by Usyskin, Alexander 4 weeks ago
> Subject: [PATCH v4] misc: mei: fix race condition between client teardown and
> read completion
> 
> This issue was discovered using a custom multi-threaded C fuzzer
> designed to stress-test HECI client lifecycles over /dev/mei0. By
> running one thread that streams asynchronous MKHI requests while a
> second thread concurrently drives rapid open/close/reconnect cycles,
> a race condition is triggered during client teardown.
> 
> In mei_release(), a host client is torn down upon close(). During this
> sequence, mei_cl_disconnect() is invoked, which temporarily releases
> dev->device_lock while waiting for the firmware response.
> 
> If an in-flight read request was previously submitted, an incoming
> completion interrupt processed concurrently by the MEI interrupt
> handler can add a completed callback into cl->rd_completed via
> mei_cl_add_rd_completed().
> 
> Because mei_cl_flush_queues(cl, NULL) was invoked before
> mei_cl_unlink(cl), an incoming completion callback can slip into
> cl->rd_completed after the flush has completed but before the client
> is unlinked from dev->file_list. When mei_cl_unlink() is subsequently
> called, the invariant check at drivers/misc/mei/client.c:698 triggers:
> 
>   WARN_ON(!list_empty(&cl->rd_completed) ||
>           !list_empty(&cl->rd_pending) ||
>           !list_empty(&cl->link));
> 
> Call trace:
>   WARNING: CPU: 2 PID: 5056 at drivers/misc/mei/client.c:698
> mei_cl_unlink+0xaa/0x140 [mei]
>   RIP: 0010:mei_cl_unlink+0xaa/0x140 [mei]
>   Call Trace:
>    <TASK>
>    mei_release+0x202/0x270 [mei]
>    __fput+0x105/0x2e0
>    __x64_sys_close+0x90/0x140
>    do_syscall_64+0xaa/0x660
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    </TASK>
> 
> Immediately following mei_cl_unlink(), mei_release() calls kfree(cl).
> If any remaining or deferred callback references the freed client, a
> use-after-free occurs.
> 
> Fix this by flushing queues after unlinking the client from
> dev->file_list inside mei_cl_unlink(), preventing concurrent IRQ
> completions from populating the completed queue during teardown.
> 
> Note: An LLM was used solely to assist with the proper phrasing and
> formatting of this commit message. The vulnerability discovery,
> fuzzer development, and code fix are entirely original author's work.
> 
> Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
> Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
> Cc: stable@vger.kernel.org
> ---
> v4:
>  - Added the fuzzer methodology to the commit message per maintainer
> request.
>  - Documented LLM usage for Changelog formatting in the commit message.
>  - 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 | 2 ++
>  drivers/misc/mei/main.c   | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> index 643b003..38b5792 100644
> --- a/drivers/misc/mei/client.c
> +++ b/drivers/misc/mei/client.c
> @@ -695,6 +695,8 @@ int mei_cl_unlink(struct mei_cl *cl)
>  	cl->state = MEI_FILE_UNINITIALIZED;
>  	cl->writing_state = MEI_IDLE;
> 
> +	mei_cl_flush_queues(cl, NULL);
> +
>  	WARN_ON(!list_empty(&cl->rd_completed) ||
>  		!list_empty(&cl->rd_pending) ||
>  		!list_empty(&cl->link));
> diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
> index 4fbf0b3..9e14ab4 100644
> --- a/drivers/misc/mei/main.c
> +++ b/drivers/misc/mei/main.c
> @@ -148,7 +148,6 @@ static int mei_release(struct inode *inode, struct file
> *file)
>  		goto out;
>  	}
> 
> -	mei_cl_flush_queues(cl, NULL);
>  	cl_dbg(dev, cl, "removing\n");
> 
>  	mei_cl_unlink(cl);

This change look like no-op in this flow as you moved mei_cl_flush_queues() inside the mei_cl_unlink(),
but mei_cl_unlink() is called here straight after mei_cl_flush_queues() without releasing device_lock.

Seems like the failure is real but analysis and patch is wrong.
I suppose there is another path that call mei_cl_unlink() without proper queue cleanup.

- - 
Thanks,
Sasha




> --
> 2.55.0
RE: [PATCH v4] misc: mei: fix race condition between client teardown and read completion
Posted by nirbhayykumarr@proton.me 3 weeks, 6 days ago
On Sunday, August 30th, 2026 at 2:06 PM, Usyskin, Alexander <alexander.usyskin@intel.com> wrote:
> 
> This change look like no-op in this flow as you moved mei_cl_flush_queues() inside the mei_cl_unlink(),
> but mei_cl_unlink() is called here straight after mei_cl_flush_queues() without releasing device_lock.
> 
> Seems like the failure is real but analysis and patch is wrong.
> I suppose there is another path that call mei_cl_unlink() without proper queue cleanup.
> 

Hi Sasha,

You were right. Moving mei_cl_flush_queues() into mei_cl_unlink() 
was a no-op because the lock is held across both calls.

Your comment prompted me to look deeper, and I realized the issue 
wasn't the call location, but that mei_cl_flush_queues() itself was 
fundamentally broken. Specifically, mei_cl_free_pending() was using 
list_first_entry_or_null() and only freeing a maximum of one callback 
per flush leaving any other queued read callbacks stranded on the list 
to trigger the WARN_ON.

I will send a v5 patch to the list which corrects this by properly 
iterating through the pending queues and allowing per-file descriptor 
filtering during teardown.

Thanks for the review and guidance.

Best regards,
Nirbhay Kumar