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

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

[Ziming Du]

Hi,

As suggested by Bjorn, this series aligns the implementation of procfs with
the sysfs counterpart and fixes procfs PCI configuration access issues.

The first two patches refactor proc_bus_pci_{write,read}() to align
with pci_write_config() and pci_read_config() respectively:
  - Rename variables (pos->off, cnt->count/size) for consistency
  - Remove rebundant bounds check

The last two patches fix potential overflow issues:
  - Prevent overflow when offset exceeds reasonable range
  - Fix implicit 64-bit to 32-bit truncation in read path

Link: https://lore.kernel.org/all/20260303193253.GA3817951@bhelgaas/

Ziming Du (4):
  PCI: Align proc_bus_pci_write() with pci_write_config()
  PCI: Align proc_bus_pci_read() with pci_read_config()
  PCI: Prevent overflow in proc_bus_pci_write()
  PCI: Prevent overflow in proc_bus_pci_read()

 drivers/pci/proc.c | 115 ++++++++++++++++++++++-----------------------
 1 file changed, 57 insertions(+), 58 deletions(-)

-- 
2.43.0

[PATCH 1/4] 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 insted of assigning it to 'size'.
  - Rename the variable 'pos' to 'off' and replace 'cnt' with 'size'.
  - Remove the redundant bounds check `if (nbytes >= size)`.

No functional change intended.

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

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index ce36e35681e8..ff5e4980c99c 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -113,73 +113,72 @@ 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;
-	int size = dev->cfg_size;
-	int cnt, ret;
+	int off = *ppos;
+	unsigned int size = nbytes;
+	int ret;
 
 	ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
 	if (ret)
 		return ret;
 
-	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 >= 4) {
 		__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;
 	i_size_write(ino, dev->cfg_size);
 	return nbytes;
 }
-- 
2.43.0

Re: [PATCH 1/4] PCI: Align proc_bus_pci_write() with pci_write_config()

[Krzysztof Wilczyński]

Hello,

> -	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;
> +	}

Some of these boundary checks have issues on the sysfs side, where for some
offsets, a gratuitous pci_config_pm_runtime_get()/put() invocation would take
place, even though there would be no real work done.  Something to fix, too,
since you are looking to align both implementations.

