Re: [PATCH v2 3/4] iommu/hyperv: Add para-virtualized IOMMU support for Hyper-V guest

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Undefined behavior when calculating `aperture_end` if the hypervisor returns `max_iova_width >= 64`.
- [High] Unconditional overwrite of `x86_init.iommu.iommu_init` causes hardware IOMMUs (VT-d/AMD) to fail initialization.
--

commit fdcb0264dfd6bd1cb7c3c4676b8a796ad8a85e4a
Author: Yu Zhang <[email protected]>

iommu/hyperv: Add para-virtualized IOMMU support for Hyper-V guest

Add a para-virtualized IOMMU driver for Linux guests running on Hyper-V.
This driver implements stage-1 IO translation within the guest OS and
integrates with the Linux IOMMU core using Hyper-V hypercalls.

> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 55a8b6de2865a..094f9f7ddb728 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -578,6 +578,10 @@ void __init hyperv_init(void)
>  	old_setup_percpu_clockev = x86_init.timers.setup_percpu_clockev;
>  	x86_init.timers.setup_percpu_clockev = hv_stimer_setup_percpu_clockev;
>  
> +#ifdef CONFIG_HYPERV_PVIOMMU
> +	x86_init.iommu.iommu_init = hv_iommu_init;

[Severity: High]
The assignment to x86_init.iommu.iommu_init must be conditionally guarded to
prevent hardware IOMMU initialization failures.

Will this unconditional overwrite break hardware IOMMUs?

Initial boot sequence logic and IOMMU detection parameters are validated.

If pci_iommu_alloc() previously detected a hardware IOMMU (like VT-d/AMD) and
set iommu_detected=1, overwriting the pointer with hv_iommu_init() ensures
failure since hv_iommu_init() immediately aborts when iommu_detected==1.

> +#endif
> +
>  	hv_apic_init();

[ ... ]

> diff --git a/drivers/iommu/hyperv/iommu.c b/drivers/iommu/hyperv/iommu.c
> new file mode 100644
> index 0000000000000..2541369464040
> --- /dev/null
> +++ b/drivers/iommu/hyperv/iommu.c

[ ... ]

> @@ -509,6 +509,16 @@ static void __init hv_init_iommu_device(struct hv_iommu_dev *hv_iommu,
> +	hv_iommu->geometry = (struct iommu_domain_geometry) {
> +		.aperture_start = 0,
> +		.aperture_end = (((u64)1) << hv_iommu->max_iova_width) - 1,

[Severity: High]
The max_iova_width value must be bounds-checked against the maximum 64-bit
integer width before performing the bitwise shift.

Does this code overflow the 64-bit shift limit?

Standard processing applied to the hypercall response.

If an untrusted hypervisor responds to HVCALL_GET_IOMMU_CAPABILITIES with
max_iova_width >= 64, shifting ((u64)1) directly by that amount results in
undefined behavior and unpredictable IOVA aperture sizing.

> +		.force_aperture = true,
> +	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3