linux-kernel - Re: [PATCH] platform/chrome: cros_ec_typec: Unregister partner on error

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] [day] [month] [year] [list]

Message-ID: <bfe33351-fc64-81a8-4cc5-e8122a2a44e1@collabora.com>
Date:   Wed, 22 Jul 2020 17:28:16 +0200
From:   Enric Balletbo i Serra <enric.balletbo@...labora.com>
To:     Prashant Malani <pmalani@...omium.org>,
        linux-kernel@...r.kernel.org
Cc:     heikki.krogerus@...ux.intel.com,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>
Subject: Re: [PATCH] platform/chrome: cros_ec_typec: Unregister partner on
 error

Hi Prashant,

Thank you for your patch.

On 16/7/20 1:49, Prashant Malani wrote:
> When port update is called during probe(), any error with setting the
> Type C muxes results in an errno being returned to probe(), which promptly
> returns that itself. Ensure that we unregister any registered partners
> when doing so, to prevent orphaned partners on the Type C connector
> class framework.
> 
> Move the cros_typec_add_partner() and cros_typec_remove_partner() code
> together to higher up in the file, so that they are together, and we can
> call cros_typec_remove_partner() from cros_unregister_ports().
> 
> Fixes: 7e7def15fa4b ("platform/chrome: cros_ec_typec: Add USB mux control")
> Signed-off-by: Prashant Malani <pmalani@...omium.org>

Queued for 5.9

> ---
>  drivers/platform/chrome/cros_ec_typec.c | 83 +++++++++++++------------
>  1 file changed, 42 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 0c041b79cbba..43e7cf367a07 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -140,6 +140,47 @@ static int cros_typec_get_switch_handles(struct cros_typec_port *port,
>  	return -ENODEV;
>  }
>  
> +static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
> +				  bool pd_en)
> +{
> +	struct cros_typec_port *port = typec->ports[port_num];
> +	struct typec_partner_desc p_desc = {
> +		.usb_pd = pd_en,
> +	};
> +	int ret = 0;
> +
> +	/*
> +	 * Fill an initial PD identity, which will then be updated with info
> +	 * from the EC.
> +	 */
> +	p_desc.identity = &port->p_identity;
> +
> +	port->partner = typec_register_partner(port->port, &p_desc);
> +	if (IS_ERR(port->partner)) {
> +		ret = PTR_ERR(port->partner);
> +		port->partner = NULL;
> +	}
> +
> +	return ret;
> +}
> +
> +static void cros_typec_remove_partner(struct cros_typec_data *typec,
> +				     int port_num)
> +{
> +	struct cros_typec_port *port = typec->ports[port_num];
> +
> +	port->state.alt = NULL;
> +	port->state.mode = TYPEC_STATE_USB;
> +	port->state.data = NULL;
> +
> +	usb_role_switch_set_role(port->role_sw, USB_ROLE_NONE);
> +	typec_switch_set(port->ori_sw, TYPEC_ORIENTATION_NONE);
> +	typec_mux_set(port->mux, &port->state);
> +
> +	typec_unregister_partner(port->partner);
> +	port->partner = NULL;
> +}
> +
>  static void cros_unregister_ports(struct cros_typec_data *typec)
>  {
>  	int i;
> @@ -147,6 +188,7 @@ static void cros_unregister_ports(struct cros_typec_data *typec)
>  	for (i = 0; i < typec->num_ports; i++) {
>  		if (!typec->ports[i])
>  			continue;
> +		cros_typec_remove_partner(typec, i);
>  		usb_role_switch_put(typec->ports[i]->role_sw);
>  		typec_switch_put(typec->ports[i]->ori_sw);
>  		typec_mux_put(typec->ports[i]->mux);
> @@ -285,47 +327,6 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
>  	return ret;
>  }
>  
> -static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
> -				  bool pd_en)
> -{
> -	struct cros_typec_port *port = typec->ports[port_num];
> -	struct typec_partner_desc p_desc = {
> -		.usb_pd = pd_en,
> -	};
> -	int ret = 0;
> -
> -	/*
> -	 * Fill an initial PD identity, which will then be updated with info
> -	 * from the EC.
> -	 */
> -	p_desc.identity = &port->p_identity;
> -
> -	port->partner = typec_register_partner(port->port, &p_desc);
> -	if (IS_ERR(port->partner)) {
> -		ret = PTR_ERR(port->partner);
> -		port->partner = NULL;
> -	}
> -
> -	return ret;
> -}
> -
> -static void cros_typec_remove_partner(struct cros_typec_data *typec,
> -				     int port_num)
> -{
> -	struct cros_typec_port *port = typec->ports[port_num];
> -
> -	port->state.alt = NULL;
> -	port->state.mode = TYPEC_STATE_USB;
> -	port->state.data = NULL;
> -
> -	usb_role_switch_set_role(port->role_sw, USB_ROLE_NONE);
> -	typec_switch_set(port->ori_sw, TYPEC_ORIENTATION_NONE);
> -	typec_mux_set(port->mux, &port->state);
> -
> -	typec_unregister_partner(port->partner);
> -	port->partner = NULL;
> -}
> -
>  static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
>  		int port_num, struct ec_response_usb_pd_control *resp)
>  {
>