Re: [PATCH 1/2] PCI/sysfs: Fix out-of-bounds read in pci_write_legacy_io()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The Alpha implementation of `pci_legacy_write` passes arguments to `outb`, `outw`, and `outl` in the wrong order, causing arbitrary I/O port writes.
--

commit 21e9260d6465f8f037bb9292c32c44b565eca000
Author: Krzysztof Wilczyński <kwilczynski@kernel.org>

PCI/sysfs: Fix out-of-bounds read in pci_write_legacy_io()

This commit addresses an out-of-bounds read in pci_write_legacy_io() by
only reading the requested number of bytes using little-endian accessors.
It also removes byte-shifting workarounds in the PowerPC architecture
code that are no longer necessary.

>     On little-endian PowerPC (POWER8 and later), they extracted the wrong
>     bytes, so a 1-byte write wrote an out-of-bounds byte instead of the
>     requested value.  On big-endian, the native load also caused out_le16()
>     and out_le32() to reverse the user's bytes on the wire for 2 and 4 byte
>     writes.  The little-endian helpers resolve both issues, so the shifts
>     are removed.
>     
>     No changes are needed for the Alpha platform.
>     
>     The legacy_io file is root-only and exists only on Alpha and PowerPC,
>     the two architectures that define HAVE_PCI_LEGACY.

[Severity: High]
This is a pre-existing issue, but the claim that no changes are needed for
Alpha doesn't seem quite right.

Looking at pci_legacy_write() in arch/alpha/kernel/pci-sysfs.c, it calls the
I/O port functions with the port and value reversed:

arch/alpha/kernel/pci-sysfs.c:pci_legacy_write() {
    ...
	switch(size) {
	case 1:
		outb(port, val);
		return 1;
	case 2:
		if (port & 1)
			return -EINVAL;
		outw(port, val);
		return 2;
	case 4:
		if (port & 3)
			return -EINVAL;
		outl(port, val);
		return 4;
	}
    ...
}

Since Alpha's outb(), outw(), and outl() expect arguments in the order
(value, port), doesn't passing port as the value and val as the port
address incorrectly use user-supplied data as the target I/O port address?

Could this allow arbitrary I/O port writes system-wide?

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