netdev - Re: [PATCH net v2 1/3] bus: fsl-mc: Fix potential double device reference in fsl_mc_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250718143923.GF2459@horms.kernel.org>
Date: Fri, 18 Jul 2025 15:39:23 +0100
From: Simon Horman <horms@...nel.org>
To: Ma Ke <make24@...as.ac.cn>
Cc: ioana.ciornei@....com, davem@...emloft.net, andrew+netdev@...n.ch,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, stable@...r.kernel.org,
	Laurentiu Tudor <laurentiu.tudor@....com>
Subject: Re: [PATCH net v2 1/3] bus: fsl-mc: Fix potential double device
 reference in fsl_mc_get_endpoint()

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@...r.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@...as.ac.cn>

Suggested-by: Simon Horman <horms@...nel.org>
Reviewed-by: Simon Horman <horms@...nel.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
>