From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Andrei Kuchynski <akuchynski@chromium.org>
Cc: Jameson Thies <jthies@google.com>,
Benson Leung <bleung@chromium.org>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Abhishek Pandit-Subedi <abhishekpandit@chromium.org>,
Pooja Katiyar <pooja.katiyar@intel.com>,
Johan Hovold <johan@kernel.org>,
Hsin-Te Yuan <yuanhsinte@chromium.org>,
Myrrh Periwinkle <myrrhperiwinkle@qtmlabs.xyz>,
Linyu Yuan <quic_linyyuan@quicinc.com>,
Jack Pham <quic_jackp@quicinc.com>,
stable@vger.kernel.org
Subject: Re: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
Date: Mon, 13 Jul 2026 12:08:09 +0300 [thread overview]
Message-ID: <alSq-cD0_fdhA3tU@kuha> (raw)
In-Reply-To: <20260707141736.1635698-1-akuchynski@chromium.org>
Tue, Jul 07, 2026 at 02:17:36PM +0000, Andrei Kuchynski kirjoitti:
> 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@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 <akuchynski@chromium.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> 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
--
heikki
prev parent reply other threads:[~2026-07-13 9:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 14:17 [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration Andrei Kuchynski
2026-07-08 15:15 ` Benson Leung
2026-07-13 9:08 ` Heikki Krogerus [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alSq-cD0_fdhA3tU@kuha \
--to=heikki.krogerus@linux.intel.com \
--cc=abhishekpandit@chromium.org \
--cc=akuchynski@chromium.org \
--cc=bleung@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=johan@kernel.org \
--cc=jthies@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=myrrhperiwinkle@qtmlabs.xyz \
--cc=pooja.katiyar@intel.com \
--cc=quic_jackp@quicinc.com \
--cc=quic_linyyuan@quicinc.com \
--cc=stable@vger.kernel.org \
--cc=yuanhsinte@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.