linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Ma Ke <make24@iscas.ac.cn>
Cc: ioana.ciornei@nxp.com, davem@davemloft.net,
	andrew+netdev@lunn.ch, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	stable@vger.kernel.org, Laurentiu Tudor <laurentiu.tudor@nxp.com>
Subject: Re: [PATCH net v2 1/3] bus: fsl-mc: Fix potential double device reference in fsl_mc_get_endpoint()
Date: Fri, 18 Jul 2025 15:39:23 +0100	[thread overview]
Message-ID: <20250718143923.GF2459@horms.kernel.org> (raw)
In-Reply-To: <20250717022309.3339976-1-make24@iscas.ac.cn>

On Thu, Jul 17, 2025 at 10:23:07AM +0800, Ma Ke wrote:
> The fsl_mc_get_endpoint() function may call fsl_mc_device_lookup() 
> twice, which would increment the device's reference count twice if 
> both lookups find a device. This could lead to a reference count leak.
> 
> Found by code review.
> 
> Cc: stable@vger.kernel.org
> Fixes: 1ac210d128ef ("bus: fsl-mc: add the fsl_mc_get_endpoint function")

I think this should be:

Fixes: 8567494cebe5 ("bus: fsl-mc: rescan devices if endpoint not found")

I've CCed Laurentiu, the author of that commit.

> Signed-off-by: Ma Ke <make24@iscas.ac.cn>

Suggested-by: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>

> ---
>  drivers/bus/fsl-mc/fsl-mc-bus.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 7671bd158545..c1c0a4759c7e 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -943,6 +943,7 @@ struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev,
>  	struct fsl_mc_obj_desc endpoint_desc = {{ 0 }};
>  	struct dprc_endpoint endpoint1 = {{ 0 }};
>  	struct dprc_endpoint endpoint2 = {{ 0 }};
> +	struct fsl_mc_bus *mc_bus;
>  	int state, err;
>  
>  	mc_bus_dev = to_fsl_mc_device(mc_dev->dev.parent);
> @@ -966,6 +967,8 @@ struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev,
>  	strcpy(endpoint_desc.type, endpoint2.type);
>  	endpoint_desc.id = endpoint2.id;
>  	endpoint = fsl_mc_device_lookup(&endpoint_desc, mc_bus_dev);
> +	if (endpoint)
> +		return endpoint;
>  
>  	/*
>  	 * We know that the device has an endpoint because we verified by
> @@ -973,17 +976,13 @@ struct fsl_mc_device *fsl_mc_get_endpoint(struct fsl_mc_device *mc_dev,
>  	 * yet discovered by the fsl-mc bus, thus the lookup returned NULL.
>  	 * Force a rescan of the devices in this container and retry the lookup.
>  	 */
> -	if (!endpoint) {
> -		struct fsl_mc_bus *mc_bus = to_fsl_mc_bus(mc_bus_dev);
> -
> -		if (mutex_trylock(&mc_bus->scan_mutex)) {
> -			err = dprc_scan_objects(mc_bus_dev, true);
> -			mutex_unlock(&mc_bus->scan_mutex);
> -		}
> -
> -		if (err < 0)
> -			return ERR_PTR(err);
> +	mc_bus = to_fsl_mc_bus(mc_bus_dev);
> +	if (mutex_trylock(&mc_bus->scan_mutex)) {
> +		err = dprc_scan_objects(mc_bus_dev, true);
> +		mutex_unlock(&mc_bus->scan_mutex);
>  	}
> +	if (err < 0)
> +		return ERR_PTR(err);
>  
>  	endpoint = fsl_mc_device_lookup(&endpoint_desc, mc_bus_dev);
>  	/*
> -- 
> 2.25.1
> 

  parent reply	other threads:[~2025-07-18 14:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-17  2:23 [PATCH net v2 1/3] bus: fsl-mc: Fix potential double device reference in fsl_mc_get_endpoint() Ma Ke
2025-07-17  2:23 ` [PATCH net v2 2/3] dpaa2-eth: Fix device reference count leak in MAC endpoint handling Ma Ke
2025-07-17 13:26   ` Ioana Ciornei
2025-07-18 14:42   ` Simon Horman
2025-07-17  2:23 ` [PATCH net v2 3/3] dpaa2-switch: " Ma Ke
2025-07-17 13:23   ` Ioana Ciornei
2025-07-18 14:43   ` Simon Horman
2025-07-17 13:15 ` [PATCH net v2 1/3] bus: fsl-mc: Fix potential double device reference in fsl_mc_get_endpoint() Ioana Ciornei
2025-07-18 14:39 ` Simon Horman [this message]
2025-07-19  0:20 ` patchwork-bot+netdevbpf

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=20250718143923.GF2459@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=kuba@kernel.org \
    --cc=laurentiu.tudor@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=make24@iscas.ac.cn \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).