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

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

[nirbhayykumarr]

In mei_release(), a host client is torn down upon close(). During this
teardown sequence, mei_cl_disconnect() is invoked, which 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 client's completed queue during teardown.

Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
Cc: stable@vger.kernel.org
---
v3:
 - Removed non-standard Helped-by tags.
 - Omitted Assisted-by: No AI or co-authors were used. The "we" in my original report referred to a colleague who merely verified and confirmed the bug.
v2:
 - Removed redundant Reported-by tag.
 - Added Fixes tag pointing to commit f35fe5f47ed0.
 - Sent inline plain text without attachments.

 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 v3] misc: mei: fix race condition between client teardown and read completion

[gregkh]

On Mon, Aug 24, 2026 at 09:32:27AM +0000, nirbhayykumarr@proton.me wrote:
> In mei_release(), a host client is torn down upon close(). During this
> teardown sequence, mei_cl_disconnect() is invoked, which 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 client's completed queue during teardown.
> 
> Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
> Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
> Cc: stable@vger.kernel.org
> ---
> v3:
>  - Removed non-standard Helped-by tags.
>  - Omitted Assisted-by: No AI or co-authors were used. The "we" in my original report referred to a colleague who merely verified and confirmed the bug.

Just to confirm, the original bug report, and test program, and this
hugely long changelog were not generated by any sort of LLM at all?

So how was this issue found?  What tool was used to poke around on the
mei codebase?

thanks,

greg k-h

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

[nirbhayykumarr]

Hi Greg,

To answer your first question directly: Yes, I used an LLM to help polish my changelog and commit description, and learn the strict kernel mailing list etiquette. This is my very first kernel contribution, and I used it to help navigate the patching process. However, the core technical work, finding the bug, root causing it, writing the C patch, and testing it is entirely my own.

Regarding how this was found: This came out of a broader research project centered around systematic fuzzing and interface testing of /dev/mei0. As part of testing various subsystem boundaries, I wrote a custom multi-threaded C stress utility that rapidly hammers the driver, more specifically, one thread streaming asynchronous MKHI requests while a second thread concurrently drives rapid open/close/reconnect cycles.

During this stress test, dmesg caught the WARNING at drivers/misc/mei/client.c:698 (mei_cl_unlink) because cl->rd_completed was not empty. Then I traced the driver source code in drivers/misc/mei/ to understand why rd_completed was non-empty at unlink time: mei_release() called mei_cl_flush_queues() before mei_cl_disconnect(), but mei_cl_disconnect() temporarily drops dev->device_lock while waiting for firmware response. During that lock-drop window, deferred read completions on the interrupt thread re-populated cl->rd_completed before mei_cl_unlink() ran. Moving the queue flush inside mei_cl_unlink() under the link state lock resolved the race and silenced the WARN_ON.

And my colleague simply helped me sanity check the trace to ensure it was a legitimate kernel driver bug and not a misunderstanding on my end before reporting it.

Thanks,
Nirbhay Kumar


On Monday, August 24th, 2026 at 3:10 PM, gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> wrote:

> On Mon, Aug 24, 2026 at 09:32:27AM +0000, nirbhayykumarr@proton.me wrote:
> > In mei_release(), a host client is torn down upon close(). During this
> > teardown sequence, mei_cl_disconnect() is invoked, which 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 client's completed queue during teardown.
> >
> > Fixes: f35fe5f47ed0 ("mei: add a vtag map for each client")
> > Signed-off-by: Nirbhay Kumar <nirbhayykumarr@proton.me>
> > Cc: stable@vger.kernel.org
> > ---
> > v3:
> >  - Removed non-standard Helped-by tags.
> >  - Omitted Assisted-by: No AI or co-authors were used. The "we" in my original report referred to a colleague who merely verified and confirmed the bug.
> 
> Just to confirm, the original bug report, and test program, and this
> hugely long changelog were not generated by any sort of LLM at all?
> 
> So how was this issue found?  What tool was used to poke around on the
> mei codebase?
> 
> thanks,
> 
> greg k-h
>

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

[gregkh]

On Mon, Aug 24, 2026 at 11:00:15AM +0000, nirbhayykumarr@proton.me wrote:
> Hi Greg,

Hi, but please don't top-post, that's not good email etiquette :)

And please wrap your lines, I have done so manually below so we can read
them...

> To answer your first question directly: Yes, I used an LLM to help
> polish my changelog and commit description, and learn the strict
> kernel mailing list etiquette. This is my very first kernel
> contribution, and I used it to help navigate the patching process.
> However, the core technical work, finding the bug, root causing it,
> writing the C patch, and testing it is entirely my own.

That's great, but as you used a LLM, please document it as such.

> Regarding how this was found: This came out of a broader research
> project centered around systematic fuzzing and interface testing of
> /dev/mei0. As part of testing various subsystem boundaries, I wrote a
> custom multi-threaded C stress utility that rapidly hammers the
> driver, more specifically, one thread streaming asynchronous MKHI
> requests while a second thread concurrently drives rapid
> open/close/reconnect cycles.

Great, fuzzers are nice, you should explain that in the changelog as
well.

thanks,

greg k-h