[PATCH] usb: typec: ucsi: fix teardown races with late notifications

[PATCH] usb: typec: ucsi: fix teardown races with late notifications

[Iván Ezequiel Rodriguez]

ucsi_acpi_remove() freed the UCSI instance before removing the ACPI
notify handler. A concurrent notify could call into ucsi_acpi_notify()
and use ua->ucsi after it was destroyed.

Clear ucsi->ntfy before disabling PPM notifications and NULL the
connector array after free so ucsi_connector_change() cannot schedule
work on a dangling connector while a backend still delivers events.

Tested: built drivers/usb/typec/ucsi/ with CONFIG_TYPEC_UCSI=m and
CONFIG_UCSI_ACPI=m via docker kbuild; checkpatch clean.

Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>
---
 drivers/usb/typec/ucsi/ucsi.c      | 11 +++++++++++
 drivers/usb/typec/ucsi/ucsi_acpi.c | 11 ++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index bef3f9b71d71..3395614764cf 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -2369,6 +2369,15 @@ void ucsi_unregister(struct ucsi *ucsi)
 
 	ucsi_debugfs_unregister(ucsi);
 
+	/*
+	 * Stop accepting connector-change events before the PPM disable
+	 * command and before freeing connectors. Backends may still deliver
+	 * a late notification (e.g. ACPI) until their own handler is removed;
+	 * with ntfy cleared, ucsi_connector_change() returns early instead of
+	 * scheduling work on a connector that is about to be freed.
+	 */
+	ucsi->ntfy = 0;
+
 	/* Disable notifications */
 	ucsi->ops->async_control(ucsi, cmd);
 
@@ -2382,6 +2391,8 @@ void ucsi_unregister(struct ucsi *ucsi)
 	}
 
 	kfree(ucsi->connector);
+	ucsi->connector = NULL;
+	memset(&ucsi->cap, 0, sizeof(ucsi->cap));
 }
 EXPORT_SYMBOL_GPL(ucsi_unregister);
 
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 18286d3e9cc5..5fc485121dbb 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -256,11 +256,16 @@ static void ucsi_acpi_remove(struct platform_device *pdev)
 {
 	struct ucsi_acpi *ua = platform_get_drvdata(pdev);
 
-	ucsi_unregister(ua->ucsi);
-	ucsi_destroy(ua->ucsi);
-
+	/*
+	 * Drop the ACPI notify handler before tearing down the UCSI instance.
+	 * Otherwise a concurrent notify can race into ucsi_acpi_notify() and
+	 * use ua->ucsi after it has been freed.
+	 */
 	acpi_remove_notify_handler(ACPI_HANDLE(&pdev->dev), ACPI_DEVICE_NOTIFY,
 				   ucsi_acpi_notify);
+
+	ucsi_unregister(ua->ucsi);
+	ucsi_destroy(ua->ucsi);
 }
 
 static int ucsi_acpi_suspend(struct device *dev)
-- 
2.43.0

Re: [PATCH] usb: typec: ucsi: fix teardown races with late notifications

[Huang Wei]

Hi Iván,

I've been poking at the ucsi teardown path lately so I took a closer
look at this. The race is real, and moving acpi_remove_notify_handler()
ahead of ucsi_unregister()/ucsi_destroy() is clearly the right call for
the ACPI path.

One thing I couldn't convince myself of: does acpi_remove_notify_handler()
wait for a handler that's already running on another CPU? From what I
can tell ACPICA removes the handler node under the device lock but
doesn't flush an in-flight dispatch, so a notify that's already entered
ucsi_acpi_notify() could still dereference ua->ucsi after the handler
is gone and ucsi_destroy() has freed it. The reorder handles new
notifies fine, I just wasn't sure it handles the one that's already
mid-flight. Did you look at that?

On the ucsi.c changes: setting ntfy to 0 before NULLing the connector
array looks right to me, ucsi_connector_change() bails on ntfy before
it touches the connector. But ucsi_notify_common() also does
complete(&ucsi->complete) and the backend's read_cci(), neither of
which is gated by ntfy. So for the non-ACPI backends (glink, ccg)
that can't be fixed by reordering the ACPI handler -- is the ucsi.c
part actually enough on its own, or do they need their own teardown
ordering? Put differently, is the ACPI reorder the only real fix and
the rest belt-and-suspenders?

Only build-tested too -- would help to know how the race was found
and what makes the reorder sufficient on its own.

Not objecting, I'd just like to understand the in-flight case.

Thanks,
Huang Wei

[PATCH v2] usb: typec: ucsi: acpi: fix use-after-free on driver removal

[Iván Ezequiel Rodriguez]

ucsi_acpi_remove() destroys the UCSI instance before removing the ACPI
notify handler:

	ucsi_unregister(ua->ucsi);
	ucsi_destroy(ua->ucsi);

	acpi_remove_notify_handler(...);

ucsi_acpi_notify() dereferences ua->ucsi, so a notify arriving after
ucsi_destroy() uses freed memory:

  CPU0					CPU1
  ----					----
  ucsi_acpi_remove()
    ucsi_unregister()
    ucsi_destroy()
      kfree(ucsi)
					ucsi_acpi_notify()
					  ua->ucsi->ops->read_cci()   <-- UAF

Simply removing the handler before ucsi_unregister() is not correct
either. ucsi_unregister() drains work that needs the notify path to make
progress: ucsi_handle_connector_change() issues GET_CONNECTOR_STATUS and
ucsi_unregister_port() drains and destroys con->wq, and those commands
block in wait_for_completion_timeout() on ucsi->complete for up to
UCSI_TIMEOUT_MS. That completion is signalled only from
ucsi_notify_common(), i.e. from the notify handler. Tearing the handler
down first would leave cancel_work_sync() and destroy_workqueue()
waiting the full timeout for a completion that can no longer arrive.

Moving the removal between ucsi_unregister() and ucsi_destroy() is not
sufficient on its own: at that point the connector array has already
been freed, so a late notify reaching ucsi_connector_change() would
queue work on a freed connector.

Teardown therefore needs two properties at the same time: no new
connector changes once connectors start going away, but the notify path
still available for command and acknowledge completions until that work
has been drained. Whether the PPM actually produces those completions is
a firmware matter; what changes here is that the path able to deliver
them is no longer torn down first.

Introduce a quiescing state, local to the ACPI backend, that provides
both. ucsi_acpi_remove() sets ua->quiescing under ua->notify_lock before
calling ucsi_unregister(); ucsi_acpi_notify() takes the same lock and,
when quiescing, reduces the CCI to the bits that ucsi_notify_common()
consumes for completions. ucsi_notify_common() looks at exactly
UCSI_CCI_BUSY, the connector number, UCSI_CCI_ACK_COMPLETE and
UCSI_CCI_COMMAND_COMPLETE; keeping the first and the last two preserves
the completion and bogus-data behaviour unchanged, while clearing the
connector number makes ucsi_connector_change() unreachable. The CCI that
the command path inspects is unaffected, because
ucsi_sync_control_common() re-reads it from the interface after the
completion.

The resulting order is:

  mutex_lock(&ua->notify_lock);
  ua->quiescing = true;			-- no new connector work
  mutex_unlock(&ua->notify_lock);

  ucsi_unregister();			-- drains work, notify path still
					   available for completions

  acpi_remove_notify_handler();		-- unlinks, then flushes
					   kacpi_notify_wq

  ucsi_destroy();			-- no notify can be in flight

which gives the following happens-before chain:

- A notify that acquires notify_lock before ucsi_acpi_remove() runs to
  completion while remove() waits on the lock, so any schedule_work() it
  performs happens before ucsi_unregister() starts cancelling.
- A notify that acquires notify_lock after remove() released it observes
  quiescing == true, so it cannot reach ucsi_connector_change() and
  cannot touch ucsi->connector.
- acpi_remove_notify_handler() unlinks the handler and then calls
  acpi_os_wait_events_complete(), which flushes kacpi_notify_wq, so a
  notify already dispatched on another CPU has returned before
  ucsi_destroy() frees the instance.

notify_lock is never held across ucsi_unregister() or
acpi_remove_notify_handler(); holding it there would deadlock against
the notify work those calls wait for. It is only ever taken as a leaf:
ucsi_notify_common() and ucsi_connector_change() take no locks, so it
cannot invert against ucsi->ppm_lock or con->lock, which the drained
work holds while waiting for the completion. The handler runs from
kacpi_notify_wq via acpi_os_execute(OSL_NOTIFY_HANDLER, ...), i.e. in
process context, so sleeping on the mutex is allowed.

