mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 22:54:17 -04:00
PCI/sysfs: Fix out-of-bounds read in pci_write_legacy_io()
pci_write_legacy_io() loads 4 bytes from the kernfs write buffer
regardless of how many bytes userspace wrote:
if (count != 1 && count != 2 && count != 4)
return -EINVAL;
return pci_legacy_write(bus, off, *(u32 *)buf, count);
kernfs_fop_write_iter() allocates the buffer with kmalloc(len + 1),
so a 1-byte write to the legacy_io sysfs file allocates 2 bytes and
the unconditional u32 load reads up to 2 bytes past the end of the
allocation, which KASAN reports as a slab-out-of-bounds read.
Similarly, a 2-byte write overreads by 1 byte.
Thus, read only the number of bytes requested using get_unaligned_le16()
and get_unaligned_le32() for the 2 and 4 byte cases, interpreting the
buffer as little-endian to match the byte ordering of PCI I/O port
space.
The PowerPC implementation previously compensated for the generic
code's native-endian 32-bit load by shifting the value into place
for the 1 and 2 byte cases. The shifts were only correct on
big-endian kernels.
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.
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260616163131.2763281-1-kwilczynski@kernel.org
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
This commit is contained in:
@@ -626,19 +626,14 @@ int pci_legacy_write(struct pci_bus *bus, loff_t port, u32 val, size_t size)
|
||||
return -ENXIO;
|
||||
addr = hose->io_base_virt + port;
|
||||
|
||||
/* WARNING: The generic code is idiotic. It gets passed a pointer
|
||||
* to what can be a 1, 2 or 4 byte quantity and always reads that
|
||||
* as a u32, which means that we have to correct the location of
|
||||
* the data read within those 32 bits for size 1 and 2
|
||||
*/
|
||||
switch(size) {
|
||||
case 1:
|
||||
out_8(addr, val >> 24);
|
||||
out_8(addr, val);
|
||||
return 1;
|
||||
case 2:
|
||||
if (port & 1)
|
||||
return -EINVAL;
|
||||
out_le16(addr, val >> 16);
|
||||
out_le16(addr, val);
|
||||
return 2;
|
||||
case 4:
|
||||
if (port & 3)
|
||||
|
||||
@@ -913,12 +913,24 @@ static ssize_t pci_write_legacy_io(struct file *filp, struct kobject *kobj,
|
||||
char *buf, loff_t off, size_t count)
|
||||
{
|
||||
struct pci_bus *bus = to_pci_bus(kobj_to_dev(kobj));
|
||||
u32 val;
|
||||
|
||||
/* Only support 1, 2 or 4 byte accesses */
|
||||
if (count != 1 && count != 2 && count != 4)
|
||||
/* Only support 1, 2 or 4 byte accesses. */
|
||||
switch (count) {
|
||||
case 1:
|
||||
val = *(u8 *)buf;
|
||||
break;
|
||||
case 2:
|
||||
val = get_unaligned_le16(buf);
|
||||
break;
|
||||
case 4:
|
||||
val = get_unaligned_le32(buf);
|
||||
break;
|
||||
default:
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
return pci_legacy_write(bus, off, *(u32 *)buf, count);
|
||||
return pci_legacy_write(bus, off, val, count);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user