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

["gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>]

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