mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
To: Andrei Kuchynski <akuchynski@chromium.org>,
	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>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Cc: 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: REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
Date: Thu, 16 Jul 2026 13:58:05 +0530	[thread overview]
Message-ID: <22064276-6c56-411a-9f20-6917ceeb865f@intel.com> (raw)
In-Reply-To: <20260707141736.1635698-1-akuchynski@chromium.org>

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);
>   	}
>   


  parent reply	other threads:[~2026-07-16  8:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07 14:17 Andrei Kuchynski
2026-07-08 15:15 ` Benson Leung
2026-07-13  9:08 ` Heikki Krogerus
2026-07-16  8:28 ` Borah, Chaitanya Kumar [this message]
2026-07-16  9:15   ` REGRESSION: " 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

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=22064276-6c56-411a-9f20-6917ceeb865f@intel.com \
    --to=chaitanya.kumar.borah@intel.com \
    --cc=abhishekpandit@chromium.org \
    --cc=akuchynski@chromium.org \
    --cc=bleung@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=johan@kernel.org \
    --cc=jthies@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=myrrhperiwinkle@qtmlabs.xyz \
    --cc=pooja.katiyar@intel.com \
    --cc=quic_jackp@quicinc.com \
    --cc=quic_linyyuan@quicinc.com \
    --cc=stable@vger.kernel.org \
    --cc=yuanhsinte@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox