[PATCH v2 0/6] PCI: Fix procfs PCI config access issues

[PATCH v2 0/6] PCI: Fix procfs PCI config access issues

[Ziming Du]

Hi,

As suggested by Bjorn and Krzysztof, this series aligns the procfs PCI
configuration access implementation with its sysfs counterpart and fixes
issues found while reviewing the two paths.

The first four patches fix the offset truncation, inode size update race,
missing driver-exclusive resource warning, and unnecessary runtime PM
operations at the end of sysfs configuration space. The final two
patches align the procfs read and write implementations with their sysfs
counterparts.

Changes from v1:
  - Merge the read and write offset fixes and move them before the
    refactoring to avoid an intermediate signed/unsigned regression.
  - Serialize i_size_write() with the inode lock.
  - Add the driver-exclusive resource warning to procfs writes.
  - Avoid unnecessary runtime PM get/put when sysfs config access starts at EOF.

Link: https://lore.kernel.org/all/20260303193253.GA3817951@bhelgaas/
Link: https://lore.kernel.org/linux-pci/20260414024544.2975605-1-duziming2@huawei.com/
Link: https://sashiko.dev/#/patchset/20260414024544.2975605-1-duziming2%40huawei.com


Ziming Du (6):
  PCI: Prevent overflow in proc_bus_pci_{read,write}()
  PCI/proc: Serialize config inode size updates
  PCI/proc: Warn on writes to driver-exclusive config regions
  PCI/sysfs: Avoid runtime PM at config-space EOF
  PCI: Align proc_bus_pci_write() with pci_write_config()
  PCI: Align proc_bus_pci_read() with pci_read_config()

 drivers/pci/pci-sysfs.c |   4 +-
 drivers/pci/proc.c      | 124 +++++++++++++++++++++-------------------
 2 files changed, 68 insertions(+), 60 deletions(-)

-- 
2.43.0

[PATCH v2 1/6] PCI: Prevent overflow in proc_bus_pci_{read,write}()

[Ziming Du]

proc_bus_pci_read() and proc_bus_pci_write() receive the file position
as a 64-bit loff_t, but store it in 32-bit variables before validating
the configuration-space bounds.

In proc_bus_pci_read(), pos is an unsigned int, so an offset such as
0x100000000 is truncated to zero. The truncated value passes the bounds
check and causes the function to read from the beginning of PCI
configuration space instead of returning EOF.

In proc_bus_pci_write(), pos is an int, so an offset greater than
INT_MAX may become negative. The negative value bypasses the
pos >= size check and is then mixed with the unsigned nbytes value.
The bounds adjustment may consequently expand a small request into a
very large transfer, causing the write loop to use invalid negative
configuration offsets and run for an excessive amount of time.