The probe error path already removes the handler before ucsi_destroy()
and is left unchanged.

Fixes: f56de278e8ec ("usb: typec: ucsi: acpi: Move to the new API")
Cc: stable@vger.kernel.org
Reported-by: Fan Wu <fanwu01@zju.edu.cn>
Link: https://lore.kernel.org/all/20260718021142.3146566-1-fanwu01@zju.edu.cn/
Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>
---

Notes:
    Hi Heikki, Greg,
    
    v2 is a rewrite rather than an incremental fixup. v1 moved
    acpi_remove_notify_handler() ahead of the teardown, and that ordering is
    wrong for the reason Fan Wu had already documented in [1]: the work that
    ucsi_unregister() drains can be waiting for a completion that only the
    notify path delivers. This version keeps the handler installed across
    ucsi_unregister() and adds an ACPI-local quiescing state instead.
    
    Wei Huang asked on v1 whether acpi_remove_notify_handler() waits for a
    callback that already entered on another CPU. It does: for
    ACPI_DEVICE_NOTIFY it calls acpi_os_wait_events_complete() after
    unlinking the handler (drivers/acpi/acpica/evxface.c), that flushes
    kacpi_notify_wq (drivers/acpi/osl.c), and device-notify dispatch runs on
    that same workqueue through acpi_os_execute(OSL_NOTIFY_HANDLER, ...).
    So the raw call is already the barrier, and acpi_dev_remove_notify_handler()
    would only add a second flush. Chasing that question is what surfaced the
    harder half of the problem, which is what this version is about.
    
    On [1]: that patch kept the handler installed across ucsi_unregister()
    for exactly the right reason, and this version preserves that property.
    What it did not cover is the window you described in that thread, where a
    notify arriving after ucsi_unregister() has freed the connectors still
    reaches ucsi_connector_change(). You also asked to keep the solution
    inside ucsi_acpi.c rather than redesigning the core, which is what this
    does.
    
    Changes since v1:
    - Do not remove the notify handler before ucsi_unregister().
    - Add the quiescing state, so connector changes stop while the notify
      path stays available for completions.
    - Drop the ucsi.c changes from v1 (ntfy = 0, connector = NULL, cap = 0).
      I could not show they were needed for the other backends, and they do
      not belong in the same patch as the ACPI lifetime fix.
    - Correct the Fixes: tag. v1 pointed at 8243edf44152, which added the
      driver; the current ordering came from f56de278e8ec.
    - Credit Fan Wu, who reported the underlying use-after-free first.
    - Use mutex_init() rather than devm_mutex_init(), which only appeared in
      4cd47222e435 (2024) and would be a needlessly modern dependency for a
      fix tagged for stable from a 2019 commit.
    
    Testing
    
    I have no machine that exercises the UCSI ACPI path, so this was tested
    with a software PPM backend that drives the real UCSI core
    (ucsi_create/ucsi_register/ucsi_unregister/ucsi_destroy/
    ucsi_notify_common) and reproduces the ACPI notify protocol, including
    the deferral to a percpu workqueue. All three candidate teardown
    orderings were run against it: the one from v1, the one from [1] and the
    one in this patch. v7.3-rc2, KASAN + lockdep + PROVE_LOCKING +
    DEBUG_MUTEXES, QEMU, oops=panic.
    
    Each case below parks a connector work in wait_for_completion_timeout()
    before teardown starts, and asserts that precondition rather than
    assuming it.
    
      teardown ordering                     result
      ------------------------------------  ----------------------------
      quiesce, unregister, unlink (this)    148 ms, clean, with a late
                                            connector notify fired after
                                            ucsi_unregister() returned
      unlink, unregister (v1)               10595 ms stall
      unregister, unlink (as in [1]), a     KASAN slab-use-after-free in
      connector notify landing in the       queue_work_on(), then a GP
      window                                fault in the kworker that
                                            picked up the freed work
      nothing in flight (this)              146 ms, clean
    
    Repeated with the teardown starting while ucsi_init_work() is still
    running, so that cancel_delayed_work_sync(&ucsi->work) has to drain an
    init command parked on the completion: this patch takes 2299 ms and
    completes cleanly, of which 1500 ms is the injected command delay, while
    the v1 ordering stalls for 10089 ms and the init gives up with
    -ETIMEDOUT.
    
    Two deterministic checks of the properties the patch claims:
    
    - CCI mask. A single CCI carrying both connector 1 and COMMAND_COMPLETE
      (0x80000002) is delivered while quiescing: the completion is signalled
      and EVENT_PENDING stays clear, i.e. ucsi_connector_change() is not
      reached. The same CCI with quiescing off sets EVENT_PENDING, so the
      check is sensitive to the path it claims to block.
    
    - notify_lock as a barrier. A notify that has entered the handler is
      held inside it for 1200 ms; the store of quiescing in the teardown
      path blocks for 1215 ms behind it. This is what makes "a notify that
      started before teardown finishes its schedule_work() before
      ucsi_unregister() begins cancelling" an ordering guarantee rather
      than a likelihood.
    
    Soak: 1000 teardown cycles with four threads hammering the notify path
    concurrently with the quiesce sequence, repeated at 1, 2, 4 and 8 vCPUs,
    so 4000 teardowns in total. No stall, no KASAN report and no lockdep
    splat in any configuration. A separate KCSAN build ran 200 of those
    cycles at 4 vCPUs with no data race reported in any UCSI path.
    
    What this does not cover: no real ACPI hardware, so the ACPICA drain
    described above is established by reading evxface.c and osl.c rather
    than by execution; and the LG gram quirk path is untouched and
    unexercised.
    
    The harness is not part of this patch. I can post it separately if it
    is useful.
    
    [1] https://lore.kernel.org/all/20260718021142.3146566-1-fanwu01@zju.edu.cn/

 drivers/usb/typec/ucsi/ucsi_acpi.c | 53 ++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 18286d3e9cc5..61bba7625d17 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -24,6 +24,15 @@ struct ucsi_acpi {
 	bool check_bogus_event;
 	guid_t guid;
 	u64 cmd;
+	/*
+	 * notify_lock serialises ucsi_acpi_notify() against the start of
+	 * teardown, so that @quiescing is observed by every notify that has
+	 * not yet run. It must not be held across ucsi_unregister(), whose
+	 * drained work may depend on the notify path, nor across
+	 * acpi_remove_notify_handler(), which flushes notify work.
+	 */
+	struct mutex notify_lock;
+	bool quiescing;
 };
 
 static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func)
@@ -179,11 +188,31 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
 	u32 cci;
 	int ret;
 
+	mutex_lock(&ua->notify_lock);
+
 	ret = ua->ucsi->ops->read_cci(ua->ucsi, &cci);
 	if (ret)
-		return;
+		goto out_unlock;
+
+	/*
+	 * Once teardown has started the connectors are being unregistered and
+	 * freed, so a connector change must not be reported any more. Command
+	 * and acknowledge completions must still be able to reach the core:
+	 * ucsi_unregister() drains connector and partner work that can be
+	 * blocked in wait_for_completion_timeout() on ucsi->complete, and that
+	 * completion is only signalled from here. Keep exactly the bits that
+	 * ucsi_notify_common() needs for that, which drops the connector
+	 * number and with it the path to ucsi_connector_change(). The busy
+	 * indicator is kept so that bogus CCI data is still ignored.
+	 */
+	if (ua->quiescing)
+		cci &= UCSI_CCI_BUSY | UCSI_CCI_ACK_COMPLETE |
+		       UCSI_CCI_COMMAND_COMPLETE;
 
 	ucsi_notify_common(ua->ucsi, cci);
+
+out_unlock:
+	mutex_unlock(&ua->notify_lock);
 }
 
 static int ucsi_acpi_probe(struct platform_device *pdev)
@@ -219,6 +248,8 @@ static int ucsi_acpi_probe(struct platform_device *pdev)
 
 	ua->dev = &pdev->dev;
 
+	mutex_init(&ua->notify_lock);
+
 	id = dmi_first_match(ucsi_acpi_quirks);
 	if (id)
 		ops = id->driver_data;
