Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
@ 2026-07-07 14:17 Andrei Kuchynski
  2026-07-08 15:15 ` Benson Leung
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Andrei Kuchynski @ 2026-07-07 14:17 UTC (permalink / raw)
  To: Heikki Krogerus, Jameson Thies, Benson Leung, linux-usb,
	linux-kernel
  Cc: Greg Kroah-Hartman, Abhishek Pandit-Subedi, Pooja Katiyar,
	Johan Hovold, Hsin-Te Yuan, Myrrh Periwinkle, Linyu Yuan,
	Jack Pham, Andrei Kuchynski, stable

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>
---
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  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
  2026-07-16  8:28 ` REGRESSION: " Borah, Chaitanya Kumar
  2 siblings, 0 replies; 8+ messages in thread
From: Benson Leung @ 2026-07-08 15:15 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Heikki Krogerus, Jameson Thies, Benson Leung, linux-usb,
	linux-kernel, Greg Kroah-Hartman, Abhishek Pandit-Subedi,
	Pooja Katiyar, Johan Hovold, Hsin-Te Yuan, Myrrh Periwinkle,
	Linyu Yuan, Jack Pham, stable

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  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
  2026-07-16  8:28 ` REGRESSION: " Borah, Chaitanya Kumar
  2 siblings, 0 replies; 8+ messages in thread
From: Heikki Krogerus @ 2026-07-13  9:08 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Jameson Thies, Benson Leung, linux-usb, linux-kernel,
	Greg Kroah-Hartman, Abhishek Pandit-Subedi, Pooja Katiyar,
	Johan Hovold, Hsin-Te Yuan, Myrrh Periwinkle, Linyu Yuan,
	Jack Pham, stable

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  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
@ 2026-07-16  8:28 ` Borah, Chaitanya Kumar
  2026-07-16  9:15   ` Andrei Kuchynski
  2 siblings, 1 reply; 8+ messages in thread
From: Borah, Chaitanya Kumar @ 2026-07-16  8:28 UTC (permalink / raw)
  To: Andrei Kuchynski, Heikki Krogerus, Jameson Thies, Benson Leung,
	linux-usb, linux-kernel, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org
  Cc: Greg Kroah-Hartman, Abhishek Pandit-Subedi, Pooja Katiyar,
	Johan Hovold, Hsin-Te Yuan, Myrrh Periwinkle, Linyu Yuan,
	Jack Pham, stable

Hello Andrei,

On 7/7/2026 7:47 PM, 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().


This seems to be causing regression in our linux-next CI. [1]

<6>[   18.008557] ------------[ cut here ]------------
<4>[   18.013166] DEBUG_LOCKS_WARN_ON(1)
<4>[   18.013167] WARNING: kernel/locking/lockdep.c:238 at 
__lock_acquire+0xa59/0x2750, CPU#1: kworker/u89:1/172
...
<4>[   18.159950] Hardware name: Intel Corporation Meteor Lake Client 
Platform/MTL-P DDR5 SODIMM SBS RVP, BIOS 
MTLPFWI1.R00.3471.D89.2401091901 01/09/2024
<4>[   18.173125] Workqueue: events_dfl_long ucsi_init_work [typec_ucsi]
<4>[   18.179257] RIP: 0010:__lock_acquire+0xa60/0x2750
<4>[   18.183927] Code: 44 8b 5d a8 85 c0 0f 84 98 fd ff ff 44 8b 35 07 
0b 72 02 45 85 f6 0f 85 88 fd ff ff 48 8d 3d 27 25 73 02 48 c7 c6 1b 51 
27 83 <67> 48 0f b9 3a 31 c0 44 8b 5d a8 44 8b 4d b0 e9 c6 f7 ff ff 31 d2
...
<4>[   18.263886] PKRU: 55555554
<4>[   18.266583] Call Trace:
<4>[   18.269014]  <TASK>
<4>[   18.271110]  ? mark_held_locks+0x46/0x90
<4>[   18.275007]  lock_acquire+0xd9/0x2e0
<4>[   18.278562]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
<4>[   18.284096]  __mutex_lock+0xb2/0x1020
<4>[   18.287735]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
<4>[   18.293265]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
<4>[   18.298797]  ? synchronize_rcu_expedited+0x2b2/0x300
<4>[   18.303724]  ? __pfx_autoremove_wake_function+0x10/0x10
<4>[   18.308912]  mutex_lock_nested+0x1b/0x30
<4>[   18.312806]  ? mutex_lock_nested+0x1b/0x30
<4>[   18.316878]  ucsi_unregister_port+0x36/0x140 [typec_ucsi]
<4>[   18.322238]  ucsi_init_work+0x66d/0xc10 [typec_ucsi]
<4>[   18.327167]  process_one_work+0x2ad/0x8b0
<4>[   18.331158]  worker_thread+0x200/0x3f0
<4>[   18.334886]  ? __pfx_worker_thread+0x10/0x10
<4>[   18.339128]  kthread+0x10d/0x150
<4>[   18.342339]  ? __pfx_kthread+0x10/0x10
<4>[   18.346067]  ret_from_fork+0x3bd/0x470
<4>[   18.349795]  ? __pfx_kthread+0x10/0x10
<4>[   18.353522]  ret_from_fork_asm+0x1a/0x30
<4>[   18.357420]  </TASK>

Detailed log can be seen found in [2]

We confirmed that reverting the patch solves the issue.

Could you please check why the patch causes this regression and provide 
a fix if necessary?

Regards
Chaitanya

[1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
[2] 
https://intel-gfx-ci.01.org/tree/linux-next/next-20260714/bat-mtlp-9/boot0.txt

-- Bisect Logs --

git bisect start
# status: waiting for both good and bad commits
# good: [5f1b513690edf51727e6928b97705b6437f9a98e] Merge remote-tracking 
branch 'spi/for-7.3' into spi-next
git bisect good 5f1b513690edf51727e6928b97705b6437f9a98e
# status: waiting for bad commit, 1 good commit known
# bad: [49362394dad7df66c274c867a271394c10ca2bb8] Add linux-next 
specific files for 20260713
git bisect bad 49362394dad7df66c274c867a271394c10ca2bb8
# bad: [dffddeb046466ed57038295bde28b42b7506a12b] Merge branch 
'nand/next' of https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
git bisect bad dffddeb046466ed57038295bde28b42b7506a12b
# bad: [d154d0766bf3fe830e1bf98fb2d8648294a4a8da] Merge branch 
'for-next' of 
https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap.git
git bisect bad d154d0766bf3fe830e1bf98fb2d8648294a4a8da
# bad: [671b0fd929bec9ebf863cb376baf9b9d1226a510] Merge branch 'master' 
of https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git
git bisect bad 671b0fd929bec9ebf863cb376baf9b9d1226a510
# good: [f4fb100039e96211609dfc44fb24b9e4a8a0f2f9] Merge tag 
's390-7.2-4' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
git bisect good f4fb100039e96211609dfc44fb24b9e4a8a0f2f9
# good: [c296d513e034df93ddb230ad1c7a118d6b58750c] Merge branch 
'arm/fixes' of https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
git bisect good c296d513e034df93ddb230ad1c7a118d6b58750c
# good: [de53c0a584f08f31eb957f8c849a59fb33b27fa8] Merge branch 'for-rc' 
of https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
git bisect good de53c0a584f08f31eb957f8c849a59fb33b27fa8
# bad: [a6969de5a038bc08a948ada52bd7199a8dfa6cec] Merge branch 
'usb-linus' of 
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
git bisect bad a6969de5a038bc08a948ada52bd7199a8dfa6cec
# good: [15eab2dd63a97587405ab6bfaad9728d5b35b5fe] Merge tag 
'asoc-fix-v7.2-rc2' of 
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
git bisect good 15eab2dd63a97587405ab6bfaad9728d5b35b5fe
# bad: [c2e819be6a5c7f34344926b4bd7e3dfca58cf48a] usb: gadget: printer: 
fix infinite loop in printer_read()
git bisect bad c2e819be6a5c7f34344926b4bd7e3dfca58cf48a
# good: [29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b] USB: gadget: 
snps-udc: fix device name leak on probe failure
git bisect good 29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b
# good: [1febec7e47cdcd01f43fb0211094e3010474666e] usb: gadget: f_ncm: 
validate datagram bounds in ncm_unwrap_ntb()
git bisect good 1febec7e47cdcd01f43fb0211094e3010474666e
# bad: [0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb] usb: gadget: udc: bdc: 
free IRQ and drain func_wake_notify before teardown
git bisect bad 0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb
# bad: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb: typec: ucsi: Fix 
race condition and ordering in port unregistration
git bisect bad 7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261
# first bad commit: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb: 
typec: ucsi: Fix race condition and ordering in port unregistration
> 
> 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>
> ---
>   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);
>   	}
>   


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  2026-07-16  8:28 ` REGRESSION: " Borah, Chaitanya Kumar
@ 2026-07-16  9:15   ` Andrei Kuchynski
  2026-07-16 13:23     ` Borah, Chaitanya Kumar
  0 siblings, 1 reply; 8+ messages in thread
