From 7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261 Mon Sep 17 00:00:00 2001 From: Andrei Kuchynski Date: Tue, 7 Jul 2026 14:17:36 +0000 Subject: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration 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: __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: 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 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 Reviewed-by: Benson Leung Link: https://patch.msgid.link/20260707141736.1635698-1-akuchynski@chromium.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/typec/ucsi/ucsi.c | 82 +++++++++++++++++------------------ 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 6a6723e8fb12..f56897e23df8 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -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); } -- 2.47.3