@@ -256,11 +287,29 @@ static void ucsi_acpi_remove(struct platform_device *pdev)
 {
 	struct ucsi_acpi *ua = platform_get_drvdata(pdev);
 
+	/*
+	 * Stop reporting connector changes, but keep the notify handler
+	 * installed so that the work ucsi_unregister() drains can still be
+	 * reached by the command completions it may be waiting for. Any notify
+	 * that already passed this point runs to completion first, so no
+	 * connector work can be queued once ucsi_unregister() starts.
+	 */
+	mutex_lock(&ua->notify_lock);
+	ua->quiescing = true;
+	mutex_unlock(&ua->notify_lock);
+
 	ucsi_unregister(ua->ucsi);
-	ucsi_destroy(ua->ucsi);
 
+	/*
+	 * Now that no work is left to serve, drop the handler. This unlinks it
+	 * and then calls acpi_os_wait_events_complete(), which flushes
+	 * kacpi_notify_wq, so a notify running on another CPU has returned
+	 * before ucsi_destroy() frees the instance that it dereferences.
+	 */
 	acpi_remove_notify_handler(ACPI_HANDLE(&pdev->dev), ACPI_DEVICE_NOTIFY,
 				   ucsi_acpi_notify);
+
+	ucsi_destroy(ua->ucsi);
 }
 
 static int ucsi_acpi_suspend(struct device *dev)
-- 
2.43.0

Re: [PATCH] usb: typec: ucsi: fix teardown races with late notifications

[Huang Wei]

Hi Iván,

I've been poking at the ucsi teardown path lately so I took a closer
look at this. The race is real, and moving acpi_remove_notify_handler()
ahead of ucsi_unregister()/ucsi_destroy() is clearly the right call for
the ACPI path.

One thing I couldn't convince myself of: does acpi_remove_notify_handler()
wait for a handler that's already running on another CPU? From what I
can tell ACPICA removes the handler node under the device lock but
doesn't flush an in-flight dispatch, so a notify that's already entered
ucsi_acpi_notify() could still dereference ua->ucsi after the handler
is gone and ucsi_destroy() has freed it. The reorder handles new
notifies fine, I just wasn't sure it handles the one that's already
mid-flight. Did you look at that?

On the ucsi.c changes: setting ntfy to 0 before NULLing the connector
array looks right to me, ucsi_connector_change() bails on ntfy before
it touches the connector. But ucsi_notify_common() also does
complete(&ucsi->complete) and the backend's read_cci(), neither of
which is gated by ntfy. So for the non-ACPI backends (glink, ccg)
that can't be fixed by reordering the ACPI handler -- is the ucsi.c
part actually enough on its own, or do they need their own teardown
ordering? Put differently, is the ACPI reorder the only real fix and
the rest belt-and-suspenders?

Only build-tested too -- would help to know how the race was found
and what makes the reorder sufficient on its own.

Not objecting, I'd just like to understand the in-flight case.

Thanks,
Huang Wei

Re: [PATCH v2] usb: typec: ucsi: acpi: fix use-after-free on driver removal

[Huang Wei]

Hi Iván,

This is a nice piece of work. (And apologies for the duplicate of my
earlier note on v1 -- that was an accidental resend.)

I went and checked the ACPICA side after reading your notes:
acpi_remove_notify_handler() does call acpi_os_wait_events_complete()
on the device-notify path, so the flush argument holds up. The stall
you measured for the v1 ordering is a good catch too -- I was happy
enough with the plain reorder on v1 and would have missed that
ucsi_unregister() depends on the notify path for its completions.

The quiescing + CCI masking looks right to me, and keeping the fix
ACPI-local answers my question about the other backends.

If you do post the harness separately I'd find it useful -- the
ordering comparison alone is worth having in the archives.

Reviewed-by: Huang Wei <huangwei@kylinos.cn>

Thanks,
Huang Wei

Re: [PATCH v2] usb: typec: ucsi: acpi: fix use-after-free on driver removal

[Heikki Krogerus]

On Thu, Sep 03, 2026 at 08:21:21PM -0300, Iván Ezequiel Rodriguez wrote:
> ucsi_acpi_remove() destroys the UCSI instance before removing the ACPI
> notify handler:
> 
> 	ucsi_unregister(ua->ucsi);
> 	ucsi_destroy(ua->ucsi);
> 
> 	acpi_remove_notify_handler(...);
> 
> ucsi_acpi_notify() dereferences ua->ucsi, so a notify arriving after
> ucsi_destroy() uses freed memory:
> 
>   CPU0					CPU1
>   ----					----
>   ucsi_acpi_remove()
>     ucsi_unregister()
>     ucsi_destroy()
>       kfree(ucsi)
> 					ucsi_acpi_notify()
> 					  ua->ucsi->ops->read_cci()   <-- UAF
> 
> Simply removing the handler before ucsi_unregister() is not correct
> either. ucsi_unregister() drains work that needs the notify path to make
> progress: ucsi_handle_connector_change() issues GET_CONNECTOR_STATUS and
> ucsi_unregister_port() drains and destroys con->wq, and those commands
> block in wait_for_completion_timeout() on ucsi->complete for up to
> UCSI_TIMEOUT_MS. That completion is signalled only from
> ucsi_notify_common(), i.e. from the notify handler. Tearing the handler
> down first would leave cancel_work_sync() and destroy_workqueue()
> waiting the full timeout for a completion that can no longer arrive.
> 
> Moving the removal between ucsi_unregister() and ucsi_destroy() is not
> sufficient on its own: at that point the connector array has already
> been freed, so a late notify reaching ucsi_connector_change() would
> queue work on a freed connector.
> 
> Teardown therefore needs two properties at the same time: no new
> connector changes once connectors start going away, but the notify path
> still available for command and acknowledge completions until that work
> has been drained. Whether the PPM actually produces those completions is
> a firmware matter; what changes here is that the path able to deliver
> them is no longer torn down first.
> 
> Introduce a quiescing state, local to the ACPI backend, that provides
> both. ucsi_acpi_remove() sets ua->quiescing under ua->notify_lock before
> calling ucsi_unregister(); ucsi_acpi_notify() takes the same lock and,
> when quiescing, reduces the CCI to the bits that ucsi_notify_common()
> consumes for completions. ucsi_notify_common() looks at exactly
> UCSI_CCI_BUSY, the connector number, UCSI_CCI_ACK_COMPLETE and
> UCSI_CCI_COMMAND_COMPLETE; keeping the first and the last two preserves
> the completion and bogus-data behaviour unchanged, while clearing the
> connector number makes ucsi_connector_change() unreachable. The CCI that
> the command path inspects is unaffected, because
> ucsi_sync_control_common() re-reads it from the interface after the
> completion.
> 
> The resulting order is:
> 
>   mutex_lock(&ua->notify_lock);
>   ua->quiescing = true;			-- no new connector work
>   mutex_unlock(&ua->notify_lock);
> 
>   ucsi_unregister();			-- drains work, notify path still
> 					   available for completions
> 
>   acpi_remove_notify_handler();		-- unlinks, then flushes
> 					   kacpi_notify_wq
> 
>   ucsi_destroy();			-- no notify can be in flight
> 
> which gives the following happens-before chain:
> 
> - A notify that acquires notify_lock before ucsi_acpi_remove() runs to
>   completion while remove() waits on the lock, so any schedule_work() it
>   performs happens before ucsi_unregister() starts cancelling.
> - A notify that acquires notify_lock after remove() released it observes
>   quiescing == true, so it cannot reach ucsi_connector_change() and
>   cannot touch ucsi->connector.
> - acpi_remove_notify_handler() unlinks the handler and then calls
>   acpi_os_wait_events_complete(), which flushes kacpi_notify_wq, so a
>   notify already dispatched on another CPU has returned before
>   ucsi_destroy() frees the instance.
> 
> notify_lock is never held across ucsi_unregister() or
> acpi_remove_notify_handler(); holding it there would deadlock against
> the notify work those calls wait for. It is only ever taken as a leaf:
> ucsi_notify_common() and ucsi_connector_change() take no locks, so it
> cannot invert against ucsi->ppm_lock or con->lock, which the drained
> work holds while waiting for the completion. The handler runs from
> kacpi_notify_wq via acpi_os_execute(OSL_NOTIFY_HANDLER, ...), i.e. in
> process context, so sleeping on the mutex is allowed.
> 
> The probe error path already removes the handler before ucsi_destroy()
> and is left unchanged.
> 
> Fixes: f56de278e8ec ("usb: typec: ucsi: acpi: Move to the new API")
> Cc: stable@vger.kernel.org
> Reported-by: Fan Wu <fanwu01@zju.edu.cn>
> Link: https://lore.kernel.org/all/20260718021142.3146566-1-fanwu01@zju.edu.cn/
> Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>

