linux-kernel - Re: [PATCH v2 2/3] PCI: Prevent overflow in proc_bus_pci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251229180742.GA69587@bhelgaas>
Date: Mon, 29 Dec 2025 12:07:42 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ziming Du <duziming2@...wei.com>
Cc: bhelgaas@...gle.com, jbarnes@...tuousgeek.org, chrisw@...hat.com,
	alex.williamson@...hat.com, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, liuyongqiang13@...wei.com,
	Krzysztof Wilczyński <kwilczynski@...nel.org>
Subject: Re: [PATCH v2 2/3] PCI: Prevent overflow in proc_bus_pci_write()

[+cc Krzysztof; I thought we looked at this long ago?]

On Wed, Dec 24, 2025 at 05:27:18PM +0800, Ziming Du wrote:
> From: Yongqiang Liu <liuyongqiang13@...wei.com>
> 
> When the value of ppos over the INT_MAX, the pos is over set to a negtive
> value which will be passed to get_user() or pci_user_write_config_dword().
> Unexpected behavior such as a softlock will happen as follows:

s/negtive/negative/
s/softlock/soft lockup/ to match message below

s/ppos/pos/ (or fix this to refer to "*ppos", which I think is what
you're referring to)

I guess the point is that proc_bus_pci_write() takes a "loff_t *ppos",
loff_t is a signed type, and negative read/write offsets are invalid.

If this is easily reproducible with "dd" or similar, could maybe
include a sample command line?

>  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 add check for the pos.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Yongqiang Liu <liuyongqiang13@...wei.com>
> Signed-off-by: Ziming Du <duziming2@...wei.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..200d42feafd8 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 >= size || pos < 0)
>  		return 0;

I see a few similar cases that look like this; maybe we should do the
same?

  if (pos < 0)
    return -EINVAL;

Looks like proc_bus_pci_read() has the same issue?

What about pci_read_config(), pci_write_config(),
pci_llseek_resource(), pci_read_legacy_io(), pci_write_legacy_io(),
pci_read_resource_io(), pci_write_resource_io(), pci_read_rom()?
These are all sysfs things; does the sysfs infrastructure take care of
negative offsets before we get to these?

>  	if (nbytes >= size)
>  		nbytes = size;
> -- 
> 2.43.0
>