Re: [PATCH v8 11/25] iommu/arm-smmu-v3-kvm: Add the kernel driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] iommu: pKVM DMA isolation bypass via unbound SMMUs falling back to host driver
- [High] iommu: Use-after-free of smmu during platform_device removal
--

commit a8ae8dbe858fbefdcb8b149d519f07175f91d794
Author: Mostafa Saleh <smostafa@google.com>

iommu/arm-smmu-v3-kvm: Add the kernel driver

This commit introduces the pKVM host driver for the Arm SMMUv3, allowing
the hypervisor to trap and emulate SMMU management when KVM runs in
protected mode. The driver probes these SMMUs and creates auxiliary
devices for them on the aux bus.

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c
> new file mode 100644
> index 0000000000000..9947d3a44304b
> --- /dev/null
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-kvm.c
[ ... ]
> +static int smmuv3_nesting_probe(struct platform_device *pdev)
> +{
> +	struct hyp_arm_smmu_v3_device *smmu = &kvm_arm_smmu_array[kvm_arm_smmu_cur];
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +
> +	/* Only device tree, ACPI not supported. */
> +	if (!dev->of_node)
> +		return -EINVAL;

[Severity: Critical]
Can an adversary bypass pKVM stage-2 memory protections here?

If an SMMU fails to bind to the hypervisor due to early returns in this
probe function (like missing a device tree node or hitting the cavium
erratum below), it is excluded from kvm_arm_smmu_array and the hypervisor
does not trap its MMIO.

When the native host arm_smmu_driver registers at device_initcall, it can
successfully bind to this unbound SMMU. Does this allow an adversary host
kernel at EL1 to natively program this untrapped SMMU to perform arbitrary
DMA into hypervisor or guest memory?

> +
> +	if (kvm_arm_smmu_cur >= kvm_arm_smmu_count)
> +		return -ENOSPC;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	if (of_property_read_bool(dev->of_node, "cavium,cn9900-broken-page1-regspace"))
> +		return -EINVAL;
[ ... ]
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 2043c6dc1bdf8..d6d1e2d30ef63 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
[ ... ]
> +static int arm_smmu_device_probe_emu(struct auxiliary_device *auxdev,
> +				     const struct auxiliary_device_id *id)
> +{
> +	struct device *parent = auxdev->dev.parent;
> +	void *group;
> +	int ret;
> +
> +	dev_info(&auxdev->dev, "Probing from %s\n", dev_name(parent));
> +	group = devres_open_group(parent, NULL, GFP_KERNEL);
> +	if (!group)
> +		return -ENOMEM;
> +
> +	ret = arm_smmu_device_probe(to_platform_device(parent));

[Severity: High]
Can this devres setup lead to a use-after-free during platform device
removal?

If the parent physical platform device is destroyed (for example, via
dynamic DT overlay removal), the driver core calls devres_release_all()
which processes cleanups in reverse order of addition (LIFO).

Because arm_smmu_device_probe() adds the smmu allocation to the parent's
devres list after the auxiliary device was created in
smmu_create_aux_device(), the smmu memory gets freed first.

Following that, the auxiliary_device_destroy devres action executes,
unregistering the aux device and triggering driver unbind via
arm_smmu_device_remove_emu() and arm_smmu_device_remove(). Does this
result in dereferencing the already freed smmu struct?

> +	if (ret)
> +		devres_release_group(parent, group);
> +	else
> +		devres_close_group(parent, group);
> +	return ret;
> +}
> +
> +static void arm_smmu_device_remove_emu(struct auxiliary_device *auxdev)
> +{
> +	arm_smmu_device_remove(to_platform_device(auxdev->dev.parent));
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922131259.2975334-1-smostafa@google.com?part=11