You could have used guard(mutex) in ucsi_acpi_notify(), but that's
not a huge problem.

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Notes:
>     Hi Heikki, Greg,
>     
>     v2 is a rewrite rather than an incremental fixup. v1 moved
>     acpi_remove_notify_handler() ahead of the teardown, and that ordering is
>     wrong for the reason Fan Wu had already documented in [1]: the work that
>     ucsi_unregister() drains can be waiting for a completion that only the
>     notify path delivers. This version keeps the handler installed across
>     ucsi_unregister() and adds an ACPI-local quiescing state instead.
>     
>     Wei Huang asked on v1 whether acpi_remove_notify_handler() waits for a
>     callback that already entered on another CPU. It does: for
>     ACPI_DEVICE_NOTIFY it calls acpi_os_wait_events_complete() after
>     unlinking the handler (drivers/acpi/acpica/evxface.c), that flushes
>     kacpi_notify_wq (drivers/acpi/osl.c), and device-notify dispatch runs on
>     that same workqueue through acpi_os_execute(OSL_NOTIFY_HANDLER, ...).
>     So the raw call is already the barrier, and acpi_dev_remove_notify_handler()
>     would only add a second flush. Chasing that question is what surfaced the
>     harder half of the problem, which is what this version is about.
>     
>     On [1]: that patch kept the handler installed across ucsi_unregister()
>     for exactly the right reason, and this version preserves that property.
>     What it did not cover is the window you described in that thread, where a
>     notify arriving after ucsi_unregister() has freed the connectors still
>     reaches ucsi_connector_change(). You also asked to keep the solution
>     inside ucsi_acpi.c rather than redesigning the core, which is what this
>     does.
>     
>     Changes since v1:
>     - Do not remove the notify handler before ucsi_unregister().
>     - Add the quiescing state, so connector changes stop while the notify
>       path stays available for completions.
>     - Drop the ucsi.c changes from v1 (ntfy = 0, connector = NULL, cap = 0).
>       I could not show they were needed for the other backends, and they do
>       not belong in the same patch as the ACPI lifetime fix.
>     - Correct the Fixes: tag. v1 pointed at 8243edf44152, which added the
>       driver; the current ordering came from f56de278e8ec.
>     - Credit Fan Wu, who reported the underlying use-after-free first.
>     - Use mutex_init() rather than devm_mutex_init(), which only appeared in
>       4cd47222e435 (2024) and would be a needlessly modern dependency for a
>       fix tagged for stable from a 2019 commit.
>     
>     Testing
>     
>     I have no machine that exercises the UCSI ACPI path, so this was tested
>     with a software PPM backend that drives the real UCSI core
>     (ucsi_create/ucsi_register/ucsi_unregister/ucsi_destroy/
>     ucsi_notify_common) and reproduces the ACPI notify protocol, including
>     the deferral to a percpu workqueue. All three candidate teardown
>     orderings were run against it: the one from v1, the one from [1] and the
>     one in this patch. v7.3-rc2, KASAN + lockdep + PROVE_LOCKING +
>     DEBUG_MUTEXES, QEMU, oops=panic.
>     
>     Each case below parks a connector work in wait_for_completion_timeout()
>     before teardown starts, and asserts that precondition rather than
>     assuming it.
>     
>       teardown ordering                     result
>       ------------------------------------  ----------------------------
>       quiesce, unregister, unlink (this)    148 ms, clean, with a late
>                                             connector notify fired after
>                                             ucsi_unregister() returned
>       unlink, unregister (v1)               10595 ms stall
>       unregister, unlink (as in [1]), a     KASAN slab-use-after-free in
>       connector notify landing in the       queue_work_on(), then a GP
>       window                                fault in the kworker that
>                                             picked up the freed work
>       nothing in flight (this)              146 ms, clean
>     
>     Repeated with the teardown starting while ucsi_init_work() is still
>     running, so that cancel_delayed_work_sync(&ucsi->work) has to drain an
>     init command parked on the completion: this patch takes 2299 ms and
>     completes cleanly, of which 1500 ms is the injected command delay, while
>     the v1 ordering stalls for 10089 ms and the init gives up with
>     -ETIMEDOUT.
>     
>     Two deterministic checks of the properties the patch claims:
>     
>     - CCI mask. A single CCI carrying both connector 1 and COMMAND_COMPLETE
>       (0x80000002) is delivered while quiescing: the completion is signalled
>       and EVENT_PENDING stays clear, i.e. ucsi_connector_change() is not
>       reached. The same CCI with quiescing off sets EVENT_PENDING, so the
>       check is sensitive to the path it claims to block.
>     
>     - notify_lock as a barrier. A notify that has entered the handler is
>       held inside it for 1200 ms; the store of quiescing in the teardown
>       path blocks for 1215 ms behind it. This is what makes "a notify that
>       started before teardown finishes its schedule_work() before
>       ucsi_unregister() begins cancelling" an ordering guarantee rather
>       than a likelihood.
>     
>     Soak: 1000 teardown cycles with four threads hammering the notify path
>     concurrently with the quiesce sequence, repeated at 1, 2, 4 and 8 vCPUs,
>     so 4000 teardowns in total. No stall, no KASAN report and no lockdep
>     splat in any configuration. A separate KCSAN build ran 200 of those
>     cycles at 4 vCPUs with no data race reported in any UCSI path.
>     
>     What this does not cover: no real ACPI hardware, so the ACPICA drain
>     described above is established by reading evxface.c and osl.c rather
>     than by execution; and the LG gram quirk path is untouched and
>     unexercised.
>     
>     The harness is not part of this patch. I can post it separately if it
>     is useful.
>     
>     [1] https://lore.kernel.org/all/20260718021142.3146566-1-fanwu01@zju.edu.cn/
> 
>  drivers/usb/typec/ucsi/ucsi_acpi.c | 53 ++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> index 18286d3e9cc5..61bba7625d17 100644
> --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> @@ -24,6 +24,15 @@ struct ucsi_acpi {
>  	bool check_bogus_event;
>  	guid_t guid;
>  	u64 cmd;
> +	/*
> +	 * notify_lock serialises ucsi_acpi_notify() against the start of
> +	 * teardown, so that @quiescing is observed by every notify that has
> +	 * not yet run. It must not be held across ucsi_unregister(), whose
> +	 * drained work may depend on the notify path, nor across
> +	 * acpi_remove_notify_handler(), which flushes notify work.
> +	 */
> +	struct mutex notify_lock;
> +	bool quiescing;
>  };
>  
>  static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func)
> @@ -179,11 +188,31 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
>  	u32 cci;
>  	int ret;
>  
> +	mutex_lock(&ua->notify_lock);
> +
>  	ret = ua->ucsi->ops->read_cci(ua->ucsi, &cci);
>  	if (ret)
> -		return;
> +		goto out_unlock;
> +
> +	/*
> +	 * Once teardown has started the connectors are being unregistered and
> +	 * freed, so a connector change must not be reported any more. Command
> +	 * and acknowledge completions must still be able to reach the core:
> +	 * ucsi_unregister() drains connector and partner work that can be
> +	 * blocked in wait_for_completion_timeout() on ucsi->complete, and that
> +	 * completion is only signalled from here. Keep exactly the bits that
> +	 * ucsi_notify_common() needs for that, which drops the connector
> +	 * number and with it the path to ucsi_connector_change(). The busy
> +	 * indicator is kept so that bogus CCI data is still ignored.
> +	 */
> +	if (ua->quiescing)
> +		cci &= UCSI_CCI_BUSY | UCSI_CCI_ACK_COMPLETE |
> +		       UCSI_CCI_COMMAND_COMPLETE;
>  
>  	ucsi_notify_common(ua->ucsi, cci);
> +
> +out_unlock:
> +	mutex_unlock(&ua->notify_lock);
>  }
>  
>  static int ucsi_acpi_probe(struct platform_device *pdev)
> @@ -219,6 +248,8 @@ static int ucsi_acpi_probe(struct platform_device *pdev)
>  
>  	ua->dev = &pdev->dev;
>  
> +	mutex_init(&ua->notify_lock);
> +
>  	id = dmi_first_match(ucsi_acpi_quirks);
>  	if (id)
>  		ops = id->driver_data;
> @@ -256,11 +287,29 @@ static void ucsi_acpi_remove(struct platform_device *pdev)
>  {
>  	struct ucsi_acpi *ua = platform_get_drvdata(pdev);
>  
> +	/*
> +	 * Stop reporting connector changes, but keep the notify handler
> +	 * installed so that the work ucsi_unregister() drains can still be
> +	 * reached by the command completions it may be waiting for. Any notify
> +	 * that already passed this point runs to completion first, so no
> +	 * connector work can be queued once ucsi_unregister() starts.
> +	 */
> +	mutex_lock(&ua->notify_lock);
> +	ua->quiescing = true;
> +	mutex_unlock(&ua->notify_lock);
> +
>  	ucsi_unregister(ua->ucsi);
> -	ucsi_destroy(ua->ucsi);
>  
> +	/*
> +	 * Now that no work is left to serve, drop the handler. This unlinks it
> +	 * and then calls acpi_os_wait_events_complete(), which flushes
> +	 * kacpi_notify_wq, so a notify running on another CPU has returned
> +	 * before ucsi_destroy() frees the instance that it dereferences.
> +	 */
>  	acpi_remove_notify_handler(ACPI_HANDLE(&pdev->dev), ACPI_DEVICE_NOTIFY,
>  				   ucsi_acpi_notify);
> +
> +	ucsi_destroy(ua->ucsi);
>  }
>  
>  static int ucsi_acpi_suspend(struct device *dev)
> -- 
> 2.43.0

