Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] iommu/hyperv: missing detach causes silent attach failures - [Medium] iommu/hyperv: page table UAF due to wrong domain teardown order - [Low] iommu/hyperv: missing MODULE_IMPORT_NS causes modpost warning --
commit e9c9e7afc1e982c251a775443c1fa2af0f333b80 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 utilizing Hyper-V hypercalls. diff --git a/drivers/iommu/hyperv/hv-iommu-guest.c b/drivers/iommu/hyperv/hv-iommu-guest.c new file mode 100644 index 0000000000000..341d70eed5924 --- /dev/null +++ b/drivers/iommu/hyperv/hv-iommu-guest.c [ ... ] > +static int hv_iommu_attach_dev(struct iommu_domain *domain, struct device > *dev, > + struct iommu_domain *old) > +{ [ ... ] > + status = hv_do_hypercall(HVCALL_ATTACH_DEVICE_DOMAIN, input, NULL); > + > + local_irq_restore(flags); > + > + if (!hv_result_success(status) && > + hv_result(status) != HV_STATUS_DEVICE_ALREADY_IN_DOMAIN) { [Severity: High] If a device is already attached to a domain (e.g., blocking or identity), HVCALL_ATTACH_DEVICE_DOMAIN fails with HV_STATUS_DEVICE_ALREADY_IN_DOMAIN. By ignoring this error code and returning success without detaching from the old domain first, does the IOMMU core assume the device is successfully isolated in the new domain while it actually remains in the old domain? Could this silently break DMA isolation? > + hv_status_err(status, "HVCALL_ATTACH_DEVICE_DOMAIN failed\n"); > + return hv_result_to_errno(status); > + } > + > + if (domain != &hv_blocking_domain.domain && > + !pdev->ats_enabled && [ ... ] > +static void hv_iommu_paging_domain_free(struct iommu_domain *domain) > +{ > + struct hv_iommu_domain *hv_domain = to_hv_iommu_domain(domain); > + > + /* Free all remaining mappings */ > + pt_iommu_deinit(&hv_domain->pt_iommu); > + > + hv_delete_device_domain(hv_domain); [Severity: Medium] In hv_iommu_paging_domain_free(), the driver frees the guest memory backing the page tables via pt_iommu_deinit() before instructing the hypervisor to destroy the domain via hv_delete_device_domain(). Since the hypervisor's device domain is still active and holds the page table root pointer, could the hypervisor access freed memory if it performs background invalidations or delayed faults before the domain is destroyed? Should the domain be deleted before the page tables are freed? > + > + kfree(hv_domain); > +} [ ... ] > +static struct iommu_domain *hv_iommu_domain_alloc_paging(struct device *dev) > +{ [ ... ] > + cfg.top_level = (hv_iommu_device->max_iova_width > 48) ? 4 : 3; > + > + ret = pt_iommu_x86_64_init(&hv_domain->pt_iommu_x86_64, &cfg, > GFP_KERNEL); [Severity: Low] The driver selects GENERIC_PT and calls pt_iommu_x86_64_init(), which is exported into the GENERIC_PT_IOMMU namespace. Without a MODULE_IMPORT_NS("GENERIC_PT_IOMMU") declaration, will this cause modpost warnings during the kernel build? > + if (ret) > + goto err_free; > + > + /* Constrain to page sizes the hypervisor supports */ [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