Fix this by changing the type of pos to loff_t.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/proc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 71ad289fcb8e3..cad749924dd89 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -30,7 +30,7 @@ static ssize_t proc_bus_pci_read(struct file *file, char __user *buf,
 				 size_t nbytes, loff_t *ppos)
 {
 	struct pci_dev *dev = pde_data(file_inode(file));
-	unsigned int pos = *ppos;
+	loff_t pos = *ppos;
 	unsigned int cnt, size;
 
 	/*
@@ -114,7 +114,7 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
 {
 	struct inode *ino = file_inode(file);
 	struct pci_dev *dev = pde_data(ino);
-	int pos = *ppos;
+	loff_t pos = *ppos;
 	int size = dev->cfg_size;
 	int cnt, ret;
 
-- 
2.43.0

Re: [PATCH v2 1/6] PCI: Prevent overflow in proc_bus_pci_{read,write}()

[sashiko-bot]

> proc_bus_pci_read() and proc_bus_pci_write() receive the file position
> as a 64-bit loff_t, but store it in 32-bit variables before validating
> the configuration-space bounds.
> 
> In proc_bus_pci_read(), pos is an unsigned int, so an offset such as
> 0x100000000 is truncated to zero. The truncated value passes the bounds
> check and causes the function to read from the beginning of PCI
> configuration space instead of returning EOF.
> 
> In proc_bus_pci_write(), pos is an int, so an offset greater than
> INT_MAX may become negative. The negative value bypasses the
> pos >= size check and is then mixed with the unsigned nbytes value.
> The bounds adjustment may consequently expand a small request into a
> very large transfer, causing the write loop to use invalid negative
> configuration offsets and run for an excessive amount of time.
> 
> Fix this by changing the type of pos to loff_t.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
> Signed-off-by: Ziming Du <duziming2@huawei.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032438.1643450-1-duziming2@huawei.com?part=1

[PATCH v2 2/6] PCI/proc: Serialize config inode size updates

[Ziming Du]

proc_bus_pci_write() calls i_size_write() without serializing concurrent
writers. On 32-bit SMP systems, concurrent updates can corrupt
i_size_seqcount and make later i_size_read() calls spin indefinitely.

Protect the inode size update with the inode lock, as required by
i_size_write().

Fixes: ecb3908046ce ("pci: write file size to inode on proc bus file write")
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/proc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index cad749924dd89..ec2338c1e667e 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -181,7 +181,9 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
 	pci_config_pm_runtime_put(dev);
 
 	*ppos = pos;
+	inode_lock(ino);
 	i_size_write(ino, dev->cfg_size);
+	inode_unlock(ino);
 	return nbytes;
 }
 
-- 
2.43.0

Re: [PATCH v2 2/6] PCI/proc: Serialize config inode size updates

[sashiko-bot]

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

Pre-existing issues:
- [High] The return values of `__get_user()` and `__put_user()` are silently ignored, risking hardware configuration corruption and silent failures on page faults.
--

--- Patch [2]: [PATCH v2 2/6] PCI/proc: Serialize config inode size updates ---
commit 3aca0ed476ea2dd3079bd4be2a68be7793fdde7b
Author: Ziming Du <duziming2@huawei.com>

PCI/proc: Serialize config inode size updates

proc_bus_pci_write() calls i_size_write() without serializing concurrent
writers. On 32-bit SMP systems, concurrent updates can corrupt
i_size_seqcount and make later i_size_read() calls spin indefinitely.
This patch protects the inode size update with the inode lock.

> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index cad749924dd89..ec2338c1e667e 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -181,7 +181,9 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
>  	pci_config_pm_runtime_put(dev);
>  
>  	*ppos = pos;
> +	inode_lock(ino);
>  	i_size_write(ino, dev->cfg_size);

[Severity: High]
This isn't a bug introduced by this patch, but looking at
proc_bus_pci_write() earlier in this function, the return value of
__get_user() is silently ignored:

drivers/pci/proc.c:proc_bus_pci_write() {
	...
	if ((off & 1) && size) {
		unsigned char val;
		__get_user(val, buf);
		pci_user_write_config_byte(dev, off, val);
	...
}

Does this code fail to handle page faults correctly?

If a process calls write() with a buffer that fails memory access,
__get_user() sets the target variable to zero and returns -EFAULT.
Because this return value is ignored, the kernel proceeds to write these
uninitialized zeroes directly into the PCI device's configuration space.

Can this cause hardware misconfiguration, system lockups, or kernel panics?

Similarly, in proc_bus_pci_read(), the return value of __put_user() is
discarded:

drivers/pci/proc.c:proc_bus_pci_read() {
	...
		unsigned char val;
		pci_user_read_config_byte(dev, pos, &val);
		__put_user(val, buf);
	...
}

Could this lead to silent failures where data is not successfully copied to
userspace despite returning success, leaving the user program to consume
uninitialized memory?

Is it possible to check the return values of __get_user() and __put_user()
in these functions and return -EFAULT if they fail?

> +	inode_unlock(ino);
>  	return nbytes;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032438.1643450-1-duziming2@huawei.com?part=2

[PATCH v2 3/6] PCI/proc: Warn on writes to driver-exclusive config regions

[Ziming Du]

The sysfs configuration-space write path warns and taints the kernel
when userspace writes a region claimed exclusively by a driver. The
procfs path can access the same configuration space but does not perform
this check.

Add the same resource_is_exclusive() warning and taint to
proc_bus_pci_write() so the two userspace interfaces treat driver-owned
ranges consistently.

Fixes: 278294798ac9 ("PCI: Allow drivers to request exclusive config regions")
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/proc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index ec2338c1e667e..3f041765da978 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -122,6 +122,13 @@ 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 %llx",
+			      current->comm, pos);
+		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+	}
+
 	if (pos >= size)
 		return 0;
 	if (nbytes >= size)
-- 
2.43.0

Re: [PATCH v2 3/6] PCI/proc: Warn on writes to driver-exclusive config regions

[sashiko-bot]

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

New issues:
- [Low] The format string in pci_warn_once lacks a terminating newline character.

Pre-existing issues:
- [High] The return values of __get_user() are ignored in proc_bus_pci_write(), leading to silent hardware writes of zero or uninitialized data on fault.
- [High] proc_bus_pci_write() performs unaligned user memory accesses via __get_user(), violating API constraints and potentially triggering alignment faults.
--

--- Patch [3]: [PATCH v2 3/6] PCI/proc: Warn on writes to driver-exclusive config regions ---
commit 7011bc96a28549a7bd6d8b00360cce10cb406486
Author: Ziming Du <duziming2@huawei.com>

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

The sysfs configuration-space write path warns and taints the kernel
when userspace writes a region claimed exclusively by a driver. The
procfs path can access the same configuration space but does not perform
this check.

Add the same resource_is_exclusive() warning and taint to
proc_bus_pci_write() so the two userspace interfaces treat driver-owned
ranges consistently.

Fixes: 278294798ac9 ("PCI: Allow drivers to request exclusive config regions")

> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index ec2338c1e667e..3f041765da978 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -122,6 +122,13 @@ 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 %llx",
> +			      current->comm, pos);

[Severity: Low]
Is a trailing \n character needed for this format string?

Without it, subsequent kernel log messages from any subsystem might be
appended to the same line, resulting in garbled dmesg output.

> +		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> +	}
> +
>  	if (pos >= size)
>  		return 0;


drivers/pci/proc.c:proc_bus_pci_write() {
...
	if ((pos & 1) && cnt) {
		unsigned char val;
		__get_user(val, buf);
		pci_user_write_config_byte(dev, pos, val);
...
}

[Severity: High]
This is a pre-existing issue, but is the return value of __get_user() being
safely ignored here?

If __get_user() fails (e.g., due to an unmapped buffer or an unhandled page
fault), it returns -EFAULT.

By ignoring this return value, the kernel silently proceeds to write zero
(or potentially uninitialized stack data on older architectures) to the PCI
hardware configuration space instead of aborting.


drivers/pci/proc.c:proc_bus_pci_write() {
...
	if ((pos & 1) && cnt) {
		unsigned char val;
		__get_user(val, buf);
		pci_user_write_config_byte(dev, pos, val);
		buf++;
		pos++;
		cnt--;
	}

	if ((pos & 3) && cnt > 2) {
		__le16 val;
		__get_user(val, (__le16 __user *) buf);
...
}

[Severity: High]
This is also a pre-existing issue, but could this sequence lead to unaligned
user memory accesses?

If a privileged user issues a write with an odd offset (pos & 1) and
length > 2, the code reads the first byte and increments buf by 1.

This makes the pointer unaligned before casting it to (__le16 __user *) for
the next __get_user() call.

Passing an unaligned pointer can trigger a kernel oops on strict-alignment
architectures that lack unaligned kernel access fixups, or it may return a
silent -EFAULT.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032438.1643450-1-duziming2@huawei.com?part=3

[PATCH v2 4/6] PCI/sysfs: Avoid runtime PM at config-space EOF

[Ziming Du]

pci_read_config() and pci_write_config() only reject offsets beyond the
end of configuration space. An offset exactly at the end is reduced to a
zero-length transfer after the bounds adjustment, but still takes and
drops a runtime PM reference despite doing no work.

Return immediately for offsets at the end of configuration space.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Krzysztof Wilczyński <kw@linux.com>
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/pci-sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5ec0b245a69bd..a4f6559c619c7 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -718,7 +718,7 @@ static ssize_t pci_read_config(struct file *filp, struct kobject *kobj,
 	else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
 		size = 128;
 
-	if (off > size)
+	if (off >= size)
 		return 0;
 	if (off + count > size) {
 		size -= off;
@@ -799,7 +799,7 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
 		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
 	}
 
-	if (off > dev->cfg_size)
+	if (off >= dev->cfg_size)
 		return 0;
 	if (off + count > dev->cfg_size) {
 		size = dev->cfg_size - off;
-- 
2.43.0

Re: [PATCH v2 4/6] PCI/sysfs: Avoid runtime PM at config-space EOF

[sashiko-bot]

> pci_read_config() and pci_write_config() only reject offsets beyond the
> end of configuration space. An offset exactly at the end is reduced to a
> zero-length transfer after the bounds adjustment, but still takes and
> drops a runtime PM reference despite doing no work.
> 
> Return immediately for offsets at the end of configuration space.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Suggested-by: Krzysztof Wilczyński <kw@linux.com>
> Signed-off-by: Ziming Du <duziming2@huawei.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032438.1643450-1-duziming2@huawei.com?part=4

[PATCH v2 5/6] PCI: Align proc_bus_pci_write() with pci_write_config()

[Ziming Du]

proc_bus_pci_write() and pci_write_config() implement essentially the
same functionality. To improve consistency across the PCI subsystem,
align the implementation in procfs with the sysfs counterpart.

Specifically:
  - Use dev->cfg_size directly instead of assigning it to 'size'.
  - Rename the variable 'pos' to 'off' and replace 'cnt' with 'size'.
  - Remove the redundant bounds check `if (nbytes >= size)`.
  - Use the same transfer loop conditions as pci_write_config().

No functional change intended.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/proc.c | 61 +++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 3f041765da978..71d1f34e13a7d 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -114,80 +114,79 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
 {
 	struct inode *ino = file_inode(file);
 	struct pci_dev *dev = pde_data(ino);
-	loff_t pos = *ppos;
-	int size = dev->cfg_size;
-	int cnt, ret;
+	loff_t off = *ppos;
+	unsigned int size = nbytes;
+	int ret;
 
 	ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
 	if (ret)
 		return ret;
 
-	if (resource_is_exclusive(&dev->driver_exclusive_resource, pos,
+	if (resource_is_exclusive(&dev->driver_exclusive_resource, off,
 				  nbytes)) {
 		pci_warn_once(dev, "%s: Unexpected write to kernel-exclusive config offset %llx",
-			      current->comm, pos);
+			      current->comm, off);
 		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
 	}
 
-	if (pos >= size)
+	if (off >= dev->cfg_size)
 		return 0;
-	if (nbytes >= size)
+	if (off + size > dev->cfg_size) {
+		size = dev->cfg_size - off;
 		nbytes = size;
-	if (pos + nbytes > size)
-		nbytes = size - pos;
-	cnt = nbytes;
+	}
 
-	if (!access_ok(buf, cnt))
+	if (!access_ok(buf, size))
 		return -EINVAL;
 
 	pci_config_pm_runtime_get(dev);
 
-	if ((pos & 1) && cnt) {
+	if ((off & 1) && size) {
 		unsigned char val;
 		__get_user(val, buf);
-		pci_user_write_config_byte(dev, pos, val);
+		pci_user_write_config_byte(dev, off, val);
 		buf++;
-		pos++;
-		cnt--;
+		off++;
+		size--;
 	}
 
-	if ((pos & 3) && cnt > 2) {
+	if ((off & 3) && size > 2) {
 		__le16 val;
 		__get_user(val, (__le16 __user *) buf);
-		pci_user_write_config_word(dev, pos, le16_to_cpu(val));
+		pci_user_write_config_word(dev, off, le16_to_cpu(val));
 		buf += 2;
-		pos += 2;
-		cnt -= 2;
+		off += 2;
+		size -= 2;
 	}
 
-	while (cnt >= 4) {
+	while (size > 3) {
 		__le32 val;
 		__get_user(val, (__le32 __user *) buf);
-		pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
+		pci_user_write_config_dword(dev, off, le32_to_cpu(val));
 		buf += 4;
-		pos += 4;
-		cnt -= 4;
+		off += 4;
+		size -= 4;
 	}
 
-	if (cnt >= 2) {
+	if (size >= 2) {
 		__le16 val;
 		__get_user(val, (__le16 __user *) buf);
-		pci_user_write_config_word(dev, pos, le16_to_cpu(val));
+		pci_user_write_config_word(dev, off, le16_to_cpu(val));
 		buf += 2;
-		pos += 2;
-		cnt -= 2;
+		off += 2;
+		size -= 2;
 	}
 
-	if (cnt) {
+	if (size) {
 		unsigned char val;
 		__get_user(val, buf);
-		pci_user_write_config_byte(dev, pos, val);
-		pos++;
+		pci_user_write_config_byte(dev, off, val);
+		off++;
 	}
 
 	pci_config_pm_runtime_put(dev);
 
-	*ppos = pos;
+	*ppos = off;
 	inode_lock(ino);
 	i_size_write(ino, dev->cfg_size);
 	inode_unlock(ino);
-- 
2.43.0

Re: [PATCH v2 5/6] PCI: Align proc_bus_pci_write() with pci_write_config()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing error handling for `__get_user()` causes silent hardware configuration corruption.
- [High] Unaligned user pointer dereferences cause alignment faults and silent failures on strict architectures.
--

--- Patch [5]: [PATCH v2 5/6] PCI: Align proc_bus_pci_write() with pci_write_config() ---
commit 9ab25f02e0980d093d99dfa257a71fcb18afa9ca
Author: Ziming Du <duziming2@huawei.com>

PCI: Align proc_bus_pci_write() with pci_write_config()

This commit aligns proc_bus_pci_write() with pci_write_config() to improve
consistency across the PCI subsystem. It updates variable names, simplifies
bounds checking, and modifies transfer loop conditions.

> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index 3f041765da978..71d1f34e13a7d 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
[ ... ]
> @@ -114,80 +114,79 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
[ ... ]
> -	if ((pos & 3) && cnt > 2) {
> +	if ((off & 3) && size > 2) {
>  		__le16 val;
>  		__get_user(val, (__le16 __user *) buf);

[Severity: High]
This isn't a bug introduced by this patch, but can this cast to a multi-byte
type result in an unaligned user pointer dereference?

If a user calls pwrite() with a buffer whose alignment phase doesn't match
the file offset (e.g., a page-aligned buffer with offset=1), synchronizing
the buf and off increments guarantees that when off reaches a 16-bit or
32-bit boundary, buf will be unaligned.

On architectures with strict alignment requirements, could this trigger
alignment faults in kernel space, causing __get_user() to abort and return
-EFAULT?

> -		pci_user_write_config_word(dev, pos, le16_to_cpu(val));
> +		pci_user_write_config_word(dev, off, le16_to_cpu(val));
>  		buf += 2;
> -		pos += 2;
> -		cnt -= 2;
> +		off += 2;
> +		size -= 2;
>  	}
>  
> -	while (cnt >= 4) {
> +	while (size > 3) {
>  		__le32 val;
>  		__get_user(val, (__le32 __user *) buf);
> -		pci_user_write_config_dword(dev, pos, le32_to_cpu(val));

[Severity: High]
This is a pre-existing issue, but does this code ignore the return value of
__get_user() before executing the hardware write?

If a user passes a buffer that spans into unmapped memory, __get_user()
will encounter a page fault, return -EFAULT, and set the output value to
zero. Because proc_bus_pci_write() completely ignores this return value,
could it proceed to silently write these zeros to critical PCI configuration
space registers (like BARs or the Command register) and potentially lock up
the bus or crash the system?

> +		pci_user_write_config_dword(dev, off, le32_to_cpu(val));
>  		buf += 4;
> -		pos += 4;
> -		cnt -= 4;
> +		off += 4;
> +		size -= 4;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032438.1643450-1-duziming2@huawei.com?part=5

[PATCH v2 6/6] PCI: Align proc_bus_pci_read() with pci_read_config()

[Ziming Du]

proc_bus_pci_read() and pci_read_config() implement essentially the
same functionality. To improve consistency across the PCI subsystem,
align the implementation in procfs with the sysfs counterpart.

Specifically:
  - Rename the variable 'pos' to 'off' and replace 'cnt' with 'count'.
  - Remove the redundant bounds check `if (nbytes >= size)`.
  - Use the same transfer loop conditions as pci_read_config().

No functional change intended.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/proc.c | 58 +++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 71d1f34e13a7d..13ac92da4624b 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -30,8 +30,9 @@ static ssize_t proc_bus_pci_read(struct file *file, char __user *buf,
 				 size_t nbytes, loff_t *ppos)
 {
 	struct pci_dev *dev = pde_data(file_inode(file));
-	loff_t pos = *ppos;
-	unsigned int cnt, size;
+	loff_t off = *ppos;
+	unsigned int count = nbytes;
+	unsigned int size;
 
 	/*
 	 * Normal users can read only the standardized portion of the
@@ -46,66 +47,65 @@ static ssize_t proc_bus_pci_read(struct file *file, char __user *buf,
 	else
 		size = 64;
 
-	if (pos >= size)
+	if (off >= size)
 		return 0;
-	if (nbytes >= size)
-		nbytes = size;
-	if (pos + nbytes > size)
-		nbytes = size - pos;
-	cnt = nbytes;
+	if (off + count > size) {
+		count = size - off;
+		nbytes = count;
+	}
 
-	if (!access_ok(buf, cnt))
+	if (!access_ok(buf, count))
 		return -EINVAL;
 
 	pci_config_pm_runtime_get(dev);
 
-	if ((pos & 1) && cnt) {
+	if ((off & 1) && count) {
 		unsigned char val;
-		pci_user_read_config_byte(dev, pos, &val);
+		pci_user_read_config_byte(dev, off, &val);
 		__put_user(val, buf);
 		buf++;
-		pos++;
-		cnt--;
+		off++;
+		count--;
 	}
 
-	if ((pos & 3) && cnt > 2) {
+	if ((off & 3) && count > 2) {
 		unsigned short val;
-		pci_user_read_config_word(dev, pos, &val);
+		pci_user_read_config_word(dev, off, &val);
 		__put_user(cpu_to_le16(val), (__le16 __user *) buf);
 		buf += 2;
-		pos += 2;
-		cnt -= 2;
+		off += 2;
+		count -= 2;
 	}
 
-	while (cnt >= 4) {
+	while (count > 3) {
 		unsigned int val;
-		pci_user_read_config_dword(dev, pos, &val);
+		pci_user_read_config_dword(dev, off, &val);
 		__put_user(cpu_to_le32(val), (__le32 __user *) buf);
 		buf += 4;
-		pos += 4;
-		cnt -= 4;
+		off += 4;
+		count -= 4;
 		cond_resched();
 	}
 
-	if (cnt >= 2) {
+	if (count >= 2) {
 		unsigned short val;
-		pci_user_read_config_word(dev, pos, &val);
+		pci_user_read_config_word(dev, off, &val);
 		__put_user(cpu_to_le16(val), (__le16 __user *) buf);
 		buf += 2;
-		pos += 2;
-		cnt -= 2;
+		off += 2;
+		count -= 2;
 	}
 
-	if (cnt) {
+	if (count > 0) {
 		unsigned char val;
-		pci_user_read_config_byte(dev, pos, &val);
+		pci_user_read_config_byte(dev, off, &val);
 		__put_user(val, buf);
-		pos++;
+		off++;
 	}
 
 	pci_config_pm_runtime_put(dev);
 
-	*ppos = pos;
+	*ppos = off;
 	return nbytes;
 }
 
-- 
2.43.0

Re: [PATCH v2 6/6] PCI: Align proc_bus_pci_read() with pci_read_config()

[sashiko-bot]

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

Pre-existing issues:
- [High] Unaligned pointer dereference via `__put_user` cast.
--

--- Patch [6]: [PATCH v2 6/6] PCI: Align proc_bus_pci_read() with pci_read_config() ---
commit 214117562117fd4692f3111d71550ff1bd6ab306
Author: Ziming Du <duziming2@huawei.com>

PCI: Align proc_bus_pci_read() with pci_read_config()

This commit aligns the implementation of proc_bus_pci_read() with
pci_read_config() to improve consistency. It renames variables and updates
the transfer loop conditions without intending functional changes.

> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index 71d1f34e13a7d..13ac92da4624b 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
[ ... ]
> @@ -46,66 +47,65 @@ static ssize_t proc_bus_pci_read(struct file *file, char __user *buf,
[ ... ]
> -	while (cnt >= 4) {
> +	while (count > 3) {
>  		unsigned int val;
> -		pci_user_read_config_dword(dev, pos, &val);
> +		pci_user_read_config_dword(dev, off, &val);
>  		__put_user(cpu_to_le32(val), (__le32 __user *) buf);

[Severity: High]
This is a pre-existing issue, but could this cast and dereference trigger
an alignment exception on strict-alignment architectures?

If a normal user opens a PCI config file and calls read() with an aligned
file offset but an unaligned buffer address, the read chunks are aligned
based on the file offset, bypassing the earlier byte and word branches.
The execution would then reach this 32-bit __put_user cast with the
unaligned user buffer.

On architectures that do not emulate unaligned kernel-mode accesses like
older ARM or SPARC, this might result in a local denial of service via a
kernel oops or panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730032438.1643450-1-duziming2@huawei.com?part=6