* [PATCH 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove
2025-07-23 22:54 [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign Stewart Hildebrand
@ 2025-07-23 22:54 ` Stewart Hildebrand
2025-07-24 1:00 ` dmkhn
2025-07-24 0:59 ` [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign dmkhn
2025-07-25 6:12 ` Bertrand Marquis
2 siblings, 1 reply; 7+ messages in thread
From: Stewart Hildebrand @ 2025-07-23 22:54 UTC (permalink / raw)
To: xen-devel
Cc: Stewart Hildebrand, Bertrand Marquis, Rahul Singh,
Stefano Stabellini, Julien Grall, Michal Orzel, Volodymyr Babchuk
When attempting to xl pci-assignable-remove a PCI device, we encounter:
$ xl pci-assignable-remove 00:01.0
(XEN) SMMUv3: <no-node>: not attached to domain 32753
(XEN) d[IO]: deassign (0000:00:01.0) failed (-3)
libxl: error: libxl_pci.c:910:libxl__device_pci_assignable_remove: failed to de-quarantine 0000:00:01.0
When a PCI device is being deassigned from domIO,
arm_smmu_deassign_dev() should return before checking the smmu domain.
Fixes: 63919fc4d1ca ("xen/arm: smmuv3: Add PCI devices support for SMMUv3")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
xen/drivers/passthrough/arm/smmu-v3.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
index db08d3c04269..9312bb3c72d8 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -2747,11 +2747,6 @@ static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct device
struct arm_smmu_domain *smmu_domain = to_smmu_domain(io_domain);
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
- if (!smmu_domain || smmu_domain->d != d) {
- dev_err(dev, " not attached to domain %d\n", d->domain_id);
- return -ESRCH;
- }
-
#ifdef CONFIG_HAS_PCI
if ( dev_is_pci(dev) )
{
@@ -2767,6 +2762,11 @@ static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct device
}
#endif
+ if (!smmu_domain || smmu_domain->d != d) {
+ dev_err(dev, " not attached to domain %d\n", d->domain_id);
+ return -ESRCH;
+ }
+
spin_lock(&xen_domain->lock);
arm_smmu_detach_dev(master);
--
2.50.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove
2025-07-23 22:54 ` [PATCH 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove Stewart Hildebrand
@ 2025-07-24 1:00 ` dmkhn
2025-07-25 6:13 ` Bertrand Marquis
0 siblings, 1 reply; 7+ messages in thread
From: dmkhn @ 2025-07-24 1:00 UTC (permalink / raw)
To: Stewart Hildebrand
Cc: xen-devel, Bertrand Marquis, Rahul Singh, Stefano Stabellini,
Julien Grall, Michal Orzel, Volodymyr Babchuk
On Wed, Jul 23, 2025 at 06:54:20PM -0400, Stewart Hildebrand wrote:
> When attempting to xl pci-assignable-remove a PCI device, we encounter:
>
> $ xl pci-assignable-remove 00:01.0
> (XEN) SMMUv3: <no-node>: not attached to domain 32753
> (XEN) d[IO]: deassign (0000:00:01.0) failed (-3)
> libxl: error: libxl_pci.c:910:libxl__device_pci_assignable_remove: failed to de-quarantine 0000:00:01.0
>
> When a PCI device is being deassigned from domIO,
> arm_smmu_deassign_dev() should return before checking the smmu domain.
>
> Fixes: 63919fc4d1ca ("xen/arm: smmuv3: Add PCI devices support for SMMUv3")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
> xen/drivers/passthrough/arm/smmu-v3.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
> index db08d3c04269..9312bb3c72d8 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -2747,11 +2747,6 @@ static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct device
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(io_domain);
> struct arm_smmu_master *master = dev_iommu_priv_get(dev);
>
> - if (!smmu_domain || smmu_domain->d != d) {
> - dev_err(dev, " not attached to domain %d\n", d->domain_id);
> - return -ESRCH;
> - }
> -
> #ifdef CONFIG_HAS_PCI
> if ( dev_is_pci(dev) )
> {
> @@ -2767,6 +2762,11 @@ static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct device
> }
> #endif
>
> + if (!smmu_domain || smmu_domain->d != d) {
> + dev_err(dev, " not attached to domain %d\n", d->domain_id);
Use %pd?
dev_err(dev, " not attached to %pd\n", d);
> + return -ESRCH;
> + }
> +
> spin_lock(&xen_domain->lock);
>
> arm_smmu_detach_dev(master);
> --
> 2.50.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove
2025-07-24 1:00 ` dmkhn
@ 2025-07-25 6:13 ` Bertrand Marquis
0 siblings, 0 replies; 7+ messages in thread
From: Bertrand Marquis @ 2025-07-25 6:13 UTC (permalink / raw)
To: dmkhn@proton.me
Cc: Stewart Hildebrand, xen-devel@lists.xenproject.org, Rahul Singh,
Stefano Stabellini, Julien Grall, Michal Orzel, Volodymyr Babchuk
> On 24 Jul 2025, at 03:00, dmkhn@proton.me wrote:
>
> On Wed, Jul 23, 2025 at 06:54:20PM -0400, Stewart Hildebrand wrote:
>> When attempting to xl pci-assignable-remove a PCI device, we encounter:
>>
>> $ xl pci-assignable-remove 00:01.0
>> (XEN) SMMUv3: <no-node>: not attached to domain 32753
>> (XEN) d[IO]: deassign (0000:00:01.0) failed (-3)
>> libxl: error: libxl_pci.c:910:libxl__device_pci_assignable_remove: failed to de-quarantine 0000:00:01.0
>>
>> When a PCI device is being deassigned from domIO,
>> arm_smmu_deassign_dev() should return before checking the smmu domain.
>>
>> Fixes: 63919fc4d1ca ("xen/arm: smmuv3: Add PCI devices support for SMMUv3")
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> ---
>> xen/drivers/passthrough/arm/smmu-v3.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
>> index db08d3c04269..9312bb3c72d8 100644
>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>> @@ -2747,11 +2747,6 @@ static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct device
>> struct arm_smmu_domain *smmu_domain = to_smmu_domain(io_domain);
>> struct arm_smmu_master *master = dev_iommu_priv_get(dev);
>>
>> - if (!smmu_domain || smmu_domain->d != d) {
>> - dev_err(dev, " not attached to domain %d\n", d->domain_id);
>> - return -ESRCH;
>> - }
>> -
>> #ifdef CONFIG_HAS_PCI
>> if ( dev_is_pci(dev) )
>> {
>> @@ -2767,6 +2762,11 @@ static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn, struct device
>> }
>> #endif
>>
>> + if (!smmu_domain || smmu_domain->d != d) {
>> + dev_err(dev, " not attached to domain %d\n", d->domain_id);
>
> Use %pd?
>
> dev_err(dev, " not attached to %pd\n", d);
Agree.
With this fixed:
Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers
Bertrand
>
>> + return -ESRCH;
>> + }
>> +
>> spin_lock(&xen_domain->lock);
>>
>> arm_smmu_detach_dev(master);
>> --
>> 2.50.1
>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign
2025-07-23 22:54 [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign Stewart Hildebrand
2025-07-23 22:54 ` [PATCH 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove Stewart Hildebrand
@ 2025-07-24 0:59 ` dmkhn
2025-07-25 6:15 ` Bertrand Marquis
2025-07-25 6:12 ` Bertrand Marquis
2 siblings, 1 reply; 7+ messages in thread
From: dmkhn @ 2025-07-24 0:59 UTC (permalink / raw)
To: Stewart Hildebrand
Cc: xen-devel, Bertrand Marquis, Rahul Singh, Stefano Stabellini,
Julien Grall, Michal Orzel, Volodymyr Babchuk
On Wed, Jul 23, 2025 at 06:54:19PM -0400, Stewart Hildebrand 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@amd.com>
> ---
> 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);
I think positive case first will be more readable.
E.g.:
if ( dom )
return container_of(dom, struct arm_smmu_domain, domain);
return NULL;
> }
>
>
> base-commit: 5c798ac8854af3528a78ca5a622c9ea68399809b
> --
> 2.50.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign
2025-07-24 0:59 ` [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign dmkhn
@ 2025-07-25 6:15 ` Bertrand Marquis
0 siblings, 0 replies; 7+ messages in thread
From: Bertrand Marquis @ 2025-07-25 6:15 UTC (permalink / raw)
To: dmkhn@proton.me
Cc: Stewart Hildebrand, xen-devel@lists.xenproject.org, Rahul Singh,
Stefano Stabellini, Julien Grall, Michal Orzel, Volodymyr Babchuk
Hi,
> On 24 Jul 2025, at 02:59, dmkhn@proton.me wrote:
>
> On Wed, Jul 23, 2025 at 06:54:19PM -0400, Stewart Hildebrand 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@amd.com>
>> ---
>> 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);
>
> I think positive case first will be more readable.
> E.g.:
>
> if ( dom )
> return container_of(dom, struct arm_smmu_domain, domain);
>
> return NULL;
>
Exiting early on error case instead of doing the handling inside a if sounds cleaner
to me.
If more needs to be done inside the function then exiting early will make things
easier. It does not really matter now but might in the future hence why i acked
the original version instead of your suggestion.
Regards
Bertrand
>> }
>>
>>
>> base-commit: 5c798ac8854af3528a78ca5a622c9ea68399809b
>> --
>> 2.50.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign
2025-07-23 22:54 [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign Stewart Hildebrand
2025-07-23 22:54 ` [PATCH 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove Stewart Hildebrand
2025-07-24 0:59 ` [PATCH 1/2] xen/arm: smmuv3: fix UB during deassign dmkhn
@ 2025-07-25 6:12 ` Bertrand Marquis
2 siblings, 0 replies; 7+ messages in thread
From: Bertrand Marquis @ 2025-07-25 6:12 UTC (permalink / raw)
To: Stewart Hildebrand
Cc: xen-devel@lists.xenproject.org, Rahul Singh, Stefano Stabellini,
Julien Grall, Michal Orzel, Volodymyr Babchuk
Hi,
> On 24 Jul 2025, at 00:54, Stewart Hildebrand <Stewart.Hildebrand@amd.com> 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@amd.com>
Sounds good to me:
Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread