Re: [PATCH] x86/hyperv: Cosmetic changes in irqdomain.c for readability

[sashiko-bot@kernel.org]

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 <mrathor@linux.microsoft.com>

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/20260601225116.956392-1-mrathor@linux.microsoft.com?part=1