Re: [PATCH] usb: typec: rt1719: unregister the port on probe failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: typec: rt1719: missing typec_unregister_partner() in rt1719_probe()
--

commit aeb856a2c764adde23b2f8df2e3537e93dbc9fee
Author: Wentao Liang <vulab@iscas.ac.cn>

usb: typec: rt1719: unregister the port on probe failure

This commit adds an err_unregister_port error path label in rt1719_probe()
to call typec_unregister_port(data->port) if rt1719_init_attach_state() or
rt1719_irq_init() fails after the Type-C port has been registered.

> diff --git a/drivers/usb/typec/rt1719.c b/drivers/usb/typec/rt1719.c
> index 0b0c23a0b014b..61fa865637c70 100644
> --- a/drivers/usb/typec/rt1719.c
> +++ b/drivers/usb/typec/rt1719.c
> @@ -902,13 +902,13 @@ static int rt1719_probe(struct i2c_client *i2c)
>  	ret = rt1719_init_attach_state(data);
>  	if (ret) {
>  		dev_err(&i2c->dev, "Failed to init attach state (%d)\n", ret);
> -		goto err_role_put;
> +		goto err_unregister_port;
>  	}
>  
>  	ret = rt1719_irq_init(data);
>  	if (ret) {
>  		dev_err(&i2c->dev, "Failed to init irq\n");
> -		goto err_role_put;
> +		goto err_unregister_port;
>  	}
>  
>  	fwnode_handle_put(fwnode);
> @@ -917,6 +917,8 @@ static int rt1719_probe(struct i2c_client *i2c)
>  
>  	return 0;
>  
> +err_unregister_port:
> +	typec_unregister_port(data->port);

[Severity: High]
Does err_unregister_port in drivers/usb/typec/rt1719.c:rt1719_probe() also
need to call typec_unregister_partner(data->partner) before
typec_unregister_port(data->port)?

Before rt1719_irq_init(data) is called, rt1719_init_attach_state(data) runs:

drivers/usb/typec/rt1719.c:rt1719_init_attach_state() {
	...
	if (conn_info & RT1719_ATTACHDEV_MASK)
		rt1719_attach(data);

	if (conn_info & RT1719_PE_EXP_CONTRACT) {
		rt1719_update_source_pdos(data);
		rt1719_update_pwr_opmode(data);
	}
	...
}

If a partner is already attached at probe time, rt1719_attach() or
rt1719_update_pwr_opmode() calls rt1719_register_partner(data), which
registers data->partner as a child device of data->port via
typec_register_partner(data->port, &data->partner_desc) with
partner->identity pointing to &data->partner_ident.

If rt1719_irq_init(data) then fails and jumps to err_unregister_port,
typec_unregister_port(data->port) is called while data->partner is still
registered. Because typec_unregister_port() only calls
device_unregister(&port->dev) and does not unregister child partner devices,
data->partner remains registered in sysfs holding its parent reference on
data->port->dev (leaking both data->partner and data->port), and its
partner->identity pointer dangles into the devm-allocated struct rt1719_data
once rt1719_probe() returns an error.

Since typec_unregister_partner() safely ignores NULL or ERR_PTR pointers,
should typec_unregister_partner(data->partner) be called right before
typec_unregister_port(data->port), matching rt1719_remove()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104021514.1468596-1-vulab@iscas.ac.cn?part=1