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

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

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>
Acked-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
---
v1->v2:
* add Bertrand's A-b

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: 6cd9b9226c65c962b0f0e040e7d3d5c4053f8e06
-- 
2.50.1