1. Patchew
  2. linux
  3. View series

[PATCH] PCI: Fix the int overflow in proc_bus_pci_write()

Wang ShaoBo posted 1 patch 4 days, 5 hours ago
There is a newer version of this series
drivers/pci/proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] PCI: Fix the int overflow in proc_bus_pci_write()
Posted by Wang ShaoBo 4 days, 5 hours ago 
Following testcase can trigger a softlockup BUG.
syscall(__NR_pwritev, /*fd=*/..., /*vec=*/..., /*vlen=*/...,
        /*pos_l=*/0x80010000, /*pos_h=*/0x100);

watchdog: BUG: soft lockup - CPU#11 stuck for 22s! [test:537]
Modules linked in:
CPU: 11 PID: 537 Comm: test Not tainted 5.10.0+ #67
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:pci_user_write_config_dword+0x67/0xc0
Code: 00 00 44 89 e2 48 8b 87 e0 00 00 00 48 8b 40 20 e8 9e 54 7e 00 48 c7 c7 20 48 a2 83 41 89 c0 c6 07 00 0f 1f 40 00 fb 45 85 c0 <7e> 12 41 8d 80 7f ff ff ff 41 b8 de ff ff ff 83 f8 08 76 0c 5b 44
RSP: 0018:ffffc900016c3d30 EFLAGS: 00000246
RAX: 0000000000000000 RBX: ffff888042058000 RCX: 0000000000000005
RDX: ffff888004058a00 RSI: 0000000000000046 RDI: ffffffff83a24820
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
R10: ffff888005c25900 R11: 0000000000000000 R12: 0000000080c48680
R13: 0000000020c38684 R14: 0000000080010000 R15: ffff888004702408
FS:  000000003ae91880(0000) GS:ffff88801f380000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020c00000 CR3: 0000000006f2c000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 proc_bus_pci_write+0x22c/0x260
 proc_reg_write+0x40/0x90
 do_loop_readv_writev.part.0+0x97/0xc0
 do_iter_write+0xf6/0x150
 vfs_writev+0x97/0x130
 ? files_cgroup_alloc_fd+0x5c/0x70
 ? do_sys_openat2+0x1c9/0x320
 __x64_sys_pwritev+0xb1/0x100
 do_syscall_64+0x2b/0x40
 entry_SYSCALL_64_after_hwframe+0x6c/0xd6

The pos_l parameter for pwritev syscall may be an integer negative value,
which will make the variable pos in proc_bus_pci_write() negative and
variable cnt a very large number.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Wang ShaoBo <bobo.shaobowang@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 9348a0fb8084..ef7a33affb3b 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -121,7 +121,7 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
 	if (ret)
 		return ret;
 
-	if (pos >= size)
+	if (pos < 0 || pos >= size)
 		return 0;
 	if (nbytes >= size)
 		nbytes = size;
-- 
2.25.1
Re: [PATCH] PCI: Fix the int overflow in proc_bus_pci_write()
Posted by Bjorn Helgaas 3 days, 19 hours ago 
[+cc linux-hardening]

On Fri, Sep 05, 2025 at 06:47:30PM +0800, Wang ShaoBo wrote:
> Following testcase can trigger a softlockup BUG.
> syscall(__NR_pwritev, /*fd=*/..., /*vec=*/..., /*vlen=*/...,
>         /*pos_l=*/0x80010000, /*pos_h=*/0x100);
> 
> watchdog: BUG: soft lockup - CPU#11 stuck for 22s! [test:537]
> Modules linked in:
> CPU: 11 PID: 537 Comm: test Not tainted 5.10.0+ #67
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:pci_user_write_config_dword+0x67/0xc0
> Code: 00 00 44 89 e2 48 8b 87 e0 00 00 00 48 8b 40 20 e8 9e 54 7e 00 48 c7 c7 20 48 a2 83 41 89 c0 c6 07 00 0f 1f 40 00 fb 45 85 c0 <7e> 12 41 8d 80 7f ff ff ff 41 b8 de ff ff ff 83 f8 08 76 0c 5b 44
> RSP: 0018:ffffc900016c3d30 EFLAGS: 00000246
> RAX: 0000000000000000 RBX: ffff888042058000 RCX: 0000000000000005
> RDX: ffff888004058a00 RSI: 0000000000000046 RDI: ffffffff83a24820
> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
> R10: ffff888005c25900 R11: 0000000000000000 R12: 0000000080c48680
> R13: 0000000020c38684 R14: 0000000080010000 R15: ffff888004702408
> FS:  000000003ae91880(0000) GS:ffff88801f380000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020c00000 CR3: 0000000006f2c000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  proc_bus_pci_write+0x22c/0x260
>  proc_reg_write+0x40/0x90
>  do_loop_readv_writev.part.0+0x97/0xc0
>  do_iter_write+0xf6/0x150
>  vfs_writev+0x97/0x130
>  ? files_cgroup_alloc_fd+0x5c/0x70
>  ? do_sys_openat2+0x1c9/0x320
>  __x64_sys_pwritev+0xb1/0x100
>  do_syscall_64+0x2b/0x40
>  entry_SYSCALL_64_after_hwframe+0x6c/0xd6
> 
> The pos_l parameter for pwritev syscall may be an integer negative value,
> which will make the variable pos in proc_bus_pci_write() negative and
> variable cnt a very large number.

Sounds like a problem; have you looked for similar problems in other
.proc_write() and .proc_read() functions?  validate_flash_write() is
one that also looks suspicious to me.