From: Andrei Kuchynski @ 2026-07-16  9:15 UTC (permalink / raw)
  To: Borah, Chaitanya Kumar
  Cc: Heikki Krogerus, Jameson Thies, Benson Leung, linux-usb,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org, Greg Kroah-Hartman,
	Abhishek Pandit-Subedi, Pooja Katiyar, Johan Hovold, Hsin-Te Yuan,
	Myrrh Periwinkle, Linyu Yuan, Jack Pham, stable

On Thu, Jul 16, 2026 at 10:28 AM Borah, Chaitanya Kumar
<chaitanya.kumar.borah@intel.com> wrote:
>
> Hello Andrei,
>
> On 7/7/2026 7:47 PM, 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().
>
>
> This seems to be causing regression in our linux-next CI. [1]
>
> <6>[   18.008557] ------------[ cut here ]------------
> <4>[   18.013166] DEBUG_LOCKS_WARN_ON(1)
> <4>[   18.013167] WARNING: kernel/locking/lockdep.c:238 at
> __lock_acquire+0xa59/0x2750, CPU#1: kworker/u89:1/172
> ...
> <4>[   18.159950] Hardware name: Intel Corporation Meteor Lake Client
> Platform/MTL-P DDR5 SODIMM SBS RVP, BIOS
> MTLPFWI1.R00.3471.D89.2401091901 01/09/2024
> <4>[   18.173125] Workqueue: events_dfl_long ucsi_init_work [typec_ucsi]
> <4>[   18.179257] RIP: 0010:__lock_acquire+0xa60/0x2750
> <4>[   18.183927] Code: 44 8b 5d a8 85 c0 0f 84 98 fd ff ff 44 8b 35 07
> 0b 72 02 45 85 f6 0f 85 88 fd ff ff 48 8d 3d 27 25 73 02 48 c7 c6 1b 51
> 27 83 <67> 48 0f b9 3a 31 c0 44 8b 5d a8 44 8b 4d b0 e9 c6 f7 ff ff 31 d2
> ...
> <4>[   18.263886] PKRU: 55555554
> <4>[   18.266583] Call Trace:
> <4>[   18.269014]  <TASK>
> <4>[   18.271110]  ? mark_held_locks+0x46/0x90
> <4>[   18.275007]  lock_acquire+0xd9/0x2e0
> <4>[   18.278562]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> <4>[   18.284096]  __mutex_lock+0xb2/0x1020
> <4>[   18.287735]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> <4>[   18.293265]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> <4>[   18.298797]  ? synchronize_rcu_expedited+0x2b2/0x300
> <4>[   18.303724]  ? __pfx_autoremove_wake_function+0x10/0x10
> <4>[   18.308912]  mutex_lock_nested+0x1b/0x30
> <4>[   18.312806]  ? mutex_lock_nested+0x1b/0x30
> <4>[   18.316878]  ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> <4>[   18.322238]  ucsi_init_work+0x66d/0xc10 [typec_ucsi]
> <4>[   18.327167]  process_one_work+0x2ad/0x8b0
> <4>[   18.331158]  worker_thread+0x200/0x3f0
> <4>[   18.334886]  ? __pfx_worker_thread+0x10/0x10
> <4>[   18.339128]  kthread+0x10d/0x150
> <4>[   18.342339]  ? __pfx_kthread+0x10/0x10
> <4>[   18.346067]  ret_from_fork+0x3bd/0x470
> <4>[   18.349795]  ? __pfx_kthread+0x10/0x10
> <4>[   18.353522]  ret_from_fork_asm+0x1a/0x30
> <4>[   18.357420]  </TASK>
>
> Detailed log can be seen found in [2]
>
> We confirmed that reverting the patch solves the issue.
>
> Could you please check why the patch causes this regression and provide
> a fix if necessary?
>
> Regards
> Chaitanya
>
> [1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
> [2]
> https://intel-gfx-ci.01.org/tree/linux-next/next-20260714/bat-mtlp-9/boot0.txt
>
> -- Bisect Logs --
>
> git bisect start
> # status: waiting for both good and bad commits
> # good: [5f1b513690edf51727e6928b97705b6437f9a98e] Merge remote-tracking
> branch 'spi/for-7.3' into spi-next
> git bisect good 5f1b513690edf51727e6928b97705b6437f9a98e
> # status: waiting for bad commit, 1 good commit known
> # bad: [49362394dad7df66c274c867a271394c10ca2bb8] Add linux-next
> specific files for 20260713
> git bisect bad 49362394dad7df66c274c867a271394c10ca2bb8
> # bad: [dffddeb046466ed57038295bde28b42b7506a12b] Merge branch
> 'nand/next' of https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
> git bisect bad dffddeb046466ed57038295bde28b42b7506a12b
> # bad: [d154d0766bf3fe830e1bf98fb2d8648294a4a8da] Merge branch
> 'for-next' of
> https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap.git
> git bisect bad d154d0766bf3fe830e1bf98fb2d8648294a4a8da
> # bad: [671b0fd929bec9ebf863cb376baf9b9d1226a510] Merge branch 'master'
> of https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git
> git bisect bad 671b0fd929bec9ebf863cb376baf9b9d1226a510
> # good: [f4fb100039e96211609dfc44fb24b9e4a8a0f2f9] Merge tag
> 's390-7.2-4' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> git bisect good f4fb100039e96211609dfc44fb24b9e4a8a0f2f9
> # good: [c296d513e034df93ddb230ad1c7a118d6b58750c] Merge branch
> 'arm/fixes' of https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
> git bisect good c296d513e034df93ddb230ad1c7a118d6b58750c
> # good: [de53c0a584f08f31eb957f8c849a59fb33b27fa8] Merge branch 'for-rc'
> of https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
> git bisect good de53c0a584f08f31eb957f8c849a59fb33b27fa8
> # bad: [a6969de5a038bc08a948ada52bd7199a8dfa6cec] Merge branch
> 'usb-linus' of
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
> git bisect bad a6969de5a038bc08a948ada52bd7199a8dfa6cec
> # good: [15eab2dd63a97587405ab6bfaad9728d5b35b5fe] Merge tag
> 'asoc-fix-v7.2-rc2' of
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
> git bisect good 15eab2dd63a97587405ab6bfaad9728d5b35b5fe
> # bad: [c2e819be6a5c7f34344926b4bd7e3dfca58cf48a] usb: gadget: printer:
> fix infinite loop in printer_read()
> git bisect bad c2e819be6a5c7f34344926b4bd7e3dfca58cf48a
> # good: [29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b] USB: gadget:
> snps-udc: fix device name leak on probe failure
> git bisect good 29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b
> # good: [1febec7e47cdcd01f43fb0211094e3010474666e] usb: gadget: f_ncm:
> validate datagram bounds in ncm_unwrap_ntb()
> git bisect good 1febec7e47cdcd01f43fb0211094e3010474666e
> # bad: [0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb] usb: gadget: udc: bdc:
> free IRQ and drain func_wake_notify before teardown
> git bisect bad 0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb
> # bad: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb: typec: ucsi: Fix
> race condition and ordering in port unregistration
> git bisect bad 7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261
> # first bad commit: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb:
> typec: ucsi: Fix race condition and ordering in port unregistration
> >
> > 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>
> > ---
> >   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);

