Re: [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Directly modifying a u32 buffer via a char pointer cast assumes Little Endian memory layout, clearing the wrong byte and corrupting adjacent registers on Big Endian architectures.
- [Medium] The patch hardwires PCI_INTERRUPT_PIN to 0 for 16/32-bit reads but completely ignores PCI_INTERRUPT_LINE, creating an inconsistent configuration space state.
--

commit badaaf9419fe573b12895ca135de8cb0ded77d3e
Author: Mukesh R <mrathor@linux.microsoft.com>

PCI: hv: hardwire PCI_INTERRUPT_PIN to 0

Hyper-V DDA does not support legacy interrupts, so the driver intercepts
8-bit configuration reads to hardwire PCI_INTERRUPT_PIN to 0. This commit
attempts to also hardwire the pin for 16-bit and 32-bit reads, which
previously bypassed the emulation and leaked host hardware values.

> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 89816a2bd7cd3..bb4e6ca4ac1ce 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1240,6 +1240,16 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
>  			mb();
>  		}
>  		spin_unlock_irqrestore(&hbus->config_lock, flags);
> +
> +		/*
> +		 * Make sure PCI_INTERRUPT_PIN is hard-wired to 0, since it
> +		 * may be read using a 32bit read, which is skipped by the
> +		 * above emulation.
> +		 */
> +		if (where <= PCI_INTERRUPT_PIN &&
> +		    PCI_INTERRUPT_PIN < (where + size))

[Severity: Medium]
Will this leave PCI_INTERRUPT_LINE leaking host hardware state on 16-bit
and 32-bit reads?

The original code correctly hardwires both PCI_INTERRUPT_LINE and
PCI_INTERRUPT_PIN to 0 for 8-bit reads. By only checking and clearing
PCI_INTERRUPT_PIN here, a 16-bit or 32-bit read starting at
PCI_INTERRUPT_LINE will bypass the emulation and return the raw host
hardware legacy configuration to the guest.

> +			*((char *)val + PCI_INTERRUPT_PIN - where) = 0;

[Severity: High]
Could this corrupt adjacent configuration registers on big-endian
architectures?

This pointer cast directly modifies the memory backing the u32 val buffer.
Because a u32 is stored in CPU native endianness, treating it as a char
array assumes a little-endian layout.

On a big-endian guest, the computed byte offset will point to the wrong byte
in the u32, such as clearing PCI_MIN_GNT instead of PCI_INTERRUPT_PIN. This
would leave the intended pin unmodified while corrupting another register.

> +
>  	} else {
>  		dev_err(dev, "Attempt to read beyond a function's config space.\n");
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913232255.2801797-1-wei.liu@kernel.org?part=1