Re: [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi,

> On 24 Jul 2025, at 00:54, Stewart Hildebrand <Stewart.Hildebrand@xxxxxxx> 
> wrote:
> 
> In arm_smmu_deassign_dev(), the return value from to_smmu_domain() is
> NULL-checked. However, the implementation of to_smmu_domain() is a
> container_of lookup, so the return value is unlikely to ever be NULL. In
> case of a NULL argument to to_smmu_domain(), we will attempt to
> dereference the non-NULL return value and encounter undefined behavior
> and a crash:
> 
> $ xl pci-assignable-remove 00:01.0
> (XEN) 
> ================================================================================
> (XEN) UBSAN: Undefined behaviour in drivers/passthrough/arm/smmu-v3.c:221:9
> (XEN) applying non-zero offset ffffffffffffffc0 to null pointer
> (XEN) Xen WARN at common/ubsan/ubsan.c:174
> (XEN) ----[ Xen-4.21-unstable  arm64  debug=y ubsan=y  Tainted:   C    ]----
> ...
> (XEN) Xen call trace:
> (XEN)    [<00000a0000350b2c>] ubsan.c#ubsan_epilogue+0x14/0xf0 (PC)
> (XEN)    [<00000a00003523e0>] __ubsan_handle_pointer_overflow+0x94/0x13c (LR)
> (XEN)    [<00000a00003523e0>] __ubsan_handle_pointer_overflow+0x94/0x13c
> (XEN)    [<00000a0000392f9c>] smmu-v3.c#to_smmu_domain+0x3c/0x40
> (XEN)    [<00000a000039e428>] smmu-v3.c#arm_smmu_deassign_dev+0x54/0x43c
> (XEN)    [<00000a00003a0300>] smmu-v3.c#arm_smmu_reassign_dev+0x74/0xc8
> (XEN)    [<00000a00003a7040>] pci.c#deassign_device+0x5fc/0xe0c
> (XEN)    [<00000a00003ade7c>] iommu_do_pci_domctl+0x7b4/0x90c
> (XEN)    [<00000a00003a34c0>] iommu_do_domctl+0x58/0xf4
> (XEN)    [<00000a00002ca66c>] do_domctl+0x2690/0x2a04
> (XEN)    [<00000a0000454d88>] traps.c#do_trap_hypercall+0xcf4/0x15b0
> (XEN)    [<00000a0000458588>] do_trap_guest_sync+0xa88/0xdd8
> (XEN)    [<00000a00003f8480>] entry.o#guest_sync_slowpath+0xa8/0xd8
> (XEN)
> (XEN) 
> ================================================================================
> (XEN) Data Abort Trap. Syndrome=0x4
> (XEN) Walking Hypervisor VA 0xfffffffffffffff8 on CPU1 via TTBR 
> 0x00000000406d0000
> (XEN) 0TH[0x1ff] = 0x0
> (XEN) CPU1: Unexpected Trap: Data Abort
> (XEN) ----[ Xen-4.21-unstable  arm64  debug=y ubsan=y  Tainted:   C    ]----
> ...
> (XEN) Xen call trace:
> (XEN)    [<00000a000039e494>] smmu-v3.c#arm_smmu_deassign_dev+0xc0/0x43c (PC)
> (XEN)    [<00000a000039e428>] smmu-v3.c#arm_smmu_deassign_dev+0x54/0x43c (LR)
> (XEN)    [<00000a00003a0300>] smmu-v3.c#arm_smmu_reassign_dev+0x74/0xc8
> (XEN)    [<00000a00003a7040>] pci.c#deassign_device+0x5fc/0xe0c
> (XEN)    [<00000a00003ade7c>] iommu_do_pci_domctl+0x7b4/0x90c
> (XEN)    [<00000a00003a34c0>] iommu_do_domctl+0x58/0xf4
> (XEN)    [<00000a00002ca66c>] do_domctl+0x2690/0x2a04
> (XEN)    [<00000a0000454d88>] traps.c#do_trap_hypercall+0xcf4/0x15b0
> (XEN)    [<00000a0000458588>] do_trap_guest_sync+0xa88/0xdd8
> (XEN)    [<00000a00003f8480>] entry.o#guest_sync_slowpath+0xa8/0xd8
> 
> Fix by changing to_smmu_domain() to return NULL in case of a NULL
> argument.
> 
> Fixes: 452ddbe3592b ("xen/arm: smmuv3: Add support for SMMUv3 driver")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>

Sounds good to me:
Acked-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> I'm unsure if that's the right Fixes: tag since I'm not sure if it can
> be triggered prior to 63919fc4d1ca ("xen/arm: smmuv3: Add PCI devices
> support for SMMUv3").
> ---
> xen/drivers/passthrough/arm/smmu-v3.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index 58f3331520df..db08d3c04269 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -218,6 +218,9 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
> 
> static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> {
> + if ( !dom )
> + return NULL;
> +
> return container_of(dom, struct arm_smmu_domain, domain);
> }
> 
> 
> base-commit: 5c798ac8854af3528a78ca5a622c9ea68399809b
> -- 
> 2.50.1
>