Commit 0ff0705a authored by Hans de Goede's avatar Hans de Goede Committed by Greg Kroah-Hartman

usb: typec: ucsi: Fix AB BA lock inversion

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: default avatarHans de Goede <hdegoede@redhat.com>
Acked-by: default avatarHeikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: default avatarGuenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20200809141904.4317-2-hdegoede@redhat.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 7a2f2974
...@@ -205,7 +205,7 @@ void ucsi_altmode_update_active(struct ucsi_connector *con) ...@@ -205,7 +205,7 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
int i; int i;
command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(con->num); 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 (ret < 0) {
if (con->ucsi->version > 0x0100) { if (con->ucsi->version > 0x0100) {
dev_err(con->ucsi->dev, dev_err(con->ucsi->dev,
...@@ -354,7 +354,7 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient) ...@@ -354,7 +354,7 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
command |= UCSI_GET_ALTMODE_RECIPIENT(recipient); command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num); command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_ALTMODE_OFFSET(i); 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. * We are collecting all altmodes first and then registering.
* Some type-C device will return zero length data beyond last * Some type-C device will return zero length data beyond last
...@@ -431,7 +431,7 @@ static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient) ...@@ -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_RECIPIENT(recipient);
command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num); command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
command |= UCSI_GET_ALTMODE_OFFSET(i); 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) if (len <= 0)
return len; return len;
...@@ -904,7 +904,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) ...@@ -904,7 +904,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
/* Get connector capability */ /* Get connector capability */
command = UCSI_GET_CONNECTOR_CAPABILITY; command = UCSI_GET_CONNECTOR_CAPABILITY;
command |= UCSI_CONNECTOR_NUMBER(con->num); 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) if (ret < 0)
return ret; return ret;
...@@ -953,8 +953,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) ...@@ -953,8 +953,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
/* Get the status */ /* Get the status */
command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
ret = ucsi_run_command(ucsi, command, &con->status, ret = ucsi_send_command(ucsi, command, &con->status, sizeof(con->status));
sizeof(con->status));
if (ret < 0) { if (ret < 0) {
dev_err(ucsi->dev, "con%d: failed to get status\n", con->num); dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
return 0; return 0;
...@@ -1044,6 +1043,8 @@ static int ucsi_init(struct ucsi *ucsi) ...@@ -1044,6 +1043,8 @@ static int ucsi_init(struct ucsi *ucsi)
goto err_reset; goto err_reset;
} }
mutex_unlock(&ucsi->ppm_lock);
/* Register all connectors */ /* Register all connectors */
for (i = 0; i < ucsi->cap.num_connectors; i++) { for (i = 0; i < ucsi->cap.num_connectors; i++) {
ret = ucsi_register_port(ucsi, i); ret = ucsi_register_port(ucsi, i);
...@@ -1054,12 +1055,10 @@ static int ucsi_init(struct ucsi *ucsi) ...@@ -1054,12 +1055,10 @@ static int ucsi_init(struct ucsi *ucsi)
/* Enable all notifications */ /* Enable all notifications */
ucsi->ntfy = UCSI_ENABLE_NTFY_ALL; ucsi->ntfy = UCSI_ENABLE_NTFY_ALL;
command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy; 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) if (ret < 0)
goto err_unregister; goto err_unregister;
mutex_unlock(&ucsi->ppm_lock);
return 0; return 0;
err_unregister: err_unregister:
...@@ -1071,6 +1070,7 @@ static int ucsi_init(struct ucsi *ucsi) ...@@ -1071,6 +1070,7 @@ static int ucsi_init(struct ucsi *ucsi)
con->port = NULL; con->port = NULL;
} }
mutex_lock(&ucsi->ppm_lock);
err_reset: err_reset:
ucsi_reset_ppm(ucsi); ucsi_reset_ppm(ucsi);
err: err:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment