The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Alexander Usyskin <alexander.usyskin@intel.com>
Cc: Reuven Abliyev <reuven.abliyev@intel.com>, linux-kernel@vger.kernel.org
Subject: Re: [char-misc-next v2] mei: bus: fix device leak
Date: Sat, 28 Jun 2025 14:10:32 +0200	[thread overview]
Message-ID: <2025062836-twentieth-kudos-1148@gregkh> (raw)
In-Reply-To: <20250624110520.1403597-1-alexander.usyskin@intel.com>

On Tue, Jun 24, 2025 at 02:05:20PM +0300, Alexander Usyskin wrote:
> The bus rescan function creates bus devices for all clients.
> The fixup routine is executed on all devices, unneeded
> devices are removed and fully initialized once set
> is_added flag to 1.

I don't understand why the mei bus is so special that it has to have
this type of flag, when no other bus has that for its devices.  The bus
code should know if the device has been properly added or not, if not,
then no release function can be called and the structure isn't even
viable to be used or touched at all.

So why is this needed?

> 
> If link to firmware is reset right after all devices are
> initialized, but before fixup is executed, the rescan tries
> to remove devices.
> The is_added flag is not set and the mei_cl_bus_dev_destroy
> returns prematurely.
> Allow to clean up device when is_added flag is unset to
> account for above scenario.
> 
> Fixes: 6009595a66e4 ("mei: bus: link client devices instead of host clients")
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/misc/mei/bus.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index 67176caf5416..f2e5d550c6b4 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -1430,17 +1430,14 @@ static void mei_cl_bus_dev_stop(struct mei_cl_device *cldev)
>   */
>  static void mei_cl_bus_dev_destroy(struct mei_cl_device *cldev)
>  {
> -
>  	WARN_ON(!mutex_is_locked(&cldev->bus->cl_bus_lock));
>  
> -	if (!cldev->is_added)
> -		return;
> -
> -	device_del(&cldev->dev);
> +	if (cldev->is_added) {
> +		device_del(&cldev->dev);
> +		cldev->is_added = 0;
> +	}

How can destroy be called here if the device has not been added before?
How can it be hanging around in memory at all if the device_add() call
was not successful when it was originally called?

confused,

greg k-h

  parent reply	other threads:[~2025-06-28 12:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-24 11:05 [char-misc-next v2] mei: bus: fix device leak Alexander Usyskin
2025-06-24 11:35 ` Greg Kroah-Hartman
2025-06-28 12:10 ` Greg Kroah-Hartman [this message]
2025-06-30 10:52   ` Usyskin, Alexander
2025-06-30 11:18     ` Greg Kroah-Hartman
2025-06-30 11:27       ` Usyskin, Alexander
2025-06-30 12:54         ` Greg Kroah-Hartman
2025-07-07  7:08           ` Usyskin, Alexander
2025-07-07  7:46             ` Greg Kroah-Hartman
2025-07-07  8:02               ` Usyskin, Alexander
2025-06-28 12:10 ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2025062836-twentieth-kudos-1148@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=alexander.usyskin@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=reuven.abliyev@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox