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