| 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
| ||
|
Message-ID: <aG664X6LuA6eZXXm@google.com>
Date: Wed, 9 Jul 2025 18:54:25 +0000
From: Benson Leung <bleung@...gle.com>
To: Radu Vele <raduvele@...gle.com>
Cc: Benson Leung <bleung@...omium.org>,
Abhishek Pandit-Subedi <abhishekpandit@...omium.org>,
Jameson Thies <jthies@...gle.com>,
Andrei Kuchynski <akuchynski@...omium.org>,
Tzung-Bi Shih <tzungbi@...nel.org>, chrome-platform@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port
Hi Radu,
On Wed, Jul 09, 2025 at 01:22:32PM +0000, Radu Vele wrote:
> Add a lock associated to each port to protect port data against
> concurrent access. Concurrency may result from sysfs commands
> and ec events.
>
> Signed-off-by: Radu Vele <raduvele@...gle.com>
> ---
> drivers/platform/chrome/cros_ec_typec.c | 38 +++++++++++++++++++------
> drivers/platform/chrome/cros_ec_typec.h | 3 ++
> 2 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 289429ef959f..bbb9e02624b8 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -8,6 +8,7 @@
>
> #include <linux/acpi.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_data/cros_ec_commands.h>
> #include <linux/platform_data/cros_usbpd_notify.h>
> @@ -54,8 +55,11 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m
> .mode_to_enter = CROS_EC_ALTMODE_USB4
> };
>
> - return cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
> + mutex_lock(&port->lock);
> + int ret = cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL,
> &req, sizeof(req), NULL, 0);
Please declare "int ret" at the top of the block, as per other examples of it
in this file.
You have it in the middle of a block right now.
> + mutex_unlock(&port->lock);
> + return ret;
> }
>
> static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type)
> @@ -70,6 +74,8 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r
> if (!data->pd_ctrl_ver)
> return -EOPNOTSUPP;
>
> + mutex_lock(&port->lock);
> +
> /* First query the state */
> req.port = port->port_num;
> req.role = USB_PD_CTRL_ROLE_NO_CHANGE;
> @@ -79,7 +85,7 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r
> ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL,
> &req, sizeof(req), &resp, sizeof(resp));
> if (ret < 0)
> - return ret;
> + goto unlock;
>
> switch (swap_type) {
> case USB_PD_CTRL_SWAP_DATA:
> @@ -92,18 +98,21 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r
> break;
> default:
> dev_warn(data->dev, "Unsupported role swap type %d", swap_type);
> - return -EOPNOTSUPP;
> + ret = -EOPNOTSUPP;
> + goto unlock;
> }
>
> - if (role == target_role)
> - return 0;
> + if (role == target_role) {
> + ret = 0;
> + goto unlock;
> + }
>
> req.swap = swap_type;
> ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL,
> &req, sizeof(req), &resp, sizeof(resp));
>
> if (ret < 0)
> - return ret;
> + goto unlock;
>
> switch (swap_type) {
> case USB_PD_CTRL_SWAP_DATA:
> @@ -117,8 +126,11 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r
> TYPEC_SINK);
> break;
> }
> + ret = 0;
>
> - return 0;
> +unlock:
> + mutex_unlock(&port->lock);
> + return ret;
> }
>
> static int cros_typec_dr_swap(struct typec_port *port, enum typec_data_role role)
> @@ -370,6 +382,7 @@ static void cros_unregister_ports(struct cros_typec_data *typec)
> if (!typec->ports[i])
> continue;
>
> + mutex_lock(&typec->ports[i]->lock);
> cros_typec_remove_partner(typec, i);
> cros_typec_remove_cable(typec, i);
>
> @@ -378,6 +391,8 @@ static void cros_unregister_ports(struct cros_typec_data *typec)
> typec_mux_put(typec->ports[i]->mux);
> cros_typec_unregister_port_altmodes(typec->ports[i]);
> typec_unregister_port(typec->ports[i]->port);
> + mutex_unlock(&typec->ports[i]->lock);
> + mutex_destroy(&typec->ports[i]->lock);
> }
> }
>
> @@ -472,6 +487,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
> goto unregister_ports;
> }
>
> + mutex_init(&cros_port->lock);
> cros_port->port_num = port_num;
> cros_port->typec_data = typec;
> typec->ports[port_num] = cros_port;
> @@ -1232,6 +1248,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> return -EINVAL;
> }
>
> + mutex_lock(&typec->ports[port_num]->lock);
> req.port = port_num;
> req.role = USB_PD_CTRL_ROLE_NO_CHANGE;
> req.mux = USB_PD_CTRL_MUX_NO_CHANGE;
> @@ -1241,7 +1258,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> EC_CMD_USB_PD_CONTROL, &req, sizeof(req),
> &resp, sizeof(resp));
> if (ret < 0)
> - return ret;
> + goto unlock;
>
> /* Update the switches if they exist, according to requested state */
> ret = cros_typec_configure_mux(typec, port_num, &resp);
> @@ -1263,7 +1280,10 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> if (typec->typec_cmd_supported)
> cros_typec_handle_status(typec, port_num);
>
> - return 0;
> + ret = 0;
> +unlock:
> + mutex_unlock(&typec->ports[port_num]->lock);
> + return ret;
> }
>
> static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
> diff --git a/drivers/platform/chrome/cros_ec_typec.h b/drivers/platform/chrome/cros_ec_typec.h
> index f9c31f04c102..d0a8a12ec91a 100644
> --- a/drivers/platform/chrome/cros_ec_typec.h
> +++ b/drivers/platform/chrome/cros_ec_typec.h
> @@ -82,6 +82,9 @@ struct cros_typec_port {
> struct usb_power_delivery_capabilities *partner_sink_caps;
>
> struct cros_typec_data *typec_data;
> +
> + /* Mutex to protect port data against concurrent access */
> + struct mutex lock;
> };
>
> #endif /* __CROS_EC_TYPEC__ */
> --
> 2.50.0.727.gbf7dc18ff4-goog
>
Powered by blists - more mailing lists