-- 
heikki

[RFC] usb: typec: ucsi: add software PPM harness for teardown races

[Iván Ezequiel Rodriguez]

Huang Wei asked for the software-PPM harness that produced the v2
numbers, so the ordering comparison is in the archives.

This is an RFC. I am not asking for it to be merged unless it is
useful to keep. The module is gated on DEBUG_KERNEL and defaults
to n.

It drives the real UCSI core through a software PPM that owns the
notify source, the same role as ucsi_acpi.c. The three teardown
orderings and the two deterministic checks:

  scenario=0  quiesce, then unregister, then unlink   ~150 ms, clean
  scenario=1  unlink before unregister (v1)           ~10 s stall
  scenario=2  unregister, then late connector notify  KASAN UAF
  scenario=8  CCI mask + positive control             MASKOK
  scenario=9  notify_lock held by an in-flight notify BARRIEROK

  qemu ... -append 'ucsi_race_test.scenario=N oops=panic'

Link: https://lore.kernel.org/linux-usb/20260903232121.271776-1-ivanrwcm25@gmail.com/
Signed-off-by: Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>
---
 drivers/usb/typec/ucsi/Kconfig          |  16 +
 drivers/usb/typec/ucsi/Makefile         |   1 +
 drivers/usb/typec/ucsi/ucsi_race_test.c | 817 ++++++++++++++++++++++++
 3 files changed, 834 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/ucsi_race_test.c

diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index 87dd992a4b9e..87c875bac204 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -104,4 +104,20 @@ config UCSI_HUAWEI_GAOKUN
 	  To compile the driver as a module, choose M here: the module will be
 	  called ucsi_huawei_gaokun.
 
+config UCSI_TEARDOWN_RACE_TEST
+	tristate "UCSI teardown race harness"
+	depends on DEBUG_KERNEL
+	help
+	  Software PPM that drives the real UCSI core through the teardown
+	  orderings discussed on the ucsi_acpi use-after-free thread, so they
+	  can be compared under KASAN, lockdep and KCSAN without a PNP0CA0
+	  ACPI device.
+
+	  This is a developer tool, not needed on production systems. Load it
+	  with ucsi_race_test.scenario=N; see the header of ucsi_race_test.c
+	  for the scenario list.
+
+	  To compile this as a module, choose M here: the module will be
+	  called ucsi_race_test.
+
 endif
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index c7e38bf01350..c531aacfc47c 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_UCSI_PMIC_GLINK)		+= ucsi_glink.o
 obj-$(CONFIG_CROS_EC_UCSI)		+= cros_ec_ucsi.o
 obj-$(CONFIG_UCSI_LENOVO_YOGA_C630)	+= ucsi_yoga_c630.o
 obj-$(CONFIG_UCSI_HUAWEI_GAOKUN)	+= ucsi_huawei_gaokun.o
+obj-$(CONFIG_UCSI_TEARDOWN_RACE_TEST)	+= ucsi_race_test.o
diff --git a/drivers/usb/typec/ucsi/ucsi_race_test.c b/drivers/usb/typec/ucsi/ucsi_race_test.c
new file mode 100644
index 000000000000..26172fc61e7d
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_race_test.c
@@ -0,0 +1,817 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Software PPM for UCSI teardown-ordering races
+ *
+ * Copyright (C) 2026 Iván Ezequiel Rodriguez <ivanrwcm25@gmail.com>
+ *
+ * Drives the real UCSI core (ucsi_create / ucsi_register / ucsi_unregister /
+ * ucsi_destroy / ucsi_notify_common) through a software PPM so that the
+ * teardown orderings discussed on the ucsi_acpi UAF thread can be compared
+ * under KASAN, lockdep and KCSAN without a PNP0CA0 ACPI device.
+ *
+ * The backend plays the same role as ucsi_acpi.c: it owns the notify source
+ * that signals ucsi->complete, and it is the thing that must be quiesced
+ * correctly during teardown.
+ *
+ * Module parameter ucsi_race_test.scenario:
+ *   0  quiesce connector changes, keep completions, then unlink (the fix)
+ *   1  unlink the notify source before ucsi_unregister() (v1; stalls)
+ *   2  no quiesce; late connector notify after ucsi_unregister() (UAF)
+ *   3  baseline: nothing in flight, correct order
+ *   4  concurrent notify storm during the fixed teardown
+ *   5  mid-init teardown with the fix
+ *   6  mid-init teardown with the v1 ordering (stalls)
+ *   8  CCI mask: completion kept, connector change dropped + positive control
+ *   9  notify_lock is a real barrier, not just a flag
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/workqueue.h>
+#include <linux/delay.h>
+#include <linux/ktime.h>
+#include <linux/spinlock.h>
+#include <linux/atomic.h>
+#include <linux/kthread.h>
+
+#include "ucsi.h"
+
+#define RACE_NUM_CONNECTORS	2
+#define RACE_TAG		"UCSI-RACE"
+
+/* Connector Change Indication field of the CCI. */
+#define UCSI_RACE_CONNECTOR_FIELD	GENMASK(7, 1)
+
+static int scenario;
+module_param(scenario, int, 0444);
+MODULE_PARM_DESC(scenario,
+		 "0=fixed 1=v1-unlink-first 2=late-notify-uaf 3=baseline 4=storm 5=mid-init 6=mid-init-v1 8=cci-mask 9=lock-barrier");
+
+static int cycles = 5;
+module_param(cycles, int, 0444);
+MODULE_PARM_DESC(cycles, "teardown cycles for the storm scenario");
+
+static int storm_delay_ms = 300;
+module_param(storm_delay_ms, int, 0444);
+MODULE_PARM_DESC(storm_delay_ms, "in-flight command delay per storm cycle");
+
+struct race_ppm {
+	struct device *dev;
+	struct ucsi *ucsi;
+
+	/* Mirrors ucsi_acpi: serialises notify against start of teardown. */
+	struct mutex notify_lock;
+	bool quiescing;
+	bool unlinked;
+
+	/*
+	 * Scenario 9 only: hold notify_lock for this long inside the handler,
+	 * so that the start of teardown provably has to wait for a notify that
+	 * entered the barrier first.
+	 */
+	unsigned int block_ms;
+	bool in_notify;
+
+	/* Protects cci, last_cmd and oneshot_delay_ms. */
+	spinlock_t cci_lock;
+	u32 cci;
+	u64 last_cmd;
+
+	struct ucsi_capability cap;
+	atomic_t cmds;
+
+	struct workqueue_struct *notify_wq;
+	struct delayed_work notify_work;
+	/*
+	 * Delay applied to the next command only, so that exactly one command
+	 * is left in flight when teardown starts. Every other command is
+	 * answered synchronously, which keeps the measured baseline low and
+	 * makes a missing completion show up as the full UCSI_TIMEOUT_MS.
+	 */
+	unsigned int oneshot_delay_ms;
+};
+
+/*
+ * CCI is a single register in which the connector-change field and the
+ * command status bits coexist. Raising a connector change must therefore
+ * not clobber a command completion that the OS has not acknowledged yet,
+ * otherwise the emulator invents timeouts that no real PPM would produce.
+ */
+static void race_raise_connector_change(struct race_ppm *rp, u8 num)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&rp->cci_lock, flags);
+	rp->cci = (rp->cci & ~UCSI_RACE_CONNECTOR_FIELD) |
+		  (((u32)num << 1) & UCSI_RACE_CONNECTOR_FIELD);
+	spin_unlock_irqrestore(&rp->cci_lock, flags);
+}
+
+static u32 race_get_cci(struct race_ppm *rp)
+{
+	unsigned long flags;
+	u32 cci;
+
+	spin_lock_irqsave(&rp->cci_lock, flags);
+	cci = rp->cci;
+	spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+	return cci;
+}
+
+/*
+ * The backend notify path. This is the code under test: it is the exact
+ * shape of ucsi_acpi_notify() after the fix.
+ */
+static void race_deliver_notify(struct race_ppm *rp)
+{
+	u32 cci;
+
+	mutex_lock(&rp->notify_lock);
+
+	/* Emulates the ACPI handler having been unlinked already. */
+	if (rp->unlinked)
+		goto out_unlock;
+
+	if (rp->block_ms) {
+		unsigned int block = rp->block_ms;
+
+		rp->block_ms = 0;
+		WRITE_ONCE(rp->in_notify, true);
+		msleep(block);
+	}
+
+	cci = race_get_cci(rp);
+
+	if (rp->quiescing)
+		cci &= UCSI_CCI_BUSY | UCSI_CCI_ACK_COMPLETE |
+		       UCSI_CCI_COMMAND_COMPLETE;
+
+	ucsi_notify_common(rp->ucsi, cci);
+
+out_unlock:
+	mutex_unlock(&rp->notify_lock);
+}
+
+static void race_notify_work(struct work_struct *work)
+{
+	struct race_ppm *rp = container_of(to_delayed_work(work),
+					   struct race_ppm, notify_work);
+
+	race_deliver_notify(rp);
+}
+
+/* ------------------------- emulated PPM operations ----------------------- */
+
+static int race_read_version(struct ucsi *ucsi, u16 *version)
+{
+	*version = UCSI_VERSION_1_2;
+	return 0;
+}
+
+static int race_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+	*cci = race_get_cci(ucsi_get_drvdata(ucsi));
+	return 0;
+}
+
+static int race_poll_cci(struct ucsi *ucsi, u32 *cci)
+{
+	return race_read_cci(ucsi, cci);
+}
+
+static int race_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
+{
+	struct race_ppm *rp = ucsi_get_drvdata(ucsi);
+	unsigned long flags;
+	u64 cmd;
+
+	spin_lock_irqsave(&rp->cci_lock, flags);
+	cmd = rp->last_cmd;
+	spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+	memset(val, 0, val_len);
+
+	if (UCSI_COMMAND(cmd) == UCSI_GET_CAPABILITY)
+		memcpy(val, &rp->cap, min(val_len, sizeof(rp->cap)));
+
+	return 0;
+}
+
+static int race_write_message_out(struct ucsi *ucsi, void *data, size_t len)
+{
+	return 0;
+}
+
+/*
+ * Accept any command and answer it with the matching completion. The
+ * completion is delivered from notify_wq, which models the ACPI notify
+ * workqueue: it is a context the teardown path has to cooperate with.
+ */
+static int race_async_control(struct ucsi *ucsi, u64 command)
+{
+	struct race_ppm *rp = ucsi_get_drvdata(ucsi);
+	unsigned long flags;
+	unsigned int delay;
+	u32 cci;
+
+	atomic_inc(&rp->cmds);
+
+	spin_lock_irqsave(&rp->cci_lock, flags);
+
+	/* A pending connector change survives until the OS acknowledges it. */
+	cci = rp->cci & UCSI_RACE_CONNECTOR_FIELD;
+
+	switch (UCSI_COMMAND(command)) {
+	case UCSI_PPM_RESET:
+		cci = UCSI_CCI_RESET_COMPLETE;
+		break;
+	case UCSI_ACK_CC_CI:
+		if (command & UCSI_ACK_CONNECTOR_CHANGE)
+			cci &= ~UCSI_RACE_CONNECTOR_FIELD;
+		cci |= UCSI_CCI_ACK_COMPLETE;
+		break;
+	default:
+		cci |= UCSI_CCI_COMMAND_COMPLETE | UCSI_SET_CCI_LENGTH(0x10);
+		break;
+	}
+
+	rp->last_cmd = command;
+	rp->cci = cci;
+
+	/* PPM_RESET is polled, not notified, so it must not eat the delay. */
+	if (UCSI_COMMAND(command) == UCSI_PPM_RESET) {
+		delay = 0;
+	} else {
+		delay = rp->oneshot_delay_ms;
+		rp->oneshot_delay_ms = 0;
+	}
+
+	spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+	/* PPM_RESET is observed by polling, no notify needed. */
+	if (UCSI_COMMAND(command) == UCSI_PPM_RESET)
+		return 0;
+
+	if (delay)
+		queue_delayed_work(rp->notify_wq, &rp->notify_work,
+				   msecs_to_jiffies(delay));
+	else
+		race_deliver_notify(rp);
+
+	return 0;
+}
+
+static const struct ucsi_operations race_ops = {
+	.read_version = race_read_version,
+	.read_cci = race_read_cci,
+	.poll_cci = race_poll_cci,
+	.read_message_in = race_read_message_in,
+	.write_message_out = race_write_message_out,
+	.sync_control = ucsi_sync_control_common,
+	.async_control = race_async_control,
+};
+
+/* ------------------------------ the scenarios ---------------------------- */
+
+/* Fire a connector-change notification for connector @num. */
+static void race_fire_connector_change(struct race_ppm *rp, u8 num)
+{
+	race_raise_connector_change(rp, num);
+	race_deliver_notify(rp);
+}
+
+static void race_unlink_notify(struct race_ppm *rp)
+{
+	/*
+	 * Models acpi_remove_notify_handler(): stop dispatching and then wait
+	 * for anything already dispatched. The flag must not be set while
+	 * holding nothing else, and the flush must happen outside the lock,
+	 * exactly like the real path.
+	 */
+	mutex_lock(&rp->notify_lock);
+	rp->unlinked = true;
+	mutex_unlock(&rp->notify_lock);
+
+	cancel_delayed_work_sync(&rp->notify_work);
+	flush_workqueue(rp->notify_wq);
+}
+
+static void race_teardown(struct race_ppm *rp)
+{
+	unsigned long flags;
+	ktime_t t0;
+	s64 ms;
+
+	/*
+	 * Put a connector work in flight: it issues GET_CONNECTOR_STATUS and
+	 * blocks in wait_for_completion_timeout() until notify_wq answers.
+	 * Scenario 3 skips this to measure the teardown floor.
+	 */
+	if (scenario != 3) {
+		spin_lock_irqsave(&rp->cci_lock, flags);
+		rp->oneshot_delay_ms = 1500;
+		spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+		race_fire_connector_change(rp, 1);
+
+		/* Let the work reach the wait before teardown starts. */
+		msleep(80);
+
+		/*
+		 * The whole test is meaningless unless a connector work is
+		 * really parked in wait_for_completion_timeout() right now.
+		 */
+		pr_info(RACE_TAG ": ntfy=0x%llx flags=0x%lx conn_change_en=%d\n",
+			rp->ucsi->ntfy, rp->ucsi->flags,
+			!!(rp->ucsi->ntfy & UCSI_ENABLE_NTFY_CONNECTOR_CHANGE));
+		pr_info(RACE_TAG ": cmds=%d notify_pending=%d cmd_pending=%d event_pending=%d\n",
+			atomic_read(&rp->cmds),
+			!!delayed_work_pending(&rp->notify_work),
+			test_bit(COMMAND_PENDING, &rp->ucsi->flags),
+			test_bit(EVENT_PENDING, &rp->ucsi->flags));
+
+		if (delayed_work_pending(&rp->notify_work) &&
+		    test_bit(COMMAND_PENDING, &rp->ucsi->flags))
+			pr_info(RACE_TAG ": PRECOND=OK work parked on completion\n");
+		else
+			pr_err(RACE_TAG ": PRECOND=FAIL no work parked on completion\n");
+	}
+
+	pr_info(RACE_TAG ": scenario %d teardown start\n", scenario);
+	t0 = ktime_get();
+
+	switch (scenario) {
+	case 3:
+		/* Baseline: nothing in flight, correct order. */
+		mutex_lock(&rp->notify_lock);
+		rp->quiescing = true;
+		mutex_unlock(&rp->notify_lock);
+
+		ucsi_unregister(rp->ucsi);
+		race_unlink_notify(rp);
+		break;
+	case 1:
+		/*
+		 * The v1 ordering: kill the notify source first. The connector
+		 * work is now waiting for a completion that can never arrive,
+		 * so ucsi_unregister() must sit through UCSI_TIMEOUT_MS.
+		 */
+		race_unlink_notify(rp);
+		ucsi_unregister(rp->ucsi);
+		break;
+	case 2:
+		/*
+		 * Handler kept alive across unregister but connector changes
+		 * are never suppressed. The late notify below reaches
+		 * ucsi_connector_change() after the connector array was freed.
+		 */
+		ucsi_unregister(rp->ucsi);
+		pr_info(RACE_TAG ": firing late connector notify after unregister\n");
+		race_fire_connector_change(rp, 1);
+		race_unlink_notify(rp);
+		break;
+	default:
+		/*
+		 * The fix: suppress connector changes, keep completions, drain,
+		 * then unlink and flush before the object is freed.
+		 */
+		mutex_lock(&rp->notify_lock);
+		rp->quiescing = true;
+		mutex_unlock(&rp->notify_lock);
+
+		ucsi_unregister(rp->ucsi);
+
+		/* A late notify must now be harmless even with connector bits. */
+		pr_info(RACE_TAG ": firing late connector notify after unregister\n");
+		race_fire_connector_change(rp, 1);
+
+		race_unlink_notify(rp);
+		break;
+	}
+
+	ms = ktime_ms_delta(ktime_get(), t0);
+	pr_info(RACE_TAG ": teardown_ms=%lld\n", ms);
+
+	if (ms >= 5000)
+		pr_err(RACE_TAG ": RESULT=STALL teardown blocked %lld ms\n", ms);
+	else
+		pr_info(RACE_TAG ": RESULT=NOSTALL teardown %lld ms\n", ms);
+
+	ucsi_destroy(rp->ucsi);
+	rp->ucsi = NULL;
+
+	pr_info(RACE_TAG ": destroy done\n");
+}
+
+/*
+ * Scenario 4: hammer the notify path from several CPUs while the fixed
+ * teardown runs, repeatedly. Any missing barrier in the quiesce protocol
+ * shows up as a KASAN report on the connector array or on the ucsi object,
+ * or as a lockdep splat on notify_lock.
+ */
+#define RACE_STORM_THREADS	4
+
+static int race_storm_thread(void *data)
+{
+	struct race_ppm *rp = data;
+
+	while (!kthread_should_stop()) {
+		race_raise_connector_change(rp, 1);
+		race_deliver_notify(rp);
+		usleep_range(20, 120);
+	}
+
+	return 0;
+}
+
+static void race_storm_cycle(struct race_ppm *rp, int cycle)
+{
+	struct task_struct *th[RACE_STORM_THREADS];
+	unsigned long flags;
+	int i, n = 0;
+
+	rp->quiescing = false;
+	rp->unlinked = false;
+
+	rp->ucsi = ucsi_create(rp->dev, &race_ops);
+	if (IS_ERR(rp->ucsi)) {
+		pr_err(RACE_TAG ": cycle %d ucsi_create failed\n", cycle);
+		rp->ucsi = NULL;
+		return;
+	}
+	ucsi_set_drvdata(rp->ucsi, rp);
+
+	if (ucsi_register(rp->ucsi)) {
+		pr_err(RACE_TAG ": cycle %d ucsi_register failed\n", cycle);
+		goto out_destroy;
+	}
+
+	for (i = 0; i < 500; i++) {
+		if (rp->ucsi->connector && rp->ucsi->ntfy)
+			break;
+		msleep(20);
+	}
+
+	if (!rp->ucsi->connector || !rp->ucsi->ntfy) {
+		pr_err(RACE_TAG ": cycle %d init did not land\n", cycle);
+		ucsi_unregister(rp->ucsi);
+		race_unlink_notify(rp);
+		goto out_destroy;
+	}
+
+	for (i = 0; i < RACE_STORM_THREADS; i++) {
+		th[n] = kthread_run(race_storm_thread, rp, "ucsi_storm%d", i);
+		if (!IS_ERR(th[n]))
+			n++;
+	}
+
+	/* Park one command mid-flight, as in the single-shot scenarios. */
+	spin_lock_irqsave(&rp->cci_lock, flags);
+	rp->oneshot_delay_ms = storm_delay_ms;
+	spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+	msleep(min(storm_delay_ms / 4 + 1, 60));
+
+	/* The fix, under concurrent notifies. */
+	mutex_lock(&rp->notify_lock);
+	rp->quiescing = true;
+	mutex_unlock(&rp->notify_lock);
+
+	ucsi_unregister(rp->ucsi);
+
+	race_unlink_notify(rp);
+
+	for (i = 0; i < n; i++)
+		kthread_stop(th[i]);
+
+out_destroy:
+	ucsi_destroy(rp->ucsi);
+	rp->ucsi = NULL;
+	if (cycles <= 20 || !((cycle + 1) % 100))
+		pr_info(RACE_TAG ": cycle %d done\n", cycle);
+}
+
+/*
+ * Scenario 8: deterministic proof of what the CCI mask does and does not do.
+ *
+ * A single CCI carrying both a connector change and a command completion is
+ * delivered while quiescing. The completion must still be signalled and the
+ * connector path must not be entered. The same CCI is then delivered with
+ * quiescing off as a positive control, so that a test that silently stopped
+ * exercising ucsi_connector_change() cannot pass.
+ */
+static void race_prove_mask(struct race_ppm *rp)
+{
+	struct ucsi *ucsi = rp->ucsi;
+	bool ev_quiesced, cmd_cleared, ev_control;
+	/* UCSI_CCI_CONNECTOR() extracts the field, so build it by hand. */
+	u32 cci = ((u32)1 << 1) | UCSI_CCI_COMMAND_COMPLETE;
+	unsigned long flags;
+
+	/* Deliver the crafted CCI rather than whatever init left behind. */
+	spin_lock_irqsave(&rp->cci_lock, flags);
+	rp->cci = cci;
+	spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+	pr_info(RACE_TAG ": mask test cci=0x%08x connector=%u\n",
+		cci, (unsigned int)UCSI_CCI_CONNECTOR(cci));
+
+	/* Quiescing: completion expected, connector dispatch not. */
+	clear_bit(EVENT_PENDING, &ucsi->flags);
+	set_bit(COMMAND_PENDING, &ucsi->flags);
+	reinit_completion(&ucsi->complete);
+
+	mutex_lock(&rp->notify_lock);
+	rp->quiescing = true;
+	mutex_unlock(&rp->notify_lock);
+
+	race_deliver_notify(rp);
+
+	ev_quiesced = test_bit(EVENT_PENDING, &ucsi->flags);
+	cmd_cleared = !test_bit(COMMAND_PENDING, &ucsi->flags);
+
+	pr_info(RACE_TAG ": quiesced: event_pending=%d command_completed=%d\n",
+		ev_quiesced, cmd_cleared);
+
+	/* Positive control: same CCI, quiescing off, connector must dispatch. */
+	clear_bit(EVENT_PENDING, &ucsi->flags);
+	set_bit(COMMAND_PENDING, &ucsi->flags);
+	reinit_completion(&ucsi->complete);
+
+	mutex_lock(&rp->notify_lock);
+	rp->quiescing = false;
+	mutex_unlock(&rp->notify_lock);
+
+	race_deliver_notify(rp);
+
+	ev_control = test_bit(EVENT_PENDING, &ucsi->flags);
+
+	pr_info(RACE_TAG ": control: event_pending=%d\n", ev_control);
+
+	if (!ev_quiesced && cmd_cleared && ev_control)
+		pr_info(RACE_TAG ": RESULT=MASKOK completion kept, connector dropped\n");
+	else
+		pr_err(RACE_TAG ": RESULT=MASKFAIL quiesced_ev=%d cmd=%d control_ev=%d\n",
+		       ev_quiesced, cmd_cleared, ev_control);
+
+	/* Let the control's connector work settle before tearing down. */
+	msleep(200);
+
+	mutex_lock(&rp->notify_lock);
+	rp->quiescing = true;
+	mutex_unlock(&rp->notify_lock);
+
+	ucsi_unregister(rp->ucsi);
+	race_unlink_notify(rp);
+	ucsi_destroy(rp->ucsi);
+	rp->ucsi = NULL;
+}
+
+static int race_block_thread(void *data)
+{
+	race_deliver_notify(data);
+	return 0;
+}
+
+/*
+ * Scenario 9: prove that notify_lock is a real barrier and not just a flag.
+ *
+ * A notify enters the handler before teardown and stays inside it. Setting
+ * quiescing must not be able to complete until that notify has returned,
+ * which is what makes "its schedule_work() happens before ucsi_unregister()
+ * starts cancelling" true rather than merely likely.
+ */
+#define RACE_BLOCK_MS	1200
+
+static void race_prove_barrier(struct race_ppm *rp)
+{
+	struct task_struct *th;
+	ktime_t t0;
+	s64 waited;
+	int i;
+
+	race_raise_connector_change(rp, 1);
+
+	mutex_lock(&rp->notify_lock);
+	rp->block_ms = RACE_BLOCK_MS;
+	rp->in_notify = false;
+	mutex_unlock(&rp->notify_lock);
+
+	th = kthread_run(race_block_thread, rp, "ucsi_block");
+	if (IS_ERR(th)) {
+		pr_err(RACE_TAG ": RESULT=BARRIERFAIL cannot start notify\n");
+		return;
+	}
+
+	/* Wait until the notify is provably inside the critical section. */
+	for (i = 0; i < 500; i++) {
+		if (READ_ONCE(rp->in_notify))
+			break;
+		usleep_range(1000, 2000);
+	}
+
+	if (!READ_ONCE(rp->in_notify)) {
+		pr_err(RACE_TAG ": RESULT=BARRIERFAIL notify never entered\n");
+		return;
+	}
+
+	pr_info(RACE_TAG ": notify inside barrier, starting teardown\n");
+
+	t0 = ktime_get();
+	mutex_lock(&rp->notify_lock);
+	rp->quiescing = true;
+	mutex_unlock(&rp->notify_lock);
+	waited = ktime_ms_delta(ktime_get(), t0);
+
+	pr_info(RACE_TAG ": quiesce waited %lld ms for the in-flight notify\n",
+		waited);
+
+	if (waited >= RACE_BLOCK_MS / 2)
+		pr_info(RACE_TAG ": RESULT=BARRIEROK teardown blocked %lld ms\n",
+			waited);
+	else
+		pr_err(RACE_TAG ": RESULT=BARRIERFAIL teardown raced past, %lld ms\n",
+		       waited);
+
+	ucsi_unregister(rp->ucsi);
+	race_unlink_notify(rp);
+	ucsi_destroy(rp->ucsi);
+	rp->ucsi = NULL;
+}
+
+static struct platform_device *race_pdev;
+
+static int __init race_init(void)
+{
+	struct race_ppm *rp;
+	int ret;
+
+	race_pdev = platform_device_register_simple("ucsi_race", -1, NULL, 0);
+	if (IS_ERR(race_pdev))
+		return PTR_ERR(race_pdev);
+
+	rp = kzalloc_obj(*rp);
+	if (!rp) {
+		ret = -ENOMEM;
+		goto err_pdev;
+	}
+
+	rp->dev = &race_pdev->dev;
+	mutex_init(&rp->notify_lock);
+	spin_lock_init(&rp->cci_lock);
+	INIT_DELAYED_WORK(&rp->notify_work, race_notify_work);
+	rp->cap.num_connectors = RACE_NUM_CONNECTORS;
+
+	/* kacpi_notify_wq is WQ_PERCPU with default concurrency. */
+	rp->notify_wq = alloc_workqueue("ucsi_race_notify", WQ_PERCPU, 0);
+	if (!rp->notify_wq) {
+		ret = -ENOMEM;
+		goto err_free;
+	}
+
+	if (scenario == 4) {
+		int c;
+
+		for (c = 0; c < cycles; c++)
+			race_storm_cycle(rp, c);
+
+		pr_info(RACE_TAG ": RESULT=STORMDONE %d cycles %d threads\n",
+			cycles, RACE_STORM_THREADS);
+		goto out_clean;
+	}
+
+	rp->ucsi = ucsi_create(rp->dev, &race_ops);
+	if (IS_ERR(rp->ucsi)) {
+		ret = PTR_ERR(rp->ucsi);
+		goto err_wq;
+	}
+	ucsi_set_drvdata(rp->ucsi, rp);
+
+	ret = ucsi_register(rp->ucsi);
+	if (ret) {
+		pr_err(RACE_TAG ": RESULT=REGFAIL ucsi_register returned %d\n", ret);
+		ucsi_destroy(rp->ucsi);
+		goto err_wq;
+	}
+
+	/*
+	 * Scenario 5: tear down while ucsi_init_work() is still running its
+	 * command sequence. ucsi_unregister() starts with
+	 * cancel_delayed_work_sync(&ucsi->work), so the init work must be able
+	 * to finish its in-flight command, which again needs the notify path
+	 * to stay alive across ucsi_unregister().
+	 */
+	if (scenario == 5 || scenario == 6) {
+		unsigned long flags;
+		ktime_t t0;
+		s64 ms;
+
+		/* Park one of the init commands in wait_for_completion(). */
+		spin_lock_irqsave(&rp->cci_lock, flags);
+		rp->oneshot_delay_ms = 1500;
+		spin_unlock_irqrestore(&rp->cci_lock, flags);
+
+		msleep(40);
+
+		pr_info(RACE_TAG ": mid-init connector=%p ntfy=0x%llx cmds=%d notify_pending=%d cmd_pending=%d\n",
+			rp->ucsi->connector, rp->ucsi->ntfy,
+			atomic_read(&rp->cmds),
+			!!delayed_work_pending(&rp->notify_work),
+			test_bit(COMMAND_PENDING, &rp->ucsi->flags));
+
+		if (delayed_work_pending(&rp->notify_work) &&
+		    test_bit(COMMAND_PENDING, &rp->ucsi->flags))
+			pr_info(RACE_TAG ": PRECOND=OK init command parked\n");
+		else
+			pr_err(RACE_TAG ": PRECOND=FAIL no init command parked\n");
+
+		t0 = ktime_get();
+
+		if (scenario == 6) {
+			/* v1 ordering: the parked init command loses its notify. */
+			race_unlink_notify(rp);
+			ucsi_unregister(rp->ucsi);
+		} else {
+			mutex_lock(&rp->notify_lock);
+			rp->quiescing = true;
+			mutex_unlock(&rp->notify_lock);
+
+			ucsi_unregister(rp->ucsi);
+			race_unlink_notify(rp);
+		}
+
+		ms = ktime_ms_delta(ktime_get(), t0);
+		pr_info(RACE_TAG ": teardown_ms=%lld\n", ms);
+		if (ms >= 5000)
+			pr_err(RACE_TAG ": RESULT=STALL mid-init %lld ms\n", ms);
+		else
+			pr_info(RACE_TAG ": RESULT=NOSTALL mid-init %lld ms\n", ms);
+
+		ucsi_destroy(rp->ucsi);
+		rp->ucsi = NULL;
+		goto out_clean;
+	}
+
+	/*
+	 * ucsi_register() only queues ucsi_init_work(); the connectors and the
+	 * notification mask appear later. Wait for init to land, otherwise
+	 * ucsi_connector_change() would bail out on the ntfy gate and no work
+	 * would ever be queued.
+	 */
+	for (ret = 0; ret < 500; ret++) {
+		if (rp->ucsi->connector && rp->ucsi->ntfy)
+			break;
+		msleep(20);
+	}
+
+	if (!rp->ucsi->connector || !rp->ucsi->ntfy) {
+		pr_err(RACE_TAG ": RESULT=INITFAIL ntfy=0x%llx connector=%p\n",
+		       rp->ucsi->ntfy, rp->ucsi->connector);
+		ucsi_unregister(rp->ucsi);
+		race_unlink_notify(rp);
+		ucsi_destroy(rp->ucsi);
+		ret = -ENODEV;
+		goto err_wq;
+	}
+
+	pr_info(RACE_TAG ": init complete, %u connectors, ntfy=0x%llx\n",
+		rp->ucsi->cap.num_connectors, rp->ucsi->ntfy);
+
+	if (scenario == 8)
+		race_prove_mask(rp);
+	else if (scenario == 9)
+		race_prove_barrier(rp);
+	else
+		race_teardown(rp);
+
+out_clean:
+	destroy_workqueue(rp->notify_wq);
+	kfree(rp);
+	platform_device_unregister(race_pdev);
+
+	pr_info(RACE_TAG ": DONE\n");
+	return 0;
+
+err_wq:
+	destroy_workqueue(rp->notify_wq);
+err_free:
+	kfree(rp);
+err_pdev:
+	platform_device_unregister(race_pdev);
+	return ret;
+}
+
+static void __exit race_exit(void)
+{
+}
+
+module_init(race_init);
+module_exit(race_exit);
+
+MODULE_DESCRIPTION("UCSI teardown race harness");
+MODULE_LICENSE("GPL v2");
-- 
2.43.0