linux-kernel - Re: [PATCH] usb: typec: ucsi: Fix missing typec_unregister

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: <aSVoYbBqC_h3mqcA@kuha>
Date: Tue, 25 Nov 2025 10:27:13 +0200
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Haotian Zhang <vulab@...as.ac.cn>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: typec: ucsi: Fix missing typec_unregister_port on
 error paths

Mon, Nov 24, 2025 at 08:20:30PM +0800, Haotian Zhang kirjoitti:
> The ucsi_register_port()  registers a Type-C port with
> typec_register_port(), but several error paths after a successful
> registration returned directly to cleanup without calling
> typec_unregister_port(). This lead to a potential resource leak.
> 
> Add a common error-unwind path that calls typec_unregister_port()
> for all failures to ensure proper cleanup.
> 
> Fixes: c1b0bc2dabfa ("usb: typec: Add support for UCSI interface")
> Signed-off-by: Haotian Zhang <vulab@...as.ac.cn>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 3f568f790f39..096c4911e8bf 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1656,14 +1656,14 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
>  	if (ret) {
>  		dev_err(ucsi->dev, "con%d: failed to register alt modes\n",
>  			con->num);
> -		goto out;
> +		goto out_unregister;
>  	}
>  
>  	/* Get the status */
>  	ret = ucsi_get_connector_status(con, false);
>  	if (ret) {
>  		dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
> -		goto out;
> +		goto out_unregister;
>  	}
>  
>  	if (ucsi->ops->connector_status)
> @@ -1717,6 +1717,10 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
>  
>  	trace_ucsi_register_port(con->num, con);
>  
> +	goto out;
> +
> +out_unregister:
> +	typec_unregister_port(con->port);

No. We don't destroy the port if status or alt modes fail. The alt
modes can't be considered critical because the firmware quite often
claims alt modes are supported even though they are not. That should
be explained, if not in documentation, then at least in the commit
messages.

Try something like this instead.

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ec6c8f928dda..8de8935320be 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1682,6 +1682,7 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
        if (ret) {
                dev_err(ucsi->dev, "con%d: failed to register alt modes\n",
                        con->num);
+               ret = 0;
                goto out;
        }
 
@@ -1689,6 +1690,7 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
        ret = ucsi_get_connector_status(con, false);
        if (ret) {
                dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
+               ret = 0;
                goto out;
        }


And the commit you want to fix is b9aa02ca39a4 ("usb: typec: ucsi: Add
polling mechanism for partner tasks like alt mode checking").

thanks,

-- 
heikki