> -	if ((pos & 1) && cnt) {
> +	if ((off & 1) && size) {
>  		unsigned char val;
>  		__get_user(val, buf);

Sashiko is complaining about __get_user() here.  However, the following
patch was sent recently, which aims to fix this (seems Syzkaller found
this, too):

  https://lore.kernel.org/linux-pci/20260502011446.125268-1-kartikey406@gmail.com/

If this one is accepted, then there is nothing to do here.  Otherwise, you
could consider moving to get_user(), etc., here, too.

> -	while (cnt >= 4) {
> +	while (size >= 4) {

This can still be aligned between here and sysfs.  Have a look there.

There is also the resource_is_exclusive() check on the sysfs side which
adds a warning and a taint when write is accessing some address ranges
which some driver claimed for itself.  procfs does not have this at the
moment (not sure if on purpose or it simly wasn't ever added).

Thank you!

	Krzysztof

[PATCH 2/4] 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)`.

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 ff5e4980c99c..6524280bc903 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -29,8 +29,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));
-	unsigned int pos = *ppos;
-	unsigned int cnt, size;
+	unsigned int off = *ppos;
+	unsigned int count = nbytes;
+	unsigned int size;
 
 	/*
 	 * Normal users can read only the standardized portion of the
@@ -45,66 +46,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 >= 4) {
 		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) {
 		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

[PATCH 3/4] PCI: Prevent overflow in proc_bus_pci_write()

[Ziming Du]

When the value of *ppos over the INT_MAX, the variable off is over
set to a negative value which will be passed to get_user() or
pci_user_write_config_dword(). Unexpected behavior such as a soft lockup
will happen as follows:

 watchdog: BUG: soft lockup - CPU#0 stuck for 130s! [syz.3.109:3444]
 RIP: 0010:_raw_spin_unlock_irq+0x17/0x30
 Call Trace:
  <TASK>
  pci_user_write_config_dword+0x126/0x1f0
  proc_bus_pci_write+0x273/0x470
  proc_reg_write+0x1b6/0x280
  do_iter_write+0x48e/0x790
  vfs_writev+0x125/0x4a0
  __x64_sys_pwritev+0x1e2/0x2a0
  do_syscall_64+0x59/0x110
  entry_SYSCALL_64_after_hwframe+0x78/0xe2

Fix this by changing the type of off to loff_t.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 6524280bc903..a5db7d23353a 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -113,7 +113,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 off = *ppos;
+	loff_t off = *ppos;
 	unsigned int size = nbytes;
 	int ret;
 
-- 
2.43.0

[PATCH 4/4] PCI: Prevent overflow in proc_bus_pci_read()

[Ziming Du]

proc_bus_pci_read() assigns the 64-bit loff_t *ppos directly to a 32-bit
integer variable. For large offsets, such as 0x100000000, this
implicit conversion could truncate the value and incorrectly consider the
offset as valid.

Fix this by changing the type of off to loff_t.

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

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index a5db7d23353a..f11f48becf52 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -29,7 +29,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 off = *ppos;
+	loff_t off = *ppos;
 	unsigned int count = nbytes;
 	unsigned int size;
 
-- 
2.43.0

Re: [PATCH 4/4] PCI: Prevent overflow in proc_bus_pci_read()

[Krzysztof Wilczyński]

Hello,

> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index a5db7d23353a..f11f48becf52 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -29,7 +29,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 off = *ppos;
> +	loff_t off = *ppos;
>  	unsigned int count = nbytes;
>  	unsigned int size;

You could merge this patch with the previews one.  This will close the gap
on the signed/unsigned promotion issue that Sashiko is complaining about.

Thank you!

	Krzysztof

[PING] PCI: Fix procfs PCI config access issues

[duziming]

在 2026/4/14 10:45, Ziming Du 写道:
> Hi,
>
> As suggested by Bjorn, this series aligns the implementation of procfs with
> the sysfs counterpart and fixes procfs PCI configuration access issues.
>
> The first two patches refactor proc_bus_pci_{write,read}() to align
> with pci_write_config() and pci_read_config() respectively:
>    - Rename variables (pos->off, cnt->count/size) for consistency
>    - Remove rebundant bounds check
>
> The last two patches fix potential overflow issues:
>    - Prevent overflow when offset exceeds reasonable range
>    - Fix implicit 64-bit to 32-bit truncation in read path
>
> Link: https://lore.kernel.org/all/20260303193253.GA3817951@bhelgaas/
>
> Ziming Du (4):
>    PCI: Align proc_bus_pci_write() with pci_write_config()
>    PCI: Align proc_bus_pci_read() with pci_read_config()
>    PCI: Prevent overflow in proc_bus_pci_write()
>    PCI: Prevent overflow in proc_bus_pci_read()
>
>   drivers/pci/proc.c | 115 ++++++++++++++++++++++-----------------------
>   1 file changed, 57 insertions(+), 58 deletions(-)
Gentle ping on this

Re: [PATCH 0/4] PCI: Fix procfs PCI config access issues

[Bjorn Helgaas]

On Tue, Apr 14, 2026 at 10:45:40AM +0800, Ziming Du wrote:
> Hi,
> 
> As suggested by Bjorn, this series aligns the implementation of procfs with
> the sysfs counterpart and fixes procfs PCI configuration access issues.
> 
> The first two patches refactor proc_bus_pci_{write,read}() to align
> with pci_write_config() and pci_read_config() respectively:
>   - Rename variables (pos->off, cnt->count/size) for consistency
>   - Remove rebundant bounds check
> 
> The last two patches fix potential overflow issues:
>   - Prevent overflow when offset exceeds reasonable range
>   - Fix implicit 64-bit to 32-bit truncation in read path
> 
> Link: https://lore.kernel.org/all/20260303193253.GA3817951@bhelgaas/
> 
> Ziming Du (4):
>   PCI: Align proc_bus_pci_write() with pci_write_config()
>   PCI: Align proc_bus_pci_read() with pci_read_config()
>   PCI: Prevent overflow in proc_bus_pci_write()
>   PCI: Prevent overflow in proc_bus_pci_read()
> 
>  drivers/pci/proc.c | 115 ++++++++++++++++++++++-----------------------
>  1 file changed, 57 insertions(+), 58 deletions(-)

Can you take a look at this:
https://sashiko.dev/#/patchset/20260414024544.2975605-1-duziming2%40huawei.com
and see whether there's anything we should do?

From a quick look, there might be a temporary regression and possibly
a pre-existing proc_bus_pci_write() issue.