Re: [PATCH] PCI/proc: Warn on writes to kernel-exclusive config space regions

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer truncation of the 64-bit file offset to a 32-bit signed int allows bypassing bounds checks and the new exclusive region check, leading to massive out-of-bounds memory writes.
--

--- Patch [1]: [PATCH] PCI/proc: Warn on writes to kernel-exclusive config space regions ---
commit ae079df2d52cc5cc8959cf281ed5e461ff2fcfd8
Author: Krzysztof Wilczyński <kwilczynski@kernel.org>

PCI/proc: Warn on writes to kernel-exclusive config space regions

This patch updates proc_bus_pci_write() to check if a user space write
targets a kernel-exclusive config space region. If so, it emits a warning
and taints the kernel, bringing the procfs interface to parity with the
sysfs interface.

> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index 71ad289fcb8e3..12200979d3a82 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
[ ... ]
> @@ -122,6 +124,12 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
>  	if (ret)
>  		return ret;
>  
> +	if (resource_is_exclusive(&dev->driver_exclusive_resource, pos, nbytes)) {
> +		pci_warn_once(dev, "%s: Unexpected write to kernel-exclusive config offset %x",
> +			      current->comm, pos);
> +		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does truncating the 64-bit file offset to a
32-bit signed integer earlier in proc_bus_pci_write() allow bypassing this
new exclusive region check?

A large positive 64-bit file offset provided via pwrite64() gets truncated to
a negative value:

drivers/pci/proc.c:proc_bus_pci_write() {
    ...
	int pos = *ppos;
    ...
}

Because resource_is_exclusive() takes a u64 address, the negative pos will
sign-extend into a massive 64-bit value that won't match any exclusive
region, bypassing this detection entirely.

Additionally, this truncation also bypasses the subsequent bounds checks:

	if (pos >= size)
		return 0;
	if (nbytes >= size)
		nbytes = size;
	if (pos + nbytes > size)
		nbytes = size - pos;

The signed comparison if (pos >= size) evaluates to false for a negative
pos. Because nbytes is unsigned, the length clipping if (pos + nbytes > size)
evaluates using unsigned arithmetic and becomes true for a negative pos. This
erroneously increases nbytes up to massive out-of-bounds lengths, potentially
leading to large writes outside the PCI configuration space.

>  	if (pos >= size)
>  		return 0;
>  	if (nbytes >= size)

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