Hi Chaitanya,
Thank you for spotting this!

Could you verify if calling lockdep_unregister_key(&connector[i].lock_key)
after ucsi_unregister_port(con) resolves the issue?

     for (con = connector; con->port; con++)
            ucsi_unregister_port(con);
     for (i = 0; i < ucsi->cap.num_connectors; i++)
               lockdep_unregister_key(&connector[i].lock_key);

I will also try testing this on my side.

Thanks,
Andrei

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  2026-07-16  9:15   ` Andrei Kuchynski
@ 2026-07-16 13:23     ` Borah, Chaitanya Kumar
  2026-07-16 13:41       ` Andrei Kuchynski
  0 siblings, 1 reply; 8+ messages in thread
From: Borah, Chaitanya Kumar @ 2026-07-16 13:23 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Heikki Krogerus, Jameson Thies, Benson Leung, linux-usb,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org, Greg Kroah-Hartman,
	Abhishek Pandit-Subedi, Pooja Katiyar, Johan Hovold, Hsin-Te Yuan,
	Myrrh Periwinkle, Linyu Yuan, Jack Pham, stable



On 7/16/2026 2:45 PM, Andrei Kuchynski wrote:
> On Thu, Jul 16, 2026 at 10:28 AM Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com> wrote:
>>
>> Hello Andrei,
>>
>> On 7/7/2026 7:47 PM, 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().
>>
>>
>> This seems to be causing regression in our linux-next CI. [1]
>>
>> <6>[   18.008557] ------------[ cut here ]------------
>> <4>[   18.013166] DEBUG_LOCKS_WARN_ON(1)
>> <4>[   18.013167] WARNING: kernel/locking/lockdep.c:238 at
>> __lock_acquire+0xa59/0x2750, CPU#1: kworker/u89:1/172
>> ...
>> <4>[   18.159950] Hardware name: Intel Corporation Meteor Lake Client
>> Platform/MTL-P DDR5 SODIMM SBS RVP, BIOS
>> MTLPFWI1.R00.3471.D89.2401091901 01/09/2024
>> <4>[   18.173125] Workqueue: events_dfl_long ucsi_init_work [typec_ucsi]
>> <4>[   18.179257] RIP: 0010:__lock_acquire+0xa60/0x2750
>> <4>[   18.183927] Code: 44 8b 5d a8 85 c0 0f 84 98 fd ff ff 44 8b 35 07
>> 0b 72 02 45 85 f6 0f 85 88 fd ff ff 48 8d 3d 27 25 73 02 48 c7 c6 1b 51
>> 27 83 <67> 48 0f b9 3a 31 c0 44 8b 5d a8 44 8b 4d b0 e9 c6 f7 ff ff 31 d2
>> ...
>> <4>[   18.263886] PKRU: 55555554
>> <4>[   18.266583] Call Trace:
>> <4>[   18.269014]  <TASK>
>> <4>[   18.271110]  ? mark_held_locks+0x46/0x90
>> <4>[   18.275007]  lock_acquire+0xd9/0x2e0
>> <4>[   18.278562]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>> <4>[   18.284096]  __mutex_lock+0xb2/0x1020
>> <4>[   18.287735]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>> <4>[   18.293265]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>> <4>[   18.298797]  ? synchronize_rcu_expedited+0x2b2/0x300
>> <4>[   18.303724]  ? __pfx_autoremove_wake_function+0x10/0x10
>> <4>[   18.308912]  mutex_lock_nested+0x1b/0x30
>> <4>[   18.312806]  ? mutex_lock_nested+0x1b/0x30
>> <4>[   18.316878]  ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>> <4>[   18.322238]  ucsi_init_work+0x66d/0xc10 [typec_ucsi]
>> <4>[   18.327167]  process_one_work+0x2ad/0x8b0
>> <4>[   18.331158]  worker_thread+0x200/0x3f0
>> <4>[   18.334886]  ? __pfx_worker_thread+0x10/0x10
>> <4>[   18.339128]  kthread+0x10d/0x150
>> <4>[   18.342339]  ? __pfx_kthread+0x10/0x10
>> <4>[   18.346067]  ret_from_fork+0x3bd/0x470
>> <4>[   18.349795]  ? __pfx_kthread+0x10/0x10
>> <4>[   18.353522]  ret_from_fork_asm+0x1a/0x30
>> <4>[   18.357420]  </TASK>
>>
>> Detailed log can be seen found in [2]
>>
>> We confirmed that reverting the patch solves the issue.
>>
>> Could you please check why the patch causes this regression and provide
>> a fix if necessary?
>>
>> Regards
>> Chaitanya
>>
>> [1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
>> [2]
>> https://intel-gfx-ci.01.org/tree/linux-next/next-20260714/bat-mtlp-9/boot0.txt
>>
>> -- Bisect Logs --
>>
>> git bisect start
>> # status: waiting for both good and bad commits
>> # good: [5f1b513690edf51727e6928b97705b6437f9a98e] Merge remote-tracking
>> branch 'spi/for-7.3' into spi-next
>> git bisect good 5f1b513690edf51727e6928b97705b6437f9a98e
>> # status: waiting for bad commit, 1 good commit known
>> # bad: [49362394dad7df66c274c867a271394c10ca2bb8] Add linux-next
>> specific files for 20260713
>> git bisect bad 49362394dad7df66c274c867a271394c10ca2bb8
>> # bad: [dffddeb046466ed57038295bde28b42b7506a12b] Merge branch
>> 'nand/next' of https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
>> git bisect bad dffddeb046466ed57038295bde28b42b7506a12b
>> # bad: [d154d0766bf3fe830e1bf98fb2d8648294a4a8da] Merge branch
>> 'for-next' of
>> https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap.git
>> git bisect bad d154d0766bf3fe830e1bf98fb2d8648294a4a8da
>> # bad: [671b0fd929bec9ebf863cb376baf9b9d1226a510] Merge branch 'master'
>> of https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git
>> git bisect bad 671b0fd929bec9ebf863cb376baf9b9d1226a510
>> # good: [f4fb100039e96211609dfc44fb24b9e4a8a0f2f9] Merge tag
>> 's390-7.2-4' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
>> git bisect good f4fb100039e96211609dfc44fb24b9e4a8a0f2f9
>> # good: [c296d513e034df93ddb230ad1c7a118d6b58750c] Merge branch
>> 'arm/fixes' of https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
>> git bisect good c296d513e034df93ddb230ad1c7a118d6b58750c
>> # good: [de53c0a584f08f31eb957f8c849a59fb33b27fa8] Merge branch 'for-rc'
>> of https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
>> git bisect good de53c0a584f08f31eb957f8c849a59fb33b27fa8
>> # bad: [a6969de5a038bc08a948ada52bd7199a8dfa6cec] Merge branch
>> 'usb-linus' of
>> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
>> git bisect bad a6969de5a038bc08a948ada52bd7199a8dfa6cec
>> # good: [15eab2dd63a97587405ab6bfaad9728d5b35b5fe] Merge tag
>> 'asoc-fix-v7.2-rc2' of
>> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
>> git bisect good 15eab2dd63a97587405ab6bfaad9728d5b35b5fe
>> # bad: [c2e819be6a5c7f34344926b4bd7e3dfca58cf48a] usb: gadget: printer:
>> fix infinite loop in printer_read()
>> git bisect bad c2e819be6a5c7f34344926b4bd7e3dfca58cf48a
>> # good: [29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b] USB: gadget:
>> snps-udc: fix device name leak on probe failure
>> git bisect good 29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b
>> # good: [1febec7e47cdcd01f43fb0211094e3010474666e] usb: gadget: f_ncm:
>> validate datagram bounds in ncm_unwrap_ntb()
>> git bisect good 1febec7e47cdcd01f43fb0211094e3010474666e
>> # bad: [0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb] usb: gadget: udc: bdc:
>> free IRQ and drain func_wake_notify before teardown
>> git bisect bad 0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb
>> # bad: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb: typec: ucsi: Fix
>> race condition and ordering in port unregistration
>> git bisect bad 7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261
>> # first bad commit: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb:
>> typec: ucsi: Fix race condition and ordering in port unregistration
>>>
>>> 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>
>>> ---
>>>    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);
> 
> Hi Chaitanya,
> Thank you for spotting this!
> 
> Could you verify if calling lockdep_unregister_key(&connector[i].lock_key)
> after ucsi_unregister_port(con) resolves the issue?
> 
>       for (con = connector; con->port; con++)
>              ucsi_unregister_port(con);
>       for (i = 0; i < ucsi->cap.num_connectors; i++)
>                 lockdep_unregister_key(&connector[i].lock_key);
> 
> I will also try testing this on my side.

Thank you Andrei for the patch. It seems to work for us.

==
Chaitanya

> 
> Thanks,
> Andrei


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  2026-07-16 13:23     ` Borah, Chaitanya Kumar
@ 2026-07-16 13:41       ` Andrei Kuchynski
  2026-07-16 13:59         ` Borah, Chaitanya Kumar
  0 siblings, 1 reply; 8+ messages in thread
