| From bed97b30968ba354035a020989df0623e52b5536 Mon Sep 17 00:00:00 2001 |
| From: Hans de Goede <hdegoede@redhat.com> |
| Date: Sun, 9 Aug 2020 16:19:04 +0200 |
| Subject: [PATCH] usb: typec: ucsi: Hold con->lock for the entire duration of |
| ucsi_register_port() |
| |
| commit bed97b30968ba354035a020989df0623e52b5536 upstream. |
| |
| Commit 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race |
| during registration") made the ucsi code hold con->lock in |
| ucsi_register_displayport(). But we really don't want any interactions |
| with the connector to run before the port-registration process is fully |
| complete. |
| |
| This commit moves the taking of con->lock from ucsi_register_displayport() |
| into ucsi_register_port() to achieve this. |
| |
| Cc: stable@vger.kernel.org |
| Fixes: 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race during registration") |
| Signed-off-by: Hans de Goede <hdegoede@redhat.com> |
| Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> |
| Reviewed-by: Guenter Roeck <linux@roeck-us.net> |
| Link: https://lore.kernel.org/r/20200809141904.4317-5-hdegoede@redhat.com |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c |
| index 048381c058a5..261131c9e37c 100644 |
| --- a/drivers/usb/typec/ucsi/displayport.c |
| +++ b/drivers/usb/typec/ucsi/displayport.c |
| @@ -288,8 +288,6 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con, |
| struct typec_altmode *alt; |
| struct ucsi_dp *dp; |
| |
| - mutex_lock(&con->lock); |
| - |
| /* We can't rely on the firmware with the capabilities. */ |
| desc->vdo |= DP_CAP_DP_SIGNALING | DP_CAP_RECEPTACLE; |
| |
| @@ -298,15 +296,12 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con, |
| desc->vdo |= all_assignments << 16; |
| |
| alt = typec_port_register_altmode(con->port, desc); |
| - if (IS_ERR(alt)) { |
| - mutex_unlock(&con->lock); |
| + if (IS_ERR(alt)) |
| return alt; |
| - } |
| |
| dp = devm_kzalloc(&alt->dev, sizeof(*dp), GFP_KERNEL); |
| if (!dp) { |
| typec_unregister_altmode(alt); |
| - mutex_unlock(&con->lock); |
| return ERR_PTR(-ENOMEM); |
| } |
| |
| @@ -319,7 +314,5 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con, |
| alt->ops = &ucsi_displayport_ops; |
| typec_altmode_set_drvdata(alt, dp); |
| |
| - mutex_unlock(&con->lock); |
| - |
| return alt; |
| } |
| diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c |
| index 8a35144211e5..e680fcfdee60 100644 |
| --- a/drivers/usb/typec/ucsi/ucsi.c |
| +++ b/drivers/usb/typec/ucsi/ucsi.c |
| @@ -898,12 +898,15 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) |
| con->num = index + 1; |
| con->ucsi = ucsi; |
| |
| + /* Delay other interactions with the con until registration is complete */ |
| + mutex_lock(&con->lock); |
| + |
| /* Get connector capability */ |
| command = UCSI_GET_CONNECTOR_CAPABILITY; |
| command |= UCSI_CONNECTOR_NUMBER(con->num); |
| ret = ucsi_send_command(ucsi, command, &con->cap, sizeof(con->cap)); |
| if (ret < 0) |
| - return ret; |
| + goto out; |
| |
| if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP) |
| cap->data = TYPEC_PORT_DRD; |
| @@ -935,26 +938,32 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) |
| |
| ret = ucsi_register_port_psy(con); |
| if (ret) |
| - return ret; |
| + goto out; |
| |
| /* Register the connector */ |
| con->port = typec_register_port(ucsi->dev, cap); |
| - if (IS_ERR(con->port)) |
| - return PTR_ERR(con->port); |
| + if (IS_ERR(con->port)) { |
| + ret = PTR_ERR(con->port); |
| + goto out; |
| + } |
| |
| /* Alternate modes */ |
| ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_CON); |
| - if (ret) |
| + if (ret) { |
| dev_err(ucsi->dev, "con%d: failed to register alt modes\n", |
| con->num); |
| + goto out; |
| + } |
| |
| /* Get the status */ |
| command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); |
| ret = ucsi_send_command(ucsi, command, &con->status, sizeof(con->status)); |
| if (ret < 0) { |
| dev_err(ucsi->dev, "con%d: failed to get status\n", con->num); |
| - return 0; |
| + ret = 0; |
| + goto out; |
| } |
| + ret = 0; /* ucsi_send_command() returns length on success */ |
| |
| switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) { |
| case UCSI_CONSTAT_PARTNER_TYPE_UFP: |
| @@ -979,17 +988,21 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) |
| |
| if (con->partner) { |
| ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_SOP); |
| - if (ret) |
| + if (ret) { |
| dev_err(ucsi->dev, |
| "con%d: failed to register alternate modes\n", |
| con->num); |
| - else |
| + ret = 0; |
| + } else { |
| ucsi_altmode_update_active(con); |
| + } |
| } |
| |
| trace_ucsi_register_port(con->num, &con->status); |
| |
| - return 0; |
| +out: |
| + mutex_unlock(&con->lock); |
| + return ret; |
| } |
| |
| /** |
| -- |
| 2.27.0 |
| |