On Tue, Jul 07, 2026 at 02:17:36PM +0000, Andrei Kuchynski wrote: > 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@vger.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 Reviewed-by: Benson Leung > --- > 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 92166a3725b16..d9668ed7c80ea 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1845,6 +1845,42 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con) > 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 @@ static int ucsi_init(struct ucsi *ucsi) > 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)); > @@ -2194,33 +2216,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.55.0.rc2.803.g1fd1e6609c-goog >