Re: [PATCH 2/8] usb: typec: tcpm: add device managed port registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: typec: tcpm: leak of port->pd_list[i] on
  tcpm_register_port() error paths
--

commit 4732c69a74cc6f816813133462e97f8deba7b034
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

usb: typec: tcpm: add device managed port registration

This commit adds devm_tcpm_register_port() to support device-managed TCPM port
registration.

> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 07d2a03d2b8d7..7d6378acf0183 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -9013,6 +9013,31 @@ void tcpm_unregister_port(struct tcpm_port *port)
>  }
>  EXPORT_SYMBOL_GPL(tcpm_unregister_port);
>  
> +static void devm_tcpm_unregister_port(void *data)
> +{
> +	struct tcpm_port *port = data;
> +
> +	tcpm_unregister_port(port);
> +}
> +
> +struct tcpm_port *devm_tcpm_register_port(struct device *dev,
> +					  struct tcpc_dev *tcpc)
> +{
> +	struct tcpm_port *result;
> +	int ret;
> +
> +	result = tcpm_register_port(dev, tcpc);
> +	if (IS_ERR(result))
> +		return result;

[Severity: Medium]
Does this leak port->pd_list[i] when tcpm_register_port() fails before
tcpm_port_register_pd()?

To prepare for devm_tcpm_register_port(), the preceding commit "usb: typec:
tcpm: allocate pd_list entries non device managed" switched port->pd_list[i]
in tcpm_fw_get_caps() to non-managed kzalloc_obj() and freed the entries in
tcpm_port_unregister_pd().

However, if tcpm_fw_get_caps() fails after allocating one or more
port->pd_list[i] entries, or if a subsequent step in tcpm_register_port()
such as usb_role_switch_get() returning -EPROBE_DEFER fails and jumps to
out_destroy_wq or out_role_sw_put, tcpm_port_unregister_pd() is bypassed and
devm_tcpm_register_port() returns before registering its cleanup action:

drivers/usb/typec/tcpm/tcpm.c:tcpm_register_port() {
	...
	port->role_sw = fwnode_usb_role_switch_get(tcpc->fwnode);
	if (!port->role_sw)
		port->role_sw = usb_role_switch_get(port->dev);
	if (IS_ERR(port->role_sw)) {
		err = PTR_ERR(port->role_sw);
		goto out_destroy_wq;
	}
	...
out_unregister_pd:
	tcpm_port_unregister_pd(port);
out_role_sw_put:
	usb_role_switch_put(port->role_sw);
out_destroy_wq:
	tcpm_debugfs_exit(port);
	kthread_destroy_worker(port->wq);
	return ERR_PTR(err);
}

Each failed or deferred probe on a PD-capable port then leaks the allocated
port->pd_list[i] entries.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-fusb302-probe-cleanup-v1-0-e5c65a8c58c8@collabora.com?part=2