From: dmkhn@proton.me
To: Stewart Hildebrand <stewart.hildebrand@amd.com>
Cc: xen-devel@lists.xenproject.org,
Bertrand Marquis <bertrand.marquis@arm.com>,
Rahul Singh <rahul.singh@arm.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Julien Grall <julien@xen.org>,
Michal Orzel <michal.orzel@amd.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v2 1/2] xen/arm: smmuv3: fix UB during deassign
Date: Fri, 25 Jul 2025 20:17:02 +0000 [thread overview]
Message-ID: <aIPmOvYVId3r+3kO@kraken> (raw)
In-Reply-To: <20250725174553.15322-1-stewart.hildebrand@amd.com>
On Fri, Jul 25, 2025 at 01:45:50PM -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>
> Acked-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Denis Mukhin <dmukhin@ford.com>
> ---
> 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
>
>
prev parent reply other threads:[~2025-07-25 20:17 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-25 17:45 [PATCH v2 1/2] xen/arm: smmuv3: fix UB during deassign Stewart Hildebrand
2025-07-25 17:45 ` [PATCH v2 2/2] xen/arm: smmuv3: fix xl pci-assignable-remove Stewart Hildebrand
2025-07-25 20:17 ` dmkhn
2025-07-25 20:17 ` dmkhn [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aIPmOvYVId3r+3kO@kraken \
--to=dmkhn@proton.me \
--cc=Volodymyr_Babchuk@epam.com \
--cc=bertrand.marquis@arm.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=rahul.singh@arm.com \
--cc=sstabellini@kernel.org \
--cc=stewart.hildebrand@amd.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.