Re: [PATCH v2 4/4] iommu/hyperv: Add page-selective IOTLB flush support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Bitfield assignment on an uninitialized hypercall buffer element causes a read of uninitialized memory, triggering a KMSAN uninit-value warning.
- [High] Missing check for an empty `iotlb_gather` causes spurious flushes of the entire IOTLB domain.
- [Medium] The calculation of `last_pfn` uses an unnecessarily complex formula that theoretically overflows for IOVAs near the end of the 64-bit address space.
--

commit 306d408fd1d04144672964e71dd7149937537189
Author: Yu Zhang <zhangyu1@linux.microsoft.com>

iommu/hyperv: Add page-selective IOTLB flush support

Adds page-selective IOTLB flush using HVCALL_FLUSH_DEVICE_DOMAIN_LIST.
Calculates the minimal power-of-two aligned range for the target IOVA region,
falling back to a full domain flush if the page-selective flush fails.

> diff --git a/drivers/iommu/hyperv/iommu.c b/drivers/iommu/hyperv/iommu.c
> index 2541369464040..e9b104a322fd8 100644
> --- a/drivers/iommu/hyperv/iommu.c
> +++ b/drivers/iommu/hyperv/iommu.c
> @@ -401,10 +402,74 @@ static void hv_iommu_flush_iotlb_all(struct iommu_domain *domain)
>  	hv_flush_device_domain(to_hv_iommu_domain(domain));
>  }
>  
> +/*
> + * Calculate the minimal power-of-two aligned range that covers [start, end]
> + * (end is inclusive). Returns a single (page_number, page_mask_shift)
> + * descriptor that may over-flush when the range is not naturally aligned.
> + */
> +static void hv_iommu_calc_flush_range(unsigned long start, unsigned long end,
> +				       union hv_iommu_flush_va *va)
> +{
> +	unsigned long start_pfn = HVPFN_DOWN(start);
> +	unsigned long last_pfn = HVPFN_UP(end + 1) - 1;

[Severity: Medium]
Can this calculation overflow?

If end is within the last 4095 bytes of the 64-bit address space, end + 1
would wrap around. HVPFN_UP() expands to (((x) + 4095) >> 12), which would
evaluate to 0 in that case.

Subtracting 1 would then result in ULONG_MAX, causing __fls() to compute a
mask_shift of 64 and triggering an undefined left shift by 64 in the later
ALIGN_DOWN macro.

Would it be safer to use HVPFN_DOWN(end) instead, which avoids the overflow
entirely?

> +	unsigned long mask_shift, aligned_pfn;

[ ... ]

> +static void hv_flush_device_domain_list(struct hv_iommu_domain *hv_domain,
> +					struct iommu_iotlb_gather *iotlb_gather)
> +{
> +	u64 status;
> +	unsigned long flags;
> +	struct hv_input_flush_device_domain_list *input;
> +
> +	local_irq_save(flags);
> +
> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +	memset(input, 0, sizeof(*input));
> +
> +	input->device_domain = hv_domain->device_domain;
> +	input->flags |= HV_FLUSH_DEVICE_DOMAIN_LIST_IOMMU_FORMAT;
> +	hv_iommu_calc_flush_range(iotlb_gather->start, iotlb_gather->end,
> +				  &input->iova_list[0]);

[Severity: High]
Does this code trigger an uninitialized memory read KMSAN warning?

Since iova_list is a flexible array member, sizeof(*input) evaluates to just
the header size (24 bytes). This means memset() clears the header, but leaves
input->iova_list[0] uninitialized.

When hv_iommu_calc_flush_range() later assigns the page_number and
page_mask_shift bitfields of the union, the compiler typically generates a
read-modify-write operation on the underlying 64-bit value. This will read
the uninitialized memory from the reused per-CPU page.

> +
> +	status = hv_do_rep_hypercall(HVCALL_FLUSH_DEVICE_DOMAIN_LIST,

[ ... ]

>  static void hv_iommu_iotlb_sync(struct iommu_domain *domain,
>  				struct iommu_iotlb_gather *iotlb_gather)
>  {
> -	hv_flush_device_domain(to_hv_iommu_domain(domain));
> +	hv_flush_device_domain_list(to_hv_iommu_domain(domain), iotlb_gather);

[Severity: High]
Does this unconditionally flush the entire IOTLB domain if the gather is
empty?

If iommu_unmap() returns without unmapping anything, iotlb_gather retains its
initialized values of start = ULONG_MAX and end = 0.

Because there is no check like if (iotlb_gather->start > iotlb_gather->end)
before issuing the flush, hv_iommu_calc_flush_range() will calculate a
mask_shift of 52. This causes a spurious flush of the full 52-bit address
space instead of returning early.

>  
>  	iommu_put_pages_list(&iotlb_gather->freelist);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702160518.311234-1-zhangyu1@linux.microsoft.com?part=4