usb: ucsi: Fix ucsi->connector race

ucsi_init() which runs from a workqueue sets ucsi->connector and
on an error will clear it again.

ucsi->connector gets dereferenced by ucsi_resume(), this checks for
ucsi->connector being NULL in case ucsi_init() has not finished yet;
or in case ucsi_init() has failed.

ucsi_init() setting ucsi->connector and then clearing it again on
an error creates a race where the check in ucsi_resume() may pass,
only to have ucsi->connector free-ed underneath it when ucsi_init()
hits an error.

Fix this race by making ucsi_init() store the connector array in
a local variable and only assign it to ucsi->connector on success.

Fixes: bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API")
Cc: stable@vger.kernel.org
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20230308154244.722337-3-hdegoede@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 0623861..8d1baf2 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1125,12 +1125,11 @@ static struct fwnode_handle *ucsi_find_fwnode(struct ucsi_connector *con)
        return NULL;
 }
 
-static int ucsi_register_port(struct ucsi *ucsi, int index)
+static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
 {
        struct usb_power_delivery_desc desc = { ucsi->cap.pd_version};
        struct usb_power_delivery_capabilities_desc pd_caps;
        struct usb_power_delivery_capabilities *pd_cap;
-       struct ucsi_connector *con = &ucsi->connector[index];
        struct typec_capability *cap = &con->typec_cap;
        enum typec_accessory *accessory = cap->accessory;
        enum usb_role u_role = USB_ROLE_NONE;
@@ -1151,7 +1150,6 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
        init_completion(&con->complete);
        mutex_init(&con->lock);
        INIT_LIST_HEAD(&con->partner_tasks);
-       con->num = index + 1;
        con->ucsi = ucsi;
 
        cap->fwnode = ucsi_find_fwnode(con);
@@ -1328,7 +1326,7 @@ out_unlock:
  */
 static int ucsi_init(struct ucsi *ucsi)
 {
-       struct ucsi_connector *con;
+       struct ucsi_connector *con, *connector;
        u64 command, ntfy;
        int ret;
        int i;
@@ -1359,16 +1357,16 @@ static int ucsi_init(struct ucsi *ucsi)
        }
 
        /* Allocate the connectors. Released in ucsi_unregister() */
-       ucsi->connector = kcalloc(ucsi->cap.num_connectors + 1,
-                                 sizeof(*ucsi->connector), GFP_KERNEL);
-       if (!ucsi->connector) {
+       connector = kcalloc(ucsi->cap.num_connectors + 1, sizeof(*connector), GFP_KERNEL);
+       if (!connector) {
                ret = -ENOMEM;
                goto err_reset;
        }
 
        /* Register all connectors */
        for (i = 0; i < ucsi->cap.num_connectors; i++) {
-               ret = ucsi_register_port(ucsi, i);
+               connector[i].num = i + 1;
+               ret = ucsi_register_port(ucsi, &connector[i]);
                if (ret)
                        goto err_unregister;
        }
@@ -1380,11 +1378,12 @@ static int ucsi_init(struct ucsi *ucsi)
        if (ret < 0)
                goto err_unregister;
 
+       ucsi->connector = connector;
        ucsi->ntfy = ntfy;
        return 0;
 
 err_unregister:
-       for (con = ucsi->connector; con->port; con++) {
+       for (con = connector; con->port; con++) {
                ucsi_unregister_partner(con);
                ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON);
                ucsi_unregister_port_psy(con);
@@ -1400,10 +1399,7 @@ err_unregister:
                typec_unregister_port(con->port);
                con->port = NULL;
        }
-
-       kfree(ucsi->connector);
-       ucsi->connector = NULL;
-
+       kfree(connector);
 err_reset:
        memset(&ucsi->cap, 0, sizeof(ucsi->cap));
        ucsi_reset_ppm(ucsi);