[PATCH v2] alpha/PCI: Fix I/O port accessor argument order in pci_legacy_write()

[PATCH v2] alpha/PCI: Fix I/O port accessor argument order in pci_legacy_write()

[Krzysztof Wilczyński]

pci_legacy_write() in arch/alpha/kernel/pci-sysfs.c passes its arguments
to outb(), outw() and outl() in the wrong order:

  outb(port, val);

The Alpha I/O accessors in arch/alpha/include/asm/io.h take the value
first and the port second:

  extern void outb(u8 b, unsigned long port);

So the port number is written as data to the I/O address taken from the
user-supplied value, and the intended write to the requested port never
happens.

The arguments have been reversed since the file was added, and the
function returns the access size regardless, so the caller sees success
while the requested port is left untouched.

Fixes: 10a0ef39fbd1 ("PCI/alpha: pci sysfs resources")
Tested-by: Magnus Lindholm <linmag7@gmail.com>
Reviewed-by: Magnus Lindholm <linmag7@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
---
Changes in v2:
 https://lore.kernel.org/linux-pci/20260612232400.585195-1-kwilczynski@kernel.org/

 - Collected Reviewed-by and Tested-by tags from Magnus Lindholm.

 arch/alpha/kernel/pci-sysfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c
index 94dbc470cd6c..7050f0f7fe3d 100644
--- a/arch/alpha/kernel/pci-sysfs.c
+++ b/arch/alpha/kernel/pci-sysfs.c
@@ -224,17 +224,17 @@ int pci_legacy_write(struct pci_bus *bus, loff_t port, u32 val, size_t size)
 
 	switch(size) {
 	case 1:
-		outb(port, val);
+		outb(val, port);
 		return 1;
 	case 2:
 		if (port & 1)
 			return -EINVAL;
-		outw(port, val);
+		outw(val, port);
 		return 2;
 	case 4:
 		if (port & 3)
 			return -EINVAL;
-		outl(port, val);
+		outl(val, port);
 		return 4;
 	}
 	return -EINVAL;
-- 
2.55.0

Re: [PATCH v2] alpha/PCI: Fix I/O port accessor argument order in pci_legacy_write()

[sashiko-bot]

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

Pre-existing issues:
- [High] KASAN Slab-Out-of-Bounds Read due to blind 32-bit cast on dynamically allocated buffer.
- [High] Missing kernel lockdown checks in legacy PCI sysfs interfaces allows Secure Boot bypass.
--

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

alpha/PCI: Fix I/O port accessor argument order in pci_legacy_write()

This patch corrects the argument order for outb(), outw(), and outl()
in the Alpha architecture's pci_legacy_write() function. The previous
implementation wrote the port number as data to the address derived from
the user-supplied value. By swapping the arguments to match the expected
value and port order, the intended I/O port writes are now performed
correctly.

> diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c
> index 94dbc470cd6c0..7050f0f7fe3d7 100644
> --- a/arch/alpha/kernel/pci-sysfs.c
> +++ b/arch/alpha/kernel/pci-sysfs.c
> @@ -224,17 +224,17 @@ int pci_legacy_write(struct pci_bus *bus, loff_t port, u32 val, size_t size)

[Severity: High]
This is a pre-existing issue, but could the caller of this function trigger a
KASAN slab-out-of-bounds read when passing the val argument?

Looking at drivers/pci/pci-sysfs.c:pci_write_legacy_io(), the buffer is cast
directly to a 32-bit integer before calling pci_legacy_write():

    return pci_legacy_write(bus, off, *(u32 *)buf, count);

If a user performs a 1-byte or 2-byte write to the legacy_io sysfs file,
fs/kernfs/file.c:kernfs_fop_write_iter() dynamically allocates buf to the
exact write length plus a null terminator:

    buf = kmalloc(len + 1, GFP_KERNEL);

Unconditionally casting this smaller buffer to u32 * and dereferencing it
reads past the allocation boundary. Should the buffer cast be bounded by the
count parameter?

[Severity: High]
This is also a pre-existing issue, but does this legacy write path allow a
Kernel Lockdown and Secure Boot bypass?

Unlike standard resource handlers like pci_write_resource(), the generic
handlers in drivers/pci/pci-sysfs.c for legacy endpoints completely omit
the security_locked_down() checks.

For example, drivers/pci/pci-sysfs.c:pci_write_legacy_io() allows writes:

    static ssize_t pci_write_legacy_io(struct file *filp, struct kobject *kobj,
                                       const struct bin_attribute *bin_attr,
                                       char *buf, loff_t off, size_t count)

And drivers/pci/pci-sysfs.c:pci_mmap_legacy_mem() allows raw memory access:

    static int pci_mmap_legacy_mem(struct file *filp, struct kobject *kobj,
                                   const struct bin_attribute *attr,
                                   struct vm_area_struct *vma)

Neither function checks security_locked_down(LOCKDOWN_PCI_ACCESS). Can a
privileged user exploit this to write directly to hardware I/O ports or mmap
legacy memory?

>  
>  	switch(size) {
>  	case 1:
> -		outb(port, val);
> +		outb(val, port);
>  		return 1;

[ ... ]

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