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 <[email protected]> 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/[email protected]?part=4
