Linux USB
 help / color / mirror / Atom feed
From: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
To: Andrei Kuchynski <akuchynski@chromium.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Jameson Thies <jthies@google.com>,
	Benson Leung <bleung@chromium.org>, <linux-usb@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>,
	Pooja Katiyar <pooja.katiyar@intel.com>,
	"Johan Hovold" <johan@kernel.org>,
	Hsin-Te Yuan <yuanhsinte@chromium.org>,
	"Myrrh Periwinkle" <myrrhperiwinkle@qtmlabs.xyz>,
	Linyu Yuan <quic_linyyuan@quicinc.com>,
	Jack Pham <quic_jackp@quicinc.com>, <stable@vger.kernel.org>
Subject: Re: REGRESSION: [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration
Date: Thu, 16 Jul 2026 19:29:37 +0530	[thread overview]
Message-ID: <47fa61c2-de68-4ec7-9edd-8f7d5f360834@intel.com> (raw)
In-Reply-To: <CAMMMRMdZciCCmfOv0_PhX_VfPtXdMEwNe-CXOLWRSmFYBAEL7Q@mail.gmail.com>



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


      reply	other threads:[~2026-07-16 13:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07 14:17 [PATCH] usb: typec: ucsi: Fix race condition and ordering in port unregistration Andrei Kuchynski
2026-07-08 15:15 ` Benson Leung
2026-07-13  9:08 ` Heikki Krogerus
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47fa61c2-de68-4ec7-9edd-8f7d5f360834@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