Re: [PATCH] usb: typec: anx7411: fix fwnode_handle reference leak

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Sat, Nov 16, 2024 at 05:51:24PM +0900, Joe Hattori wrote:
> An fwnode_handle is obtained with an incremented refcount in
> anx7411_typec_port_probe(), however the refcount is not decremented in
> the error path or in the .remove() function. Therefore call
> fwnode_handle_put() accordingly.
> 
> Fixes: fe6d8a9c8e64 ("usb: typec: anx7411: Add Analogix PD ANX7411 support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
> ---
>  drivers/usb/typec/anx7411.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/typec/anx7411.c b/drivers/usb/typec/anx7411.c
> index 7e61c3ac8777..d3c5d8f410ca 100644
> --- a/drivers/usb/typec/anx7411.c
> +++ b/drivers/usb/typec/anx7411.c
> @@ -1023,6 +1023,12 @@ static void anx7411_port_unregister_altmodes(struct typec_altmode **adev)
>  		}
>  }
>  
> +static void anx7411_port_unregister(struct typec_params *typecp)
> +{
> +	fwnode_handle_put(typecp->caps.fwnode);
> +	anx7411_port_unregister_altmodes(typecp->port_amode);
> +}

Why not remove the port here while at it.
Otherwise this LGTM.

>  static int anx7411_usb_mux_set(struct typec_mux_dev *mux,
>  			       struct typec_mux_state *state)
>  {
> @@ -1158,34 +1164,34 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
>  	ret = fwnode_property_read_string(fwnode, "power-role", &buf);
>  	if (ret) {
>  		dev_err(dev, "power-role not found: %d\n", ret);
> -		return ret;
> +		goto put_fwnode;
>  	}
>  
>  	ret = typec_find_port_power_role(buf);
>  	if (ret < 0)
> -		return ret;
> +		goto put_fwnode;
>  	cap->type = ret;
>  
>  	ret = fwnode_property_read_string(fwnode, "data-role", &buf);
>  	if (ret) {
>  		dev_err(dev, "data-role not found: %d\n", ret);
> -		return ret;
> +		goto put_fwnode;
>  	}
>  
>  	ret = typec_find_port_data_role(buf);
>  	if (ret < 0)
> -		return ret;
> +		goto put_fwnode;
>  	cap->data = ret;
>  
>  	ret = fwnode_property_read_string(fwnode, "try-power-role", &buf);
>  	if (ret) {
>  		dev_err(dev, "try-power-role not found: %d\n", ret);
> -		return ret;
> +		goto put_fwnode;
>  	}
>  
>  	ret = typec_find_power_role(buf);
>  	if (ret < 0)
> -		return ret;
> +		goto put_fwnode;
>  	cap->prefer_role = ret;
>  
>  	/* Get source pdos */
> @@ -1197,7 +1203,7 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
>  						     typecp->src_pdo_nr);
>  		if (ret < 0) {
>  			dev_err(dev, "source cap validate failed: %d\n", ret);
> -			return -EINVAL;
> +			goto put_fwnode;
>  		}
>  
>  		typecp->caps_flags |= HAS_SOURCE_CAP;
> @@ -1211,7 +1217,7 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
>  						     typecp->sink_pdo_nr);
>  		if (ret < 0) {
>  			dev_err(dev, "sink cap validate failed: %d\n", ret);
> -			return -EINVAL;
> +			goto put_fwnode;
>  		}
>  
>  		for (i = 0; i < typecp->sink_pdo_nr; i++) {
> @@ -1255,13 +1261,18 @@ static int anx7411_typec_port_probe(struct anx7411_data *ctx,
>  		ret = PTR_ERR(ctx->typec.port);
>  		ctx->typec.port = NULL;
>  		dev_err(dev, "Failed to register type c port %d\n", ret);
> -		return ret;
> +		goto put_fwnode;
>  	}
>  
>  	typec_port_register_altmodes(ctx->typec.port, NULL, ctx,
>  				     ctx->typec.port_amode,
>  				     MAX_ALTMODE);
>  	return 0;
> +
> +put_fwnode:
> +	fwnode_handle_put(fwnode);
> +
> +	return ret;
>  }
>  
>  static int anx7411_typec_check_connection(struct anx7411_data *ctx)
> @@ -1528,7 +1539,7 @@ static int anx7411_i2c_probe(struct i2c_client *client)
>  
>  free_typec_port:
>  	typec_unregister_port(plat->typec.port);
> -	anx7411_port_unregister_altmodes(plat->typec.port_amode);
> +	anx7411_port_unregister(&plat->typec);
>  
>  free_typec_switch:
>  	anx7411_unregister_switch(plat);
> @@ -1562,7 +1573,7 @@ static void anx7411_i2c_remove(struct i2c_client *client)
>  	if (plat->typec.port)
>  		typec_unregister_port(plat->typec.port);
>  
> -	anx7411_port_unregister_altmodes(plat->typec.port_amode);
> +	anx7411_port_unregister(&plat->typec);
>  }
>  
>  static const struct i2c_device_id anx7411_id[] = {

thanks,

-- 
heikki