Skip to content

Commit

Permalink
usb: typec: ucsi: Fix AB BA lock inversion
Browse files Browse the repository at this point in the history
commit 0ff0705 upstream.

Lockdep reports an AB BA lock inversion between ucsi_init() and
ucsi_handle_connector_change():

AB order:

1. ucsi_init takes ucsi->ppm_lock (it runs with that locked for the
   duration of the function)
2. usci_init eventually end up calling ucsi_register_displayport,
   which takes ucsi_connector->lock

BA order:

1. ucsi_handle_connector_change work is started, takes ucsi_connector->lock
2. ucsi_handle_connector_change calls ucsi_send_command which takes
   ucsi->ppm_lock

The ppm_lock really only needs to be hold during 2 functions:
ucsi_reset_ppm() and ucsi_run_command().

This commit fixes the AB BA lock inversion by making ucsi_init drop the
ucsi->ppm_lock before it starts registering ports; and replacing any
ucsi_run_command() calls after this point with ucsi_send_command()
(which is a wrapper around run_command taking the lock while handling
the command).

Some of the replacing of ucsi_run_command with ucsi_send_command
in the helpers used during port registration also fixes a number of
code paths after registration which call ucsi_run_command() without
holding the ppm_lock:
1. ucsi_altmode_update_active() call in ucsi/displayport.c
2. ucsi_register_altmodes() call from ucsi_handle_connector_change()
   (through ucsi_partner_change())

Cc: stable@vger.kernel.org
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-2-hdegoede@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
jwrdegoede authored and gregkh committed Sep 3, 2020
1 parent 53965c7 commit 4b59dab
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions drivers/usb/typec/ucsi/ucsi.c
Expand Up @@ -205,7 +205,7 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
int i;

command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(con->num);
ret = ucsi_run_command(con->ucsi, command, &cur, sizeof(cur));
ret = ucsi_send_command(con->ucsi, command, &cur, sizeof(cur));
if (ret < 0) {
if (con->ucsi->version > 0x0100) {
dev_err(con->ucsi->dev,
Expand Down Expand Up @@ -354,7 +354,7 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_ALTMODE_OFFSET(i);
len = ucsi_run_command(con->ucsi, command, &alt, sizeof(alt));
len = ucsi_send_command(con->ucsi, command, &alt, sizeof(alt));
/*
* We are collecting all altmodes first and then registering.
* Some type-C device will return zero length data beyond last
Expand Down Expand Up @@ -431,7 +431,7 @@ static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_ALTMODE_OFFSET(i);
len = ucsi_run_command(con->ucsi, command, alt, sizeof(alt));
len = ucsi_send_command(con->ucsi, command, alt, sizeof(alt));
if (len <= 0)
return len;

Expand Down Expand Up @@ -904,7 +904,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
/* Get connector capability */
command = UCSI_GET_CONNECTOR_CAPABILITY;
command |= UCSI_CONNECTOR_NUMBER(con->num);
ret = ucsi_run_command(ucsi, command, &con->cap, sizeof(con->cap));
ret = ucsi_send_command(ucsi, command, &con->cap, sizeof(con->cap));
if (ret < 0)
return ret;

Expand Down Expand Up @@ -953,8 +953,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)

/* Get the status */
command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
ret = ucsi_run_command(ucsi, command, &con->status,
sizeof(con->status));
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;
Expand Down Expand Up @@ -1044,6 +1043,8 @@ int ucsi_init(struct ucsi *ucsi)
goto err_reset;
}

mutex_unlock(&ucsi->ppm_lock);

/* Register all connectors */
for (i = 0; i < ucsi->cap.num_connectors; i++) {
ret = ucsi_register_port(ucsi, i);
Expand All @@ -1054,12 +1055,10 @@ int ucsi_init(struct ucsi *ucsi)
/* Enable all notifications */
ucsi->ntfy = UCSI_ENABLE_NTFY_ALL;
command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
ret = ucsi_run_command(ucsi, command, NULL, 0);
ret = ucsi_send_command(ucsi, command, NULL, 0);
if (ret < 0)
goto err_unregister;

mutex_unlock(&ucsi->ppm_lock);

return 0;

err_unregister:
Expand All @@ -1071,6 +1070,7 @@ int ucsi_init(struct ucsi *ucsi)
con->port = NULL;
}

mutex_lock(&ucsi->ppm_lock);
err_reset:
ucsi_reset_ppm(ucsi);
err:
Expand Down

0 comments on commit 4b59dab

Please sign in to comment.