Subject: Re: [PATCH net v2 1/3] bus: fsl-mc: Fix potential double device reference in fsl_mc_get_endpoint()

From: Simon Horman <[email-masked]>
Date: Fri, 18 Jul 2025 15:39:23 +0100

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: [email-masked]
> 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 <[email-masked]>

Suggested-by: Simon Horman <[email-masked]>
Reviewed-by: Simon Horman <[email-masked]>

> ---
>  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
>