I think you're describing this code:

  static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
				    size_t nbytes, loff_t *ppos)
  {
        int pos = *ppos;
        int size = dev->cfg_size;
        int cnt, ret;

        if (pos + nbytes > size)
                nbytes = size - pos;
        cnt = nbytes;
	...
	while (cnt >= 4) {
		...
                pos += 4;
                cnt -= 4;
	}

proc_bus_pci_read() is quite similar but "pos", "cnt", and "size" are
unsigned:

  static ssize_t proc_bus_pci_read(struct file *file, char __user *buf,
				   size_t nbytes, loff_t *ppos)
  {
        unsigned int pos = *ppos;
        unsigned int cnt, size;

It seems like they should use the same strategy to avoid this problem.

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wang ShaoBo <bobo.shaobowang@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 9348a0fb8084..ef7a33affb3b 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -121,7 +121,7 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
>  	if (ret)
>  		return ret;
>  
> -	if (pos >= size)
> +	if (pos < 0 || pos >= size)
>  		return 0;
>  	if (nbytes >= size)
>  		nbytes = size;
> -- 
> 2.25.1
>
答复: [PATCH] PCI: Fix the int overflow in proc_bus_pci_write()
Posted by Wangshaobo (bobo) 1 day, 3 hours ago 

> -----邮件原件-----
> 发件人: Bjorn Helgaas [mailto:helgaas@kernel.org]
> 发送时间: 2025年9月6日 4:49
> 收件人: Wangshaobo (bobo) <bobo.shaobowang@huawei.com>
> 抄送: linux-pci@vger.kernel.org; bhelgaas@google.com; leijitang
> <leijitang@huawei.com>; linux-kernel@vger.kernel.org;
> akpm@linux-foundation.org; christian.brauner@ubuntu.com;
> linux-hardening@vger.kernel.org
> 主题: Re: [PATCH] PCI: Fix the int overflow in proc_bus_pci_write()
> 
> [+cc linux-hardening]
> 
> On Fri, Sep 05, 2025 at 06:47:30PM +0800, Wang ShaoBo wrote:
> > Following testcase can trigger a softlockup BUG.
> > syscall(__NR_pwritev, /*fd=*/..., /*vec=*/..., /*vlen=*/...,
> >         /*pos_l=*/0x80010000, /*pos_h=*/0x100);
> >
> > watchdog: BUG: soft lockup - CPU#11 stuck for 22s! [test:537] Modules
> > linked in:
> > CPU: 11 PID: 537 Comm: test Not tainted 5.10.0+ #67 Hardware name:
> > QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1
> > 04/01/2014
> > RIP: 0010:pci_user_write_config_dword+0x67/0xc0
> > Code: 00 00 44 89 e2 48 8b 87 e0 00 00 00 48 8b 40 20 e8 9e 54 7e 00
> > 48 c7 c7 20 48 a2 83 41 89 c0 c6 07 00 0f 1f 40 00 fb 45 85 c0 <7e> 12
> > 41 8d 80 7f ff ff ff 41 b8 de ff ff ff 83 f8 08 76 0c 5b 44
> > RSP: 0018:ffffc900016c3d30 EFLAGS: 00000246
> > RAX: 0000000000000000 RBX: ffff888042058000 RCX:
> 0000000000000005
> > RDX: ffff888004058a00 RSI: 0000000000000046 RDI: ffffffff83a24820
> > RBP: 0000000000000000 R08: 0000000000000000 R09:
> 0000000000000001
> > R10: ffff888005c25900 R11: 0000000000000000 R12:
> 0000000080c48680
> > R13: 0000000020c38684 R14: 0000000080010000 R15:
> ffff888004702408
> > FS:  000000003ae91880(0000) GS:ffff88801f380000(0000)
> > knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000020c00000 CR3: 0000000006f2c000 CR4:
> 00000000000006e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400 Call
> > Trace:
> >  proc_bus_pci_write+0x22c/0x260
> >  proc_reg_write+0x40/0x90
> >  do_loop_readv_writev.part.0+0x97/0xc0
> >  do_iter_write+0xf6/0x150
> >  vfs_writev+0x97/0x130
> >  ? files_cgroup_alloc_fd+0x5c/0x70
> >  ? do_sys_openat2+0x1c9/0x320
> >  __x64_sys_pwritev+0xb1/0x100
> >  do_syscall_64+0x2b/0x40
> >  entry_SYSCALL_64_after_hwframe+0x6c/0xd6
> >
> > The pos_l parameter for pwritev syscall may be an integer negative
> > value, which will make the variable pos in proc_bus_pci_write()
> > negative and variable cnt a very large number.
> 
> Sounds like a problem; have you looked for similar problems in other
> .proc_write() and .proc_read() functions?  validate_flash_write() is one
> that also looks suspicious to me.
> 
> I think you're describing this code:
> 
>   static ssize_t proc_bus_pci_write(struct file *file, const char __user
> *buf,
> 				    size_t nbytes, loff_t *ppos)
>   {
>         int pos = *ppos;
>         int size = dev->cfg_size;
>         int cnt, ret;
> 
>         if (pos + nbytes > size)
>                 nbytes = size - pos;
>         cnt = nbytes;
> 	...
> 	while (cnt >= 4) {
> 		...
>                 pos += 4;
>                 cnt -= 4;
> 	}
> 
> proc_bus_pci_read() is quite similar but "pos", "cnt", and "size" are
> unsigned:
> 
>   static ssize_t proc_bus_pci_read(struct file *file, char __user *buf,
> 				   size_t nbytes, loff_t *ppos)
>   {
>         unsigned int pos = *ppos;
>         unsigned int cnt, size;
> 
> It seems like they should use the same strategy to avoid this problem.
> 

Thanks, it's better to use unsigned int to to avoid this problem.

- Wang ShaoBo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.