iommu: PCI reset helpers' lock-free entry reads race the iommu_device_unregister() traversal -> group use-after-free or NULL deref inside the reset path

iommu: PCI reset helpers' lock-free entry reads race the iommu_device_unregister() traversal -> group use-after-free or NULL deref inside the reset path

[徐晗翔]

[-- Attachment #1: Type: text/plain, Size: 7448 bytes --]

The PCI reset IOMMU helpers read pdev->dev.iommu_group and
dev->iommu before taking any lock, and the whole PCI reset body
(FLR alone sleeps 100ms) runs between their prepare and done halves
-- meanwhile the iommu_device_unregister() traversal can tear the
device's group away entirely, so the helper's post-reset half locks
an already-freed group (use-after-free spinlock) or walks
&NULL->devices (NULL deref oops).  The kernel's own comment above
these helpers states the contract -- "callers must ensure there will
be no racy iommu_release_device() call, which otherwise would UAF the
dev->iommu_group pointer" (drivers/iommu/iommu.c:4027-4032) -- but
the unregister traversal is precisely an iommu_release_device()
source that no reset-path caller can ensure against: it takes no
device lock, so the device_lock the reset path holds (which does
serialize device_del) protects nothing here.

Concretely, on mainline 7.3: pci_dev_reset_iommu_prepare()
(iommu.c:4034) reads the group at :4036 and tests dev_has_iommu() at
:4042 before guard(mutex)(&group->mutex) at :4045;
pci_dev_reset_iommu_done() (:4146) has the same shape at :4148,
:4153, :4156 and then calls __dev_to_gdev() at :4158, which walks
group->devices.  The reset body bracketed by these helpers includes
five call-site pairs in drivers/pci/pci.c (:4426/:4446 with the FLR
msleep(100) at :4442, :4503/:4524 with another at :4520, :4561/:4578,
:5019/:5031, :5056/:5076) -- a >=100ms window during which the
traversal's millisecond-scale teardown fits with room to spare.
Unlike the removal-side race I reported earlier, triggering needs NO
concurrent device_del: a client device performing an FLR or bus
reset (administrator "echo 1 > /sys/bus/pci/devices/<BDF>/reset", or
a driver-initiated reset such as NIC reconfiguration) while the IOMMU
instance is unregistered (virtio-iommu sysfs unbind, platform IOMMU)
is enough -- the reset body's width makes this the easiest of the
unregister-race family to hit.

The detailed description:

* Test environment / affected version

  Linux version 7.3.0 (gcc 13.3.0, GNU ld 2.42) #1 SMP PREEMPT_DYNAMIC
  -- vanilla mainline 7.3 release tarball (kernel.org, unmodified).
  All functions cited are byte-identical in 7.1.5 and 7.3; line
  numbers are from 7.3.  This report is source-level: my test
  environment provides no PCI+ATS client to drive the E side, so the
  interleaving itself was not reproduced dynamically; the shared
  teardown half (the traversal freeing a group through the
  unconditional iommu_group_put() at iommu.c:775) is the same
  component my removal-side report demonstrated with KASAN.

* Mechanism

  The helper pair's shape (entry reads precede every lock):

      int pci_dev_reset_iommu_prepare(struct pci_dev *pdev)   /* :4034 */
      {
              struct iommu_group *group = pdev->dev.iommu_group;  /* :4036
                                                                  lock-free */
              ...
              if (!pci_ats_supported(pdev) || !dev_has_iommu(&pdev->dev))
                                                              /* :4042, second
                                                                  lock-free read */
                      return 0;
              guard(mutex)(&group->mutex);                    /* :4045 */
              ...

      void pci_dev_reset_iommu_done(struct pci_dev *pdev)     /* :4146 */
      {
              struct iommu_group *group = pdev->dev.iommu_group;  /* :4148 */
              ...
              if (!pci_ats_supported(pdev) || !dev_has_iommu(&pdev->dev))
                                                              /* :4153 */
                      return;
              guard(mutex)(&group->mutex);                    /* :4156 */
              gdev = __dev_to_gdev(&pdev->dev);               /* :4158: walks
                                                         group->devices     */

  A side: iommu_device_unregister() -> bus_for_each_dev ->
  remove_iommu_group() (:273) -> iommu_release_device() (:778) ->
  __iommu_group_remove_device() (:751) clears dev->iommu_group under
  the group lock (:766; also via iommu_deinit_device at :611) and,
  with the unconditional iommu_group_put() at :775, can free the
  group outright when this device held the last reference.

  Interleaving (done() half shown; prepare is symmetric):

      E:  group = pdev->dev.iommu_group;        /* :4148, reads G */
      ... reset body: msleep(100) ...           /* >=100ms        */
      A:  traversal completes: group lock teardown, dev->iommu_group
          = NULL, param freed, :775 put drops the last ref -> G freed
      E:  dev_has_iommu(&pdev->dev)             /* :4153:
          - read lands before A's param clear -> true
            -> guard(mutex)(&group->mutex)      /* :4156: mutex_lock on
                                                 FREED group -> UAF
                                                 (spinlock on freed/
                                                 reused memory)  */
          - G survived (other refs) -> __dev_to_gdev(&pdev->dev)
                                               /* :4158: re-reads
                                                  dev->iommu_group ==
                                                  NULL -> for_each_
                                                  group_device(NULL)
                                                  walks &NULL->devices
                                                  -> NULL deref oops */

      Symmetric variant: :4148 already reads NULL (after A's :766)
      while :4153 still sees the param -> guard(mutex) on a NULL
      group pointer.

  Why the existing locks do not help: the reset path holds the
  device lock, which serializes device_del (the case the :4027
  comment was designed for -- PCI sysfs is removed before the
  removal notification).  The unregister traversal is not
  device_del: it walks the bus's klist and calls
  iommu_release_device() directly, taking no client device lock and
  none of the locks the helpers take before their entry reads.

* Trigger model

  Any runtime-unregistrable IOMMU instance (virtio-iommu sysfs
  unbind; platform IOMMUs without suppress_bind_attrs; PCI
  hot-unplug paths) times a client device undergoing FLR/bus reset
  during unregistration:

      # terminal A                                   (root)
      echo <virtio-bus-id> > /sys/bus/virtio/drivers/virtio_iommu/unbind
      # terminal B                                   (root)
      echo 1 > /sys/bus/pci/devices/<BDF>/reset      # loop, or let the
                                                     # driver reconfig
                                                     # trigger FLR

  The >=100ms reset body makes the hit probability far higher than
  the removal-side race's millisecond overlap.

* Evidence

  Source-level on 7.3, function by function (every anchor is quoted
  in the mechanism section above; a consolidated annotated list is
  available on request).  The A-side teardown that the interleaving
  consumes -- the traversal entering iommu_release_device() without
  any of the probe-side locks and freeing a group via the
  unconditional :775 put -- is the identical component demonstrated
  with KASAN in my removal-side report (reproducer attached again
  here for convenience; its "ab" action drives exactly that teardown
  half).


Thanks for taking a look.

Regards,
Hanxiang Xu
xuhx@hust.edu.cn



[-- Attachment #2: iommurace.c --]
[-- Type: text/plain, Size: 12723 bytes --]

// SPDX-License-Identifier: GPL-2.0
/*
 * iommurace.c - dynamic verification driver for the IOMMU lifecycle race
 * audit (HV-3 / HV-4 in docs/manual-review-drivers-base-2026-10-20.md).
 *
 * Registers a FAKE IOMMU instance (struct iommu_device + struct iommu_ops)
 * through the real EXPORT_SYMBOL_GPL core APIs, and drives the REAL core
 * code paths of drivers/iommu/iommu.c with REAL platform-bus devices:
 *
 *   entry A (instance unregister traversal):
 *       iommu_device_unregister() -> bus_for_each_dev -> remove_iommu_group
 *   entry B (client removal):
 *       platform_device_unregister() -> device_del ->
 *       BUS_NOTIFY_REMOVED_DEVICE -> iommu_bus_notifier -> iommu_release_device
 *   entry C (client probe):
 *       platform_device_add() -> device_add -> BUS_NOTIFY_ADD_DEVICE ->
 *       iommu_bus_notifier -> iommu_probe_device -> iommu_init_device
 *
 * Clients are bound to the fake instance by calling iommu_fwspec_init()
 * (EXPORT_SYMBOL_GPL) between platform_device_alloc() and
 * platform_device_add(), so the ADD notifier's probe matches the fake ops
 * via iommu_fwspec_ops() on the shared software fwnode.
 *
 * actions (echo <cmd> > /sys/module/iommurace/parameters/action):
 *   setup           - carrier device + software fwnode + iommu_device_sysfs_add
 *                     + iommu_device_register (real bus_iommu_probe sweep)
 *   unreg           - one iommu_device_unregister (real traversal)
 *   reg             - one iommu_device_register
 *   regloop N       - N sequential register+unregister rounds (baseline)
 *   client N        - N clients added then removed sequentially (baseline;
 *                     expect zero splats with a stable registered instance)
 *   ab N            - HV-3 A x B: kthread A loops register+unregister,
 *                     kthread B loops client add+del; real paths on both sides
 *   ac N            - HV-4 A x C: kthread A loops register+unregister,
 *                     kthread C loops client add only (W1/W2 windows,
 *                     clients kept for post-mortem inspection)
 *   ac1 N           - same as ac but device_group = generic_single_device_group
 *                     (W3: unregister tail singleton_group race surface)
 *   cleanup         - best-effort teardown (unregister, drop clients, sysfs)
 *   status          - log stats
 *
 * Kernel source is NOT modified; everything is driven through exported
 * iommu core APIs, matching the methodology of pmu_race/mcrsim/regmap_race.
 *
 * NOTE: ab/ac/ac1 may leak module references when the audit's "W2 defective
 * state" hits (probe succeeded -> try_module_get, later removal skips
 * iommu_deinit_device -> no module_put). rmmod then fails with EBUSY; the
 * plans therefore do not rmmod after race actions - the leak itself is a
 * verified prediction of the report.
 */
#define pr_fmt(fmt) "IOMMURACE: " fmt

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/kthread.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/string.h>
#include <linux/slab.h>
#include <linux/completion.h>
#include <linux/atomic.h>
#include <linux/platform_device.h>
#include <linux/iommu.h>
#include <linux/property.h>

/* ------------------------------------------------------------------ */
/* fake IOMMU instance                                                 */

static struct iommu_device fom_iommu;
static struct iommu_ops fom_ops;
static struct platform_device *carrier;
static struct fwnode_handle *sw_node;
static atomic_t reg_state = ATOMIC_INIT(0); /* 1 = registered */
static int use_singleton;

/* stats */
static unsigned long stat_reg, stat_unreg, stat_reg_fail;
static unsigned long stat_add_ok, stat_add_fail, stat_fw_fail, stat_del;
static unsigned long stat_b_done, stat_c_done, stat_a_done;

/* fake static identity domain: attach is a no-op, free must not kfree */
static int fom_attach_dev(struct iommu_domain *dom, struct device *dev,
			  struct iommu_domain *old)
{
	return 0;
}

static void fom_dom_free(struct iommu_domain *dom)
{
	pr_info("fom_dom_free on static identity domain (ignored)\n");
}

static const struct iommu_domain_ops fom_dom_ops = {
	.attach_dev = fom_attach_dev,
	.free = fom_dom_free,
};

static struct iommu_domain fom_identity_dom = {
	.type = IOMMU_DOMAIN_IDENTITY,
	.ops = &fom_dom_ops,
	.owner = &fom_ops,
	.pgsize_bitmap = 0,
	.geometry = {
		.aperture_start = 0,
		.aperture_end = ~0UL,
		.force_aperture = false,
	},
};

static struct iommu_device *fom_probe_device(struct device *dev)
{
	return &fom_iommu;
}

static void fom_release_device(struct device *dev)
{
}

static int fom_def_domain_type(struct device *dev)
{
	return IOMMU_DOMAIN_IDENTITY;
}

static int fom_register(void)
{
	int ret;

	if (atomic_xchg(&reg_state, 1) != 0)
		return 0; /* already registered */

	fom_ops = (struct iommu_ops){
		.owner			= THIS_MODULE,
		.probe_device		= fom_probe_device,
		.release_device		= fom_release_device,
		.device_group		= use_singleton ?
					  generic_single_device_group :
					  generic_device_group,
		.def_domain_type	= fom_def_domain_type,
		.identity_domain	= &fom_identity_dom,
	};

	ret = iommu_device_register(&fom_iommu, &fom_ops, &carrier->dev);
	if (ret) {
		atomic_set(&reg_state, 0);
		stat_reg_fail++;
		return ret;
	}
	stat_reg++;
	return 0;
}

static void fom_unregister(void)
{
	if (atomic_xchg(&reg_state, 0) != 1)
		return; /* not registered */
	iommu_device_unregister(&fom_iommu);
	stat_unreg++;
}

/* ------------------------------------------------------------------ */
/* fake client devices (real platform bus devices)                     */

#define MAX_CLI 512
static struct platform_device *cli[MAX_CLI];
static int nr_cli;

static int fom_new_client(struct platform_device **out)
{
	struct platform_device *pdev;
	int ret;

	pdev = platform_device_alloc("fomcli", PLATFORM_DEVID_AUTO);
	if (!pdev)
		return -ENOMEM;

	/*
	 * Bind the client's fwspec to the fake instance BEFORE device_add so
	 * the ADD notifier's iommu_probe_device() matches our fake ops.
	 * Fails with -ENODEV/-EPROBE_DEFER when the instance is currently
	 * unregistered / mid-registration: the natural non-racy branch.
	 */
	ret = iommu_fwspec_init(&pdev->dev, sw_node);
	if (ret) {
		stat_fw_fail++;
		platform_device_put(pdev);
		return ret;
	}

	ret = platform_device_add(pdev);
	if (ret) {
		stat_add_fail++;
		platform_device_put(pdev);
		return ret;
	}
	stat_add_ok++;
	*out = pdev;
	return 0;
}

static void fom_del_client(struct platform_device *pdev)
{
	platform_device_unregister(pdev);
	stat_del++;
}

/* ------------------------------------------------------------------ */
/* race threads                                                        */

struct rargs {
	int rounds;
};

static struct rargs ra, rb;

static int thread_a(void *data) /* entry A: register/unregister rounds */
{
	struct rargs *a = data;
	int i;

	for (i = 0; i < a->rounds; i++) {
		fom_register();
		fom_unregister();
		if ((i % 500) == 499)
			pr_info("thread_a round %d\n", i + 1);
		/*
		 * No sleep: maximize overlap of the unregister traversal
		 * with concurrent client add/del/probe on other CPUs.
		 */
	}
	stat_a_done++;
	pr_info("thread_a finished %d rounds\n", a->rounds);
	return 0;
}

static int thread_b(void *data) /* entry B: client add+del churn */
{
	struct rargs *a = data;
	int i;
	struct platform_device *pdev;

	for (i = 0; i < a->rounds; i++) {
		if (fom_new_client(&pdev))
			continue;
		fom_del_client(pdev);
		if ((i % 500) == 499)
			pr_info("thread_b round %d\n", i + 1);
	}
	stat_b_done++;
	pr_info("thread_b finished %d rounds (add_ok=%lu fw_fail=%lu)\n",
		a->rounds, stat_add_ok, stat_fw_fail);
	return 0;
}

static int thread_c(void *data) /* entry C: client add only (HV-4) */
{
	struct rargs *a = data;
	int i;

	for (i = 0; i < a->rounds; i++) {
		struct platform_device *pdev;

		if (nr_cli < MAX_CLI) {
			if (fom_new_client(&pdev) == 0)
				cli[nr_cli++] = pdev;
		}
		if ((i % 500) == 499)
			pr_info("thread_c round %d\n", i + 1);
	}
	stat_c_done++;
	pr_info("thread_c finished %d rounds (added=%d)\n", a->rounds, nr_cli);
	return 0;
}

static void run_race(const char *kind, int rounds)
{
	struct task_struct *ta, *tb;

	ra.rounds = rounds;
	rb.rounds = rounds;

	if (strcmp(kind, "ab") == 0)
		tb = kthread_run(thread_b, &rb, "iommurace_b");
	else
		tb = kthread_run(thread_c, &rb, "iommurace_c");
	ta = kthread_run(thread_a, &ra, "iommurace_a");
	if (IS_ERR(ta) || IS_ERR(tb)) {
		pr_err("kthread creation failed\n");
		return;
	}
	/* block until both sides complete (bounded: a crashed thread never
	 * increments its done counter - bail out after 150s and let the
	 * plan continue; the KASAN report in dmesg is the evidence) */
	{
		int waited = 0;

		while (stat_a_done + stat_b_done + stat_c_done < 2 &&
		       waited < 150000) {
			msleep(200);
			waited += 200;
		}
	}
	pr_info("race %s rounds=%d complete\n", kind, rounds);
}

/* ------------------------------------------------------------------ */
/* action parameter                                                    */

static char action_buf[64];

static int action_set(const char *val, const struct kernel_param *kp)
{
	char cmd[16];
	long n = 0;
	int got;

	strscpy(action_buf, val, sizeof(action_buf));
	got = sscanf(val, "%15s %ld", cmd, &n);
	if (got < 1)
		return -EINVAL;

	if (strcmp(cmd, "setup") == 0) {
		int ret;

		if (carrier) {
			pr_info("setup: already done\n");
			return 0;
		}
		sw_node = fwnode_create_software_node(NULL, NULL);
		if (IS_ERR(sw_node)) {
			sw_node = NULL;
			return PTR_ERR(sw_node);
		}
		carrier = platform_device_alloc("fomhw", PLATFORM_DEVID_AUTO);
		if (!carrier) {
			fwnode_remove_software_node(sw_node);
			sw_node = NULL;
			return -ENOMEM;
		}
		device_set_node(&carrier->dev, sw_node);
		ret = platform_device_add(carrier);
		if (ret) {
			platform_device_put(carrier);
			carrier = NULL;
			fwnode_remove_software_node(sw_node);
			sw_node = NULL;
			return ret;
		}
		ret = iommu_device_sysfs_add(&fom_iommu, &carrier->dev, NULL,
					     "fom_iommu0");
		if (ret) {
			platform_device_unregister(carrier);
			carrier = NULL;
			fwnode_remove_software_node(sw_node);
			sw_node = NULL;
			return ret;
		}
		ret = fom_register();
		pr_info("setup done (register ret=%d)\n", ret);
		return ret;
	}

	if (strcmp(cmd, "unreg") == 0) {
		fom_unregister();
		pr_info("unreg done\n");
		return 0;
	}
	if (strcmp(cmd, "reg") == 0) {
		int ret = fom_register();

		pr_info("reg ret=%d\n", ret);
		return ret;
	}
	if (strcmp(cmd, "regloop") == 0 && got == 2) {
		long i;

		for (i = 0; i < n; i++) {
			fom_register();
			fom_unregister();
		}
		pr_info("regloop %ld done\n", n);
		return 0;
	}
	if (strcmp(cmd, "client") == 0 && got == 2) {
		long i;
		int err = 0;

		if (n > MAX_CLI)
			n = MAX_CLI;
		for (i = 0; i < n; i++) {
			if (fom_new_client(&cli[nr_cli])) {
				err = 1;
				break;
			}
			nr_cli++;
		}
		msleep(500); /* let probes settle */
		for (i = 0; i < nr_cli; i++)
			fom_del_client(cli[i]);
		nr_cli = 0;
		pr_info("client %ld done (err=%d)\n", n, err);
		return 0;
	}
	if ((strcmp(cmd, "ab") == 0 || strcmp(cmd, "ac") == 0 ||
	     strcmp(cmd, "ac1") == 0) && got == 2) {
		use_singleton = (cmd[2] == '1');
		stat_a_done = stat_b_done = stat_c_done = 0;
		run_race(cmd, (int)n);
		return 0;
	}
	if (strcmp(cmd, "cleanup") == 0) {
		int i;

		fom_unregister();
		for (i = 0; i < nr_cli; i++)
			if (cli[i])
				fom_del_client(cli[i]);
		nr_cli = 0;
		pr_info("cleanup done (module refcount leak => rmmod EBUSY is expected after races)\n");
		return 0;
	}
	if (strcmp(cmd, "status") == 0) {
		pr_info("stats: reg=%lu unreg=%lu reg_fail=%lu add_ok=%lu add_fail=%lu fw_fail=%lu del=%lu\n",
			stat_reg, stat_unreg, stat_reg_fail, stat_add_ok,
			stat_add_fail, stat_fw_fail, stat_del);
		pr_info("clients tracked=%d singleton=%d\n", nr_cli, use_singleton);
		return 0;
	}

	return -EINVAL;
}

static const struct kernel_param_ops action_ops = {
	.set = action_set,
	.get = param_get_string,
};
module_param_cb(action, &action_ops, &action_buf, 0600);
MODULE_PARM_DESC(action, "setup|unreg|reg|regloop N|client N|ab N|ac N|ac1 N|cleanup|status");

static int __init iommurace_init(void)
{
	pr_info("loaded (kernel has IOMMU_API)\n");
	return 0;
}

static void __exit iommurace_exit(void)
{
	int i;

	fom_unregister();
	for (i = 0; i < nr_cli; i++)
		if (cli[i])
			fom_del_client(cli[i]);
	nr_cli = 0;
	if (carrier) {
		iommu_device_sysfs_remove(&fom_iommu);
		platform_device_unregister(carrier);
		fwnode_remove_software_node(sw_node);
	}
	pr_info("unloaded\n");
}

module_init(iommurace_init);
module_exit(iommurace_exit);

MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("IOMMU lifecycle race verification driver (HV-3/HV-4)");

Re: iommu: PCI reset helpers' lock-free entry reads race the iommu_device_unregister() traversal -> group use-after-free or NULL deref inside the reset path

[Nicolin Chen]

On Thu, Oct 22, 2026 at 10:05:04PM +0800, 徐晗翔 wrote:
> The PCI reset IOMMU helpers read pdev->dev.iommu_group and
> dev->iommu before taking any lock, and the whole PCI reset body
> (FLR alone sleeps 100ms) runs between their prepare and done halves
> -- meanwhile the iommu_device_unregister() traversal can tear the
> device's group away entirely, so the helper's post-reset half locks
> an already-freed group (use-after-free spinlock) or walks
> &NULL->devices (NULL deref oops).

Unplugging an IOMMU instance while a device behind it is still
alive? That doesn't sound like a reasonable use case to me.

> The kernel's own comment above
> these helpers states the contract -- "callers must ensure there will
> be no racy iommu_release_device() call, which otherwise would UAF the
> dev->iommu_group pointer" (drivers/iommu/iommu.c:4027-4032) -- but
> the unregister traversal is precisely an iommu_release_device()
> source that no reset-path caller can ensure against: it takes no
> device lock, so the device_lock the reset path holds (which does
> serialize device_del) protects nothing here.

The contract in the kdocs literally means iommu_release_device()
must not be called during a reset. So, iommu_device_unregister()
invoking iommu_release_device() violates that contract.

Nicolin