From: Benson Leung <bleung@google.com>
To: Andrei Kuchynski <akuchynski@chromium.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
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: Wed, 8 Jul 2026 15:15:50 +0000 [thread overview]
Message-ID: <ak5pptaSqRi2222y@google.com> (raw)
In-Reply-To: <20260707141736.1635698-1-akuchynski@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 6356 bytes --]
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:
> <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: Benson Leung <bleung@chromium.org>
> ---
> 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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
prev parent reply other threads:[~2026-07-08 15:15 UTC|newest]
Thread overview: 2+ 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 [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=ak5pptaSqRi2222y@google.com \
--to=bleung@google.com \
--cc=abhishekpandit@chromium.org \
--cc=akuchynski@chromium.org \
--cc=bleung@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox