* Re: [PATCH v2] usb: typec: ucsi: acpi: fix use-after-free on driver removal
2026-09-03 23:21 ` [PATCH v2] usb: typec: ucsi: acpi: fix use-after-free on driver removal Iván Ezequiel Rodriguez
2026-09-04 3:48 ` Huang Wei
@ 2026-09-07 11:55 ` Heikki Krogerus
2026-09-07 21:01 ` [RFC] usb: typec: ucsi: add software PPM harness for teardown races Iván Ezequiel Rodriguez
2 siblings, 0 replies; 7+ messages in thread
From: Heikki Krogerus @ 2026-09-07 11:55 UTC (permalink / raw)
To: Iván Ezequiel Rodriguez
Cc: Greg Kroah-Hartman, Fan Wu, Wei Huang, linux-usb, linux-kernel,
stable
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [RFC] usb: typec: ucsi: add software PPM harness for teardown races
2026-09-03 23:21 ` [PATCH v2] usb: typec: ucsi: acpi: fix use-after-free on driver removal Iván Ezequiel Rodriguez
2026-09-04 3:48 ` Huang Wei
2026-09-07 11:55 ` Heikki Krogerus
@ 2026-09-07 21:01 ` Iván Ezequiel Rodriguez
2 siblings, 0 replies; 7+ messages in thread
From: Iván Ezequiel Rodriguez @ 2026-09-07 21:01 UTC (permalink / raw)
To: Heikki Krogerus, Huang Wei
Cc: Greg Kroah-Hartman, Fan Wu, linux-usb, linux-kernel,
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
^ permalink raw reply related [flat|nested] 7+ messages in thread