From: Andrei Kuchynski @ 2026-07-16 13:41 UTC (permalink / raw)
  To: Borah, Chaitanya Kumar
  Cc: Heikki Krogerus, Jameson Thies, Benson Leung, linux-usb,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org, Greg Kroah-Hartman,
	Abhishek Pandit-Subedi, Pooja Katiyar, Johan Hovold, Hsin-Te Yuan,
	Myrrh Periwinkle, Linyu Yuan, Jack Pham, stable

On Thu, Jul 16, 2026 at 3:23 PM Borah, Chaitanya Kumar
<chaitanya.kumar.borah@intel.com> wrote:
>
>
>
> On 7/16/2026 2:45 PM, Andrei Kuchynski wrote:
> > On Thu, Jul 16, 2026 at 10:28 AM Borah, Chaitanya Kumar
> > <chaitanya.kumar.borah@intel.com> wrote:
> >>
> >> Hello Andrei,
> >>
> >> On 7/7/2026 7:47 PM, 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().
> >>
> >>
> >> This seems to be causing regression in our linux-next CI. [1]
> >>
> >> <6>[   18.008557] ------------[ cut here ]------------
> >> <4>[   18.013166] DEBUG_LOCKS_WARN_ON(1)
> >> <4>[   18.013167] WARNING: kernel/locking/lockdep.c:238 at
> >> __lock_acquire+0xa59/0x2750, CPU#1: kworker/u89:1/172
> >> ...
> >> <4>[   18.159950] Hardware name: Intel Corporation Meteor Lake Client
> >> Platform/MTL-P DDR5 SODIMM SBS RVP, BIOS
> >> MTLPFWI1.R00.3471.D89.2401091901 01/09/2024
> >> <4>[   18.173125] Workqueue: events_dfl_long ucsi_init_work [typec_ucsi]
> >> <4>[   18.179257] RIP: 0010:__lock_acquire+0xa60/0x2750
> >> <4>[   18.183927] Code: 44 8b 5d a8 85 c0 0f 84 98 fd ff ff 44 8b 35 07
> >> 0b 72 02 45 85 f6 0f 85 88 fd ff ff 48 8d 3d 27 25 73 02 48 c7 c6 1b 51
> >> 27 83 <67> 48 0f b9 3a 31 c0 44 8b 5d a8 44 8b 4d b0 e9 c6 f7 ff ff 31 d2
> >> ...
> >> <4>[   18.263886] PKRU: 55555554
> >> <4>[   18.266583] Call Trace:
> >> <4>[   18.269014]  <TASK>
> >> <4>[   18.271110]  ? mark_held_locks+0x46/0x90
> >> <4>[   18.275007]  lock_acquire+0xd9/0x2e0
> >> <4>[   18.278562]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> >> <4>[   18.284096]  __mutex_lock+0xb2/0x1020
> >> <4>[   18.287735]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> >> <4>[   18.293265]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> >> <4>[   18.298797]  ? synchronize_rcu_expedited+0x2b2/0x300
> >> <4>[   18.303724]  ? __pfx_autoremove_wake_function+0x10/0x10
> >> <4>[   18.308912]  mutex_lock_nested+0x1b/0x30
> >> <4>[   18.312806]  ? mutex_lock_nested+0x1b/0x30
> >> <4>[   18.316878]  ucsi_unregister_port+0x36/0x140 [typec_ucsi]
> >> <4>[   18.322238]  ucsi_init_work+0x66d/0xc10 [typec_ucsi]
> >> <4>[   18.327167]  process_one_work+0x2ad/0x8b0
> >> <4>[   18.331158]  worker_thread+0x200/0x3f0
> >> <4>[   18.334886]  ? __pfx_worker_thread+0x10/0x10
> >> <4>[   18.339128]  kthread+0x10d/0x150
> >> <4>[   18.342339]  ? __pfx_kthread+0x10/0x10
> >> <4>[   18.346067]  ret_from_fork+0x3bd/0x470
> >> <4>[   18.349795]  ? __pfx_kthread+0x10/0x10
> >> <4>[   18.353522]  ret_from_fork_asm+0x1a/0x30
> >> <4>[   18.357420]  </TASK>
> >>
> >> Detailed log can be seen found in [2]
> >>
> >> We confirmed that reverting the patch solves the issue.
> >>
> >> Could you please check why the patch causes this regression and provide
> >> a fix if necessary?
> >>
> >> Regards
> >> Chaitanya
> >>
> >> [1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
> >> [2]
> >> https://intel-gfx-ci.01.org/tree/linux-next/next-20260714/bat-mtlp-9/boot0.txt
> >>
> >> -- Bisect Logs --
> >>
> >> git bisect start
> >> # status: waiting for both good and bad commits
> >> # good: [5f1b513690edf51727e6928b97705b6437f9a98e] Merge remote-tracking
> >> branch 'spi/for-7.3' into spi-next
> >> git bisect good 5f1b513690edf51727e6928b97705b6437f9a98e
> >> # status: waiting for bad commit, 1 good commit known
> >> # bad: [49362394dad7df66c274c867a271394c10ca2bb8] Add linux-next
> >> specific files for 20260713
> >> git bisect bad 49362394dad7df66c274c867a271394c10ca2bb8
> >> # bad: [dffddeb046466ed57038295bde28b42b7506a12b] Merge branch
> >> 'nand/next' of https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
> >> git bisect bad dffddeb046466ed57038295bde28b42b7506a12b
> >> # bad: [d154d0766bf3fe830e1bf98fb2d8648294a4a8da] Merge branch
> >> 'for-next' of
> >> https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap.git
> >> git bisect bad d154d0766bf3fe830e1bf98fb2d8648294a4a8da
> >> # bad: [671b0fd929bec9ebf863cb376baf9b9d1226a510] Merge branch 'master'
> >> of https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git
> >> git bisect bad 671b0fd929bec9ebf863cb376baf9b9d1226a510
> >> # good: [f4fb100039e96211609dfc44fb24b9e4a8a0f2f9] Merge tag
> >> 's390-7.2-4' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> >> git bisect good f4fb100039e96211609dfc44fb24b9e4a8a0f2f9
> >> # good: [c296d513e034df93ddb230ad1c7a118d6b58750c] Merge branch
> >> 'arm/fixes' of https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
> >> git bisect good c296d513e034df93ddb230ad1c7a118d6b58750c
> >> # good: [de53c0a584f08f31eb957f8c849a59fb33b27fa8] Merge branch 'for-rc'
> >> of https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
> >> git bisect good de53c0a584f08f31eb957f8c849a59fb33b27fa8
> >> # bad: [a6969de5a038bc08a948ada52bd7199a8dfa6cec] Merge branch
> >> 'usb-linus' of
> >> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
> >> git bisect bad a6969de5a038bc08a948ada52bd7199a8dfa6cec
> >> # good: [15eab2dd63a97587405ab6bfaad9728d5b35b5fe] Merge tag
> >> 'asoc-fix-v7.2-rc2' of
> >> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
> >> git bisect good 15eab2dd63a97587405ab6bfaad9728d5b35b5fe
> >> # bad: [c2e819be6a5c7f34344926b4bd7e3dfca58cf48a] usb: gadget: printer:
> >> fix infinite loop in printer_read()
> >> git bisect bad c2e819be6a5c7f34344926b4bd7e3dfca58cf48a
> >> # good: [29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b] USB: gadget:
> >> snps-udc: fix device name leak on probe failure
> >> git bisect good 29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b
> >> # good: [1febec7e47cdcd01f43fb0211094e3010474666e] usb: gadget: f_ncm:
> >> validate datagram bounds in ncm_unwrap_ntb()
> >> git bisect good 1febec7e47cdcd01f43fb0211094e3010474666e
> >> # bad: [0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb] usb: gadget: udc: bdc:
> >> free IRQ and drain func_wake_notify before teardown
> >> git bisect bad 0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb
> >> # bad: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb: typec: ucsi: Fix
> >> race condition and ordering in port unregistration
> >> git bisect bad 7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261
> >> # first bad commit: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb:
> >> typec: ucsi: Fix race condition and ordering in port unregistration
> >>>
> >>> 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>
> >>> ---
> >>>    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);
> >
> > Hi Chaitanya,
> > Thank you for spotting this!
> >
> > Could you verify if calling lockdep_unregister_key(&connector[i].lock_key)
> > after ucsi_unregister_port(con) resolves the issue?
> >
> >       for (con = connector; con->port; con++)
> >              ucsi_unregister_port(con);
> >       for (i = 0; i < ucsi->cap.num_connectors; i++)
> >                 lockdep_unregister_key(&connector[i].lock_key);
> >
> > I will also try testing this on my side.
>
> Thank you Andrei for the patch. It seems to work for us.
>

Hi Chaitanya,

Thanks for the testing!
I will prepare a fix or maybe V2 of this patch.

Could you please share your configuration details? Specifically, do you
have any of the following enabled?
CONFIG_DEBUG_LOCKDEP
CONFIG_LOCK_STAT
CONFIG_LOCKDEP_SMALL
CONFIG_DEBUG_LOCKDEP
CONFIG_PROVE_LOCKING

I am still  not able to reproduce it.

Thanks,
Andrei

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
  2026-07-16 13:41       ` Andrei Kuchynski
@ 2026-07-16 13:59         ` Borah, Chaitanya Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Borah, Chaitanya Kumar @ 2026-07-16 13:59 UTC (permalink / raw)
  To: Andrei Kuchynski
  Cc: Heikki Krogerus, Jameson Thies, Benson Leung, linux-usb,
	linux-kernel, intel-gfx@lists.freedesktop.org,
	intel-xe@lists.freedesktop.org, Greg Kroah-Hartman,
	Abhishek Pandit-Subedi, Pooja Katiyar, Johan Hovold, Hsin-Te Yuan,
	Myrrh Periwinkle, Linyu Yuan, Jack Pham, stable



On 7/16/2026 7:11 PM, Andrei Kuchynski wrote:
> On Thu, Jul 16, 2026 at 3:23 PM Borah, Chaitanya Kumar
> <chaitanya.kumar.borah@intel.com> wrote:
>>
>>
>>
>> On 7/16/2026 2:45 PM, Andrei Kuchynski wrote:
>>> On Thu, Jul 16, 2026 at 10:28 AM Borah, Chaitanya Kumar
>>> <chaitanya.kumar.borah@intel.com> wrote:
>>>>
>>>> Hello Andrei,
>>>>
>>>> On 7/7/2026 7:47 PM, 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().
>>>>
>>>>
>>>> This seems to be causing regression in our linux-next CI. [1]
>>>>
>>>> <6>[   18.008557] ------------[ cut here ]------------
>>>> <4>[   18.013166] DEBUG_LOCKS_WARN_ON(1)
>>>> <4>[   18.013167] WARNING: kernel/locking/lockdep.c:238 at
>>>> __lock_acquire+0xa59/0x2750, CPU#1: kworker/u89:1/172
>>>> ...
>>>> <4>[   18.159950] Hardware name: Intel Corporation Meteor Lake Client
>>>> Platform/MTL-P DDR5 SODIMM SBS RVP, BIOS
>>>> MTLPFWI1.R00.3471.D89.2401091901 01/09/2024
>>>> <4>[   18.173125] Workqueue: events_dfl_long ucsi_init_work [typec_ucsi]
>>>> <4>[   18.179257] RIP: 0010:__lock_acquire+0xa60/0x2750
>>>> <4>[   18.183927] Code: 44 8b 5d a8 85 c0 0f 84 98 fd ff ff 44 8b 35 07
>>>> 0b 72 02 45 85 f6 0f 85 88 fd ff ff 48 8d 3d 27 25 73 02 48 c7 c6 1b 51
>>>> 27 83 <67> 48 0f b9 3a 31 c0 44 8b 5d a8 44 8b 4d b0 e9 c6 f7 ff ff 31 d2
>>>> ...
>>>> <4>[   18.263886] PKRU: 55555554
>>>> <4>[   18.266583] Call Trace:
>>>> <4>[   18.269014]  <TASK>
>>>> <4>[   18.271110]  ? mark_held_locks+0x46/0x90
>>>> <4>[   18.275007]  lock_acquire+0xd9/0x2e0
>>>> <4>[   18.278562]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>>>> <4>[   18.284096]  __mutex_lock+0xb2/0x1020
>>>> <4>[   18.287735]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>>>> <4>[   18.293265]  ? ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>>>> <4>[   18.298797]  ? synchronize_rcu_expedited+0x2b2/0x300
>>>> <4>[   18.303724]  ? __pfx_autoremove_wake_function+0x10/0x10
>>>> <4>[   18.308912]  mutex_lock_nested+0x1b/0x30
>>>> <4>[   18.312806]  ? mutex_lock_nested+0x1b/0x30
>>>> <4>[   18.316878]  ucsi_unregister_port+0x36/0x140 [typec_ucsi]
>>>> <4>[   18.322238]  ucsi_init_work+0x66d/0xc10 [typec_ucsi]
>>>> <4>[   18.327167]  process_one_work+0x2ad/0x8b0
>>>> <4>[   18.331158]  worker_thread+0x200/0x3f0
>>>> <4>[   18.334886]  ? __pfx_worker_thread+0x10/0x10
>>>> <4>[   18.339128]  kthread+0x10d/0x150
>>>> <4>[   18.342339]  ? __pfx_kthread+0x10/0x10
>>>> <4>[   18.346067]  ret_from_fork+0x3bd/0x470
>>>> <4>[   18.349795]  ? __pfx_kthread+0x10/0x10
>>>> <4>[   18.353522]  ret_from_fork_asm+0x1a/0x30
>>>> <4>[   18.357420]  </TASK>
>>>>
>>>> Detailed log can be seen found in [2]
>>>>
>>>> We confirmed that reverting the patch solves the issue.
>>>>
>>>> Could you please check why the patch causes this regression and provide
>>>> a fix if necessary?
>>>>
>>>> Regards
>>>> Chaitanya
>>>>
>>>> [1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
>>>> [2]
>>>> https://intel-gfx-ci.01.org/tree/linux-next/next-20260714/bat-mtlp-9/boot0.txt
>>>>
>>>> -- Bisect Logs --
>>>>
>>>> git bisect start
>>>> # status: waiting for both good and bad commits
>>>> # good: [5f1b513690edf51727e6928b97705b6437f9a98e] Merge remote-tracking
>>>> branch 'spi/for-7.3' into spi-next
>>>> git bisect good 5f1b513690edf51727e6928b97705b6437f9a98e
>>>> # status: waiting for bad commit, 1 good commit known
>>>> # bad: [49362394dad7df66c274c867a271394c10ca2bb8] Add linux-next
>>>> specific files for 20260713
>>>> git bisect bad 49362394dad7df66c274c867a271394c10ca2bb8
>>>> # bad: [dffddeb046466ed57038295bde28b42b7506a12b] Merge branch
>>>> 'nand/next' of https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
>>>> git bisect bad dffddeb046466ed57038295bde28b42b7506a12b
>>>> # bad: [d154d0766bf3fe830e1bf98fb2d8648294a4a8da] Merge branch
>>>> 'for-next' of
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap.git
>>>> git bisect bad d154d0766bf3fe830e1bf98fb2d8648294a4a8da
>>>> # bad: [671b0fd929bec9ebf863cb376baf9b9d1226a510] Merge branch 'master'
>>>> of https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git
>>>> git bisect bad 671b0fd929bec9ebf863cb376baf9b9d1226a510
>>>> # good: [f4fb100039e96211609dfc44fb24b9e4a8a0f2f9] Merge tag
>>>> 's390-7.2-4' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
>>>> git bisect good f4fb100039e96211609dfc44fb24b9e4a8a0f2f9
>>>> # good: [c296d513e034df93ddb230ad1c7a118d6b58750c] Merge branch
>>>> 'arm/fixes' of https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
>>>> git bisect good c296d513e034df93ddb230ad1c7a118d6b58750c
>>>> # good: [de53c0a584f08f31eb957f8c849a59fb33b27fa8] Merge branch 'for-rc'
>>>> of https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
>>>> git bisect good de53c0a584f08f31eb957f8c849a59fb33b27fa8
>>>> # bad: [a6969de5a038bc08a948ada52bd7199a8dfa6cec] Merge branch
>>>> 'usb-linus' of
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
>>>> git bisect bad a6969de5a038bc08a948ada52bd7199a8dfa6cec
>>>> # good: [15eab2dd63a97587405ab6bfaad9728d5b35b5fe] Merge tag
>>>> 'asoc-fix-v7.2-rc2' of
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus
>>>> git bisect good 15eab2dd63a97587405ab6bfaad9728d5b35b5fe
>>>> # bad: [c2e819be6a5c7f34344926b4bd7e3dfca58cf48a] usb: gadget: printer:
>>>> fix infinite loop in printer_read()
>>>> git bisect bad c2e819be6a5c7f34344926b4bd7e3dfca58cf48a
>>>> # good: [29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b] USB: gadget:
>>>> snps-udc: fix device name leak on probe failure
>>>> git bisect good 29a142d3e8b35ebc9e0bcc78f4bc26c9b6a9ac0b
>>>> # good: [1febec7e47cdcd01f43fb0211094e3010474666e] usb: gadget: f_ncm:
>>>> validate datagram bounds in ncm_unwrap_ntb()
>>>> git bisect good 1febec7e47cdcd01f43fb0211094e3010474666e
>>>> # bad: [0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb] usb: gadget: udc: bdc:
>>>> free IRQ and drain func_wake_notify before teardown
>>>> git bisect bad 0583f2fbf8f86ae3a0ce054f96783dd83e65d9bb
>>>> # bad: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb: typec: ucsi: Fix
>>>> race condition and ordering in port unregistration
>>>> git bisect bad 7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261
>>>> # first bad commit: [7aa7d4bf9d3fa9a6a47b640ad103ab433b7ff261] usb:
>>>> typec: ucsi: Fix race condition and ordering in port unregistration
>>>>>
>>>>> 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>
>>>>> ---
>>>>>     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);
>>>
>>> Hi Chaitanya,
>>> Thank you for spotting this!
>>>
>>> Could you verify if calling lockdep_unregister_key(&connector[i].lock_key)
>>> after ucsi_unregister_port(con) resolves the issue?
>>>
>>>        for (con = connector; con->port; con++)
>>>               ucsi_unregister_port(con);
>>>        for (i = 0; i < ucsi->cap.num_connectors; i++)
>>>                  lockdep_unregister_key(&connector[i].lock_key);
>>>
>>> I will also try testing this on my side.
>>
>> Thank you Andrei for the patch. It seems to work for us.
>>
> 
> Hi Chaitanya,
> 
> Thanks for the testing!
> I will prepare a fix or maybe V2 of this patch.
> 
> Could you please share your configuration details? Specifically, do you
> have any of the following enabled?
> CONFIG_DEBUG_LOCKDEP
> CONFIG_LOCK_STAT
> CONFIG_LOCKDEP_SMALL
> CONFIG_DEBUG_LOCKDEP
> CONFIG_PROVE_LOCKING
> 
> I am still  not able to reproduce it.

I hope this is accessible.

https://intel-gfx-ci.01.org/tree/linux-next/next-20260713/kconfig.txt

We also see this issue only in one of the CI machines. So it might not 
be the config.

==
Chaitanya

> 
> Thanks,
> Andrei


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-16 13:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-07-16  8:28 ` REGRESSION: " Borah, Chaitanya Kumar
2026-07-16  9:15   ` Andrei Kuchynski
2026-07-16 13:23     ` Borah, Chaitanya Kumar
2026-07-16 13:41       ` Andrei Kuchynski
2026-07-16 13:59         ` Borah, Chaitanya Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox