]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
usb: typec: ucsi: Fix race condition and ordering in port unregistration
authorAndrei Kuchynski <akuchynski@chromium.org>
Tue, 7 Jul 2026 14:17:36 +0000 (14:17 +0000)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Mon, 13 Jul 2026 05:10:11 +0000 (07:10 +0200)
A synchronization issue exists during port unregistration where pending
partner work items can race against workqueue destruction, leading to
use-after-free conditions:

  cros_ec_ucsi cros_ec_ucsi.3.auto: error -ETIMEDOUT: PPM init failed
  BUG: kernel NULL pointer dereference, address: 0000000000000000
  RIP: 0010:__queue_work+0x83/0x4a0
  Call Trace:
    <IRQ>
    __cfi_delayed_work_timer_fn+0x10/0x10
    run_timer_softirq+0x3b6/0xbd0
    sched_clock_cpu+0xc/0x110
    irq_exit_rcu+0x18d/0x330
    fred_sysvec_apic_timer_interrupt+0x5e/0x80

Fix this by ensuring strict ordering and proper serialization during
teardown:

1. Move ucsi_unregister_partner() to the beginning of the teardown
sequence and protect it under the connector mutex lock.
2. Ensure all pending partner tasks are explicitly flushed and finished
before the workqueue is destroyed.
3. Switch from mod_delayed_work() to a cancel_delayed_work() and
queue_delayed_work() sequence. This guarantees that items currently marked
as pending won't be scheduled an additional time, preventing a double
release of resources which leads to the following crash:

  Oops: general protection fault, probably for non-canonical address
    0xdead000000000122: 0000 [#1] SMP NOPTI
  Workqueue: cros_ec_ucsi.3.auto-con2 ucsi_poll_worker
  RIP: 0010:ucsi_poll_worker+0x65/0x1e0
  Call Trace:
  <TASK>
    process_scheduled_works+0x218/0x6d0
    worker_thread+0x188/0x3f0
    __cfi_worker_thread+0x10/0x10
    kthread+0x226/0x2a0

To ensure these rules are applied identically across both the normal
teardown and the ucsi_init() error paths, consolidate the cleanup logic
into a new helper, ucsi_unregister_port().

Cc: stable <stable@kernel.org>
Fixes: b9aa02ca39a4 ("usb: typec: ucsi: Add polling mechanism for partner tasks like alt mode checking")
Fixes: b13abcb7ddd8 ("usb: typec: ucsi: Fix NULL pointer access")
Fixes: fac4b8633fd6 ("usb: ucsi: Ensure connector delayed work items are flushed")
Signed-off-by: Andrei Kuchynski <akuchynski@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
Link: https://patch.msgid.link/20260707141736.1635698-1-akuchynski@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/typec/ucsi/ucsi.c

index 6a6723e8fb127fb9969f70475179c4bbfd3ecd51..f56897e23df80402fbd158f16758bfb5026939c7 100644 (file)
@@ -1845,6 +1845,42 @@ out_unlock:
        return ret;
 }
 
+static void ucsi_unregister_port(struct ucsi_connector *con)
+{
+       struct ucsi_work *uwork;
+
+       if (con->wq) {
+               mutex_lock(&con->lock);
+               ucsi_unregister_partner(con);
+               /*
+                * queue delayed items immediately so they can execute
+                * and free themselves before the wq is destroyed
+                */
+               list_for_each_entry(uwork, &con->partner_tasks, node) {
+                       if (cancel_delayed_work(&uwork->work))
+                               queue_delayed_work(con->wq, &uwork->work, 0);
+               }
+               mutex_unlock(&con->lock);
+
+               destroy_workqueue(con->wq);
+               con->wq = NULL;
+       } else {
+               ucsi_unregister_partner(con);
+       }
+
+       ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON);
+       ucsi_unregister_port_psy(con);
+
+       usb_power_delivery_unregister_capabilities(con->port_sink_caps);
+       con->port_sink_caps = NULL;
+       usb_power_delivery_unregister_capabilities(con->port_source_caps);
+       con->port_source_caps = NULL;
+       usb_power_delivery_unregister(con->pd);
+       con->pd = NULL;
+       typec_unregister_port(con->port);
+       con->port = NULL;
+}
+
 static u64 ucsi_get_supported_notifications(struct ucsi *ucsi)
 {
        u16 features = ucsi->cap.features;
@@ -1971,22 +2007,8 @@ err_unregister:
        for (i = 0; i < ucsi->cap.num_connectors; i++)
                lockdep_unregister_key(&connector[i].lock_key);
 
-       for (con = connector; con->port; con++) {
-               if (con->wq)
-                       destroy_workqueue(con->wq);
-               ucsi_unregister_partner(con);
-               ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON);
-               ucsi_unregister_port_psy(con);
-
-               usb_power_delivery_unregister_capabilities(con->port_sink_caps);
-               con->port_sink_caps = NULL;
-               usb_power_delivery_unregister_capabilities(con->port_source_caps);
-               con->port_source_caps = NULL;
-               usb_power_delivery_unregister(con->pd);
-               con->pd = NULL;
-               typec_unregister_port(con->port);
-               con->port = NULL;
-       }
+       for (con = connector; con->port; con++)
+               ucsi_unregister_port(con);
        kfree(connector);
 err_reset:
        memset(&ucsi->cap, 0, sizeof(ucsi->cap));
@@ -2214,33 +2236,7 @@ void ucsi_unregister(struct ucsi *ucsi)
 
        for (i = 0; i < ucsi->cap.num_connectors; i++) {
                cancel_work_sync(&ucsi->connector[i].work);
-
-               if (ucsi->connector[i].wq) {
-                       struct ucsi_work *uwork;
-
-                       mutex_lock(&ucsi->connector[i].lock);
-                       /*
-                        * queue delayed items immediately so they can execute
-                        * and free themselves before the wq is destroyed
-                        */
-                       list_for_each_entry(uwork, &ucsi->connector[i].partner_tasks, node)
-                               mod_delayed_work(ucsi->connector[i].wq, &uwork->work, 0);
-                       mutex_unlock(&ucsi->connector[i].lock);
-                       destroy_workqueue(ucsi->connector[i].wq);
-               }
-
-               ucsi_unregister_partner(&ucsi->connector[i]);
-               ucsi_unregister_altmodes(&ucsi->connector[i],
-                                        UCSI_RECIPIENT_CON);
-               ucsi_unregister_port_psy(&ucsi->connector[i]);
-
-               usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_sink_caps);
-               ucsi->connector[i].port_sink_caps = NULL;
-               usb_power_delivery_unregister_capabilities(ucsi->connector[i].port_source_caps);
-               ucsi->connector[i].port_source_caps = NULL;
-               usb_power_delivery_unregister(ucsi->connector[i].pd);
-               ucsi->connector[i].pd = NULL;
-               typec_unregister_port(ucsi->connector[i].port);
+               ucsi_unregister_port(&ucsi->connector[i]);
                lockdep_unregister_key(&ucsi->connector[i].lock_key);
        }