Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] Allocation or mapping failure in `hv_irq_compose_msi_msg()` causes the function to return without initializing the caller-provided `msg` structure, potentially leading to uncontrolled MSI writes by the device. - [High] The `hv_msi_domain_free` function fails to iterate through allocated IRQs for multi-vector MSI, leading to leaked memory and stale hypervisor mappings. -- commit 042e553a37dead023377268bdb9c0d43c5394d75 Author: Mukesh R <[email protected]> x86/hyperv: Cosmetic changes in irqdomain.c for readability This commit makes various cosmetic changes to irqdomain.c, such as renaming variables, fixing indentation, replacing spaces with tabs, and resolving checkpatch warnings, with no functional changes intended. > diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c > index 365e364268d9a..b3ad50a874dca 100644 > --- a/arch/x86/hyperv/irqdomain.c > +++ b/arch/x86/hyperv/irqdomain.c [ ... ] > @@ -249,10 +253,8 @@ static void hv_irq_compose_msi_msg(struct irq_data > *data, struct msi_msg *msg) > } > > stored_entry = kzalloc_obj(*stored_entry, GFP_ATOMIC); > - if (!stored_entry) { > - pr_debug("%s: failed to allocate chip data\n", __func__); > + if (!stored_entry) > return; > - } > > ret = hv_map_msi_interrupt(data, stored_entry); > if (ret) { > kfree(stored_entry); > return; > } [Severity: High] This is a pre-existing issue, but does returning early here leave the caller-provided msg structure uninitialized? If kzalloc_obj() or hv_map_msi_interrupt() fails in hv_irq_compose_msi_msg(), the function returns without populating msg. Because the callback returns void, the error is swallowed and the caller might write the uninitialized contents of msg to the device's MSI address/data registers, potentially causing the device to perform DMA writes to arbitrary or null physical addresses when an interrupt fires. [ ... ] > -static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq, > unsigned int nr_irqs) > +static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq, > + unsigned int nr_irqs) > { > for (int i = 0; i < nr_irqs; ++i) { > struct irq_data *irqd = irq_domain_get_irq_data(d, virq); [Severity: High] This is a pre-existing issue, but does this loop process the same base IRQ on every iteration? By calling irq_domain_get_irq_data() with virq rather than virq + i inside hv_msi_domain_free(), subsequent iterations will skip the remaining interrupts in a multi-vector MSI scenario. This could leave them unfreed and potentially lead to leaked kernel memory and stale hypervisor mappings. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
