public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Miscellaneous fixes for pci subsystem
@ 2025-12-16  8:39 Ziming Du
  2025-12-16  8:39 ` [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug Ziming Du
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Ziming Du @ 2025-12-16  8:39 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, chrisw, jbarnes, alex.williamson,
	liuyongqiang13, duziming2

Hello!

This series contains miscellaneous fixes for pci subsystem:

Yongqiang Liu (2):
  PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  PCI: Prevent overflow in proc_bus_pci_write()

Ziming Du (1):
  PCI/sysfs: fix null pointer dereference during PCI hotplug

							Thanx, Du

 drivers/pci/pci-sysfs.c | 14 ++++++++++++++
 drivers/pci/proc.c      |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug
  2025-12-16  8:39 [PATCH 0/3] Miscellaneous fixes for pci subsystem Ziming Du
@ 2025-12-16  8:39 ` Ziming Du
  2025-12-23 16:55   ` Bjorn Helgaas
  2025-12-16  8:39 ` [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86 Ziming Du
  2025-12-16  8:39 ` [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write() Ziming Du
  2 siblings, 1 reply; 16+ messages in thread
From: Ziming Du @ 2025-12-16  8:39 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, chrisw, jbarnes, alex.williamson,
	liuyongqiang13, duziming2

During the concurrent process of creating and rescanning in VF, the resource
files for the same "pci_dev" may be created twice. The second creation attempt
fails, resulting the "res_attr" in "pci_dev" to 'kfree', but the pointer is not
set to NULL. This will subsequently lead to dereferencing a null pointer when
removing the device.

When we perform the following operation:
"echo $vfcount > /sys/class/net/"$pfname"/device/sriov_numvfs &
 sleep 0.5
 echo 1 > /sys/bus/pci/rescan
 pci_remove "$pfname" "
system will crash as follows:

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=000020400d47b000
0000000000000000 pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 #1 SMP
CPU: 115 PID: 13659 Comm: testEL_vf_resca Kdump: loaded Tainted: G W E 6.6.0 #9
Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __pi_strlen+0x14/0x150
lr : kernfs_name_hash+0x24/0xa8
sp : ffff8001425c38f0
x29: ffff8001425c38f0 x28: ffff204021a21540 x27: 0000000000000000
x26: 0000000000000000 x25: 0000000000000000 x24: ffff20400f97fad0
x23: ffff20403145a0c0 x22: 0000000000000000 x21: 0000000000000000
x20: 0000000000000000 x19: 0000000000000000 x18: ffffffffffffffff
x17: 2f35322f38302038 x16: 392e3020534f4942 x15: 00000000fffffffd
x14: 0000000000000000 x13: 30643378302f3863 x12: 3378302b636e7973
x11: 00000000ffff7fff x10: 0000000000000000 x9 : ffff800100594b3c
x8 : 0101010101010101 x7 : 0000000000210d00 x6 : 67241f72241f7224
x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
__pi_strlen+0x14/0x150
kernfs_find_ns+0x54/0x120
kernfs_remove_by_name_ns+0x58/0xf0
sysfs_remove_bin_file+0x24/0x38
pci_remove_resource_files+0x44/0x90
pci_remove_sysfs_dev_files+0x28/0x40
pci_stop_bus_device+0xb8/0x118
pci_stop_and_remove_bus_device+0x20/0x40
pci_iov_remove_virtfn+0xb8/0x138
sriov_disable+0xbc/0x190
pci_disable_sriov+0x30/0x48
hinic_pci_sriov_disable+0x54/0x138 [hinic]
hinic_remove+0x140/0x290 [hinic]
pci_device_remove+0x4c/0xf8
device_remove+0x54/0x90
device_release_driver_internal+0x1d4/0x238
device_release_driver+0x20/0x38
pci_stop_bus_device+0xa8/0x118
pci_stop_and_remove_bus_device_locked+0x28/0x50
remove_store+0x128/0x208

Fix this by set the pointer to NULL after releasing 'res_attr' immediately.

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

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index c2df915ad2d2..7e697b82c5e1 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1222,12 +1222,14 @@ static void pci_remove_resource_files(struct pci_dev *pdev)
 		if (res_attr) {
 			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
 			kfree(res_attr);
+			pdev->res_attr[i] = NULL;
 		}
 
 		res_attr = pdev->res_attr_wc[i];
 		if (res_attr) {
 			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
 			kfree(res_attr);
+			pdev->res_attr_wc[i] = NULL;
 		}
 	}
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-16  8:39 [PATCH 0/3] Miscellaneous fixes for pci subsystem Ziming Du
  2025-12-16  8:39 ` [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug Ziming Du
@ 2025-12-16  8:39 ` Ziming Du
  2025-12-16 10:43   ` Ilpo Järvinen
                     ` (2 more replies)
  2025-12-16  8:39 ` [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write() Ziming Du
  2 siblings, 3 replies; 16+ messages in thread
From: Ziming Du @ 2025-12-16  8:39 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, chrisw, jbarnes, alex.williamson,
	liuyongqiang13, duziming2

From: Yongqiang Liu <liuyongqiang13@huawei.com>

Unaligned access is harmful for non-x86 archs such as arm64. When we
use pwrite or pread to access the I/O port resources with unaligned
offset, system will crash as follows:

Unable to handle kernel paging request at virtual address fffffbfffe8010c1
Internal error: Oops: 0000000096000061 [#1] SMP
Modules linked in:
CPU: 1 PID: 44230 Comm: syz.1.10955 Not tainted 6.6.0+ #1
Hardware name: linux,dummy-virt (DT)
pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __raw_writew arch/arm64/include/asm/io.h:33 [inline]
pc : _outw include/asm-generic/io.h:594 [inline]
pc : logic_outw+0x54/0x218 lib/logic_pio.c:305
lr : _outw include/asm-generic/io.h:593 [inline]
lr : logic_outw+0x40/0x218 lib/logic_pio.c:305
sp : ffff800083097a30
x29: ffff800083097a30 x28: ffffba71ba86e130 x27: 1ffff00010612f93
x26: ffff3bae63b3a420 x25: ffffba71bbf585d0 x24: 0000000000005ac1
x23: 00000000000010c1 x22: ffff3baf0deb6488 x21: 0000000000000002
x20: 00000000000010c1 x19: 0000000000ffbffe x18: 0000000000000000
x17: 0000000000000000 x16: ffffba71b9f44b48 x15: 00000000200002c0
x14: 0000000000000000 x13: 0000000000000000 x12: ffff6775ca80451f
x11: 1fffe775ca80451e x10: ffff6775ca80451e x9 : ffffba71bb78cf2c
x8 : 0000988a357fbae2 x7 : ffff3bae540228f7 x6 : 0000000000000001
x5 : 1fffe775e2b43c78 x4 : dfff800000000000 x3 : ffffba71b9a00000
x2 : ffff80008d22a000 x1 : ffffc58ec6600000 x0 : fffffbfffe8010c1
Call trace:
 _outw include/asm-generic/io.h:594 [inline]
 logic_outw+0x54/0x218 lib/logic_pio.c:305
 pci_resource_io drivers/pci/pci-sysfs.c:1157 [inline]
 pci_write_resource_io drivers/pci/pci-sysfs.c:1191 [inline]
 pci_write_resource_io+0x208/0x260 drivers/pci/pci-sysfs.c:1181
 sysfs_kf_bin_write+0x188/0x210 fs/sysfs/file.c:158
 kernfs_fop_write_iter+0x2e8/0x4b0 fs/kernfs/file.c:338
 call_write_iter include/linux/fs.h:2085 [inline]
 new_sync_write fs/read_write.c:493 [inline]
 vfs_write+0x7bc/0xac8 fs/read_write.c:586
 ksys_write+0x12c/0x270 fs/read_write.c:639
 __do_sys_write fs/read_write.c:651 [inline]
 __se_sys_write fs/read_write.c:648 [inline]
 __arm64_sys_write+0x78/0xb8 fs/read_write.c:648
 __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
 invoke_syscall+0x8c/0x2e0 arch/arm64/kernel/syscall.c:51
 el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:134
 do_el0_svc+0x4c/0x70 arch/arm64/kernel/syscall.c:176
 el0_svc+0x44/0x1d8 arch/arm64/kernel/entry-common.c:806
 el0t_64_sync_handler+0x100/0x130 arch/arm64/kernel/entry-common.c:844
 el0t_64_sync+0x3c8/0x3d0 arch/arm64/kernel/entry.S:757

Powerpc seems affected as well, so prohibit the unaligned access
on non-x86 archs.

Fixes: 8633328be242 ("PCI: Allow read/write access to sysfs I/O port resources")
Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
Signed-off-by: Ziming Du <duziming2@huawei.com>
---
 drivers/pci/pci-sysfs.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 7e697b82c5e1..6fa3c9d0e97e 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1141,6 +1141,13 @@ static int pci_mmap_resource_wc(struct file *filp, struct kobject *kobj,
 	return pci_mmap_resource(kobj, attr, vma, 1);
 }
 
+#if !defined(CONFIG_X86)
+static bool is_unaligned(unsigned long port, size_t size)
+{
+	return port & (size - 1);
+}
+#endif
+
 static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
 			       const struct bin_attribute *attr, char *buf,
 			       loff_t off, size_t count, bool write)
@@ -1158,6 +1165,11 @@ static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
 	if (port + count - 1 > pci_resource_end(pdev, bar))
 		return -EINVAL;
 
+#if !defined(CONFIG_X86)
+	if (is_unaligned(port, count))
+		return -EFAULT;
+#endif
+
 	switch (count) {
 	case 1:
 		if (write)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write()
  2025-12-16  8:39 [PATCH 0/3] Miscellaneous fixes for pci subsystem Ziming Du
  2025-12-16  8:39 ` [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug Ziming Du
  2025-12-16  8:39 ` [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86 Ziming Du
@ 2025-12-16  8:39 ` Ziming Du
  2025-12-16 10:57   ` Ilpo Järvinen
  2 siblings, 1 reply; 16+ messages in thread
From: Ziming Du @ 2025-12-16  8:39 UTC (permalink / raw)
  To: bhelgaas
  Cc: linux-pci, linux-kernel, chrisw, jbarnes, alex.williamson,
	liuyongqiang13, duziming2

When the value of ppos over the INT_MAX, the pos will be
set a negtive value which will be pass to get_user() or
pci_user_write_config_dword(). And unexpected behavior
such as a softlock happens:

 watchdog: BUG: soft lockup - CPU#0 stuck for 130s! [syz.3.109:3444]
 Modules linked in:
 CPU: 0 PID: 3444 Comm: syz.3.109 Not tainted 6.6.0+ #33
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
 RIP: 0010:_raw_spin_unlock_irq+0x17/0x30
 Code: cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 e8 52 12 00 00 90 fb 65 ff 0d b1 a1 86 6d <74> 05 e9 42 52 00 00 0f 1f 44 00 00 c3 cc cc cc cc 0f 1f 84 00 00
 RSP: 0018:ffff88816851fb50 EFLAGS: 00000246
 RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff927daf9b
 RDX: 0000000000000cfc RSI: 0000000000000046 RDI: ffffffff9a7c7400
 RBP: 00000000818bb9dc R08: 0000000000000001 R09: ffffed102d0a3f59
 R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000000
 R13: ffff888102220000 R14: ffffffff926d3b10 R15: 00000000210bbb5f
 FS:  00007ff2d4e56640(0000) GS:ffff8881f5c00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 00000000210bbb5b CR3: 0000000147374002 CR4: 0000000000772ef0
 PKRU: 00000000
 Call Trace:
  <TASK>
  pci_user_write_config_dword+0x126/0x1f0
  ? __get_user_nocheck_8+0x20/0x20
  proc_bus_pci_write+0x273/0x470
  proc_reg_write+0x1b6/0x280
  do_iter_write+0x48e/0x790
  ? import_iovec+0x47/0x90
  vfs_writev+0x125/0x4a0
  ? futex_wake+0xed/0x500
  ? __pfx_vfs_writev+0x10/0x10
  ? userfaultfd_ioctl+0x131/0x1ae0
  ? userfaultfd_ioctl+0x131/0x1ae0
  ? do_futex+0x17e/0x220
  ? __pfx_do_futex+0x10/0x10
  ? __fget_files+0x193/0x2b0
  __x64_sys_pwritev+0x1e2/0x2a0
  ? __pfx___x64_sys_pwritev+0x10/0x10
  do_syscall_64+0x59/0x110
  entry_SYSCALL_64_after_hwframe+0x78/0xe2

Fix this by use unsigned int for the pos.

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 9348a0fb8084..dbec1d4209c9 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 pos = *ppos;
+	unsigned int pos = *ppos;
 	int size = dev->cfg_size;
 	int cnt, ret;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-16  8:39 ` [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86 Ziming Du
@ 2025-12-16 10:43   ` Ilpo Järvinen
  2025-12-17  9:47     ` duziming
  2025-12-20 16:20   ` kernel test robot
  2025-12-22  5:01   ` kernel test robot
  2 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-12-16 10:43 UTC (permalink / raw)
  To: Ziming Du
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13

On Tue, 16 Dec 2025, Ziming Du wrote:

> From: Yongqiang Liu <liuyongqiang13@huawei.com>
> 
> Unaligned access is harmful for non-x86 archs such as arm64. When we
> use pwrite or pread to access the I/O port resources with unaligned
> offset, system will crash as follows:
> 
> Unable to handle kernel paging request at virtual address fffffbfffe8010c1
> Internal error: Oops: 0000000096000061 [#1] SMP
> Modules linked in:
> CPU: 1 PID: 44230 Comm: syz.1.10955 Not tainted 6.6.0+ #1
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __raw_writew arch/arm64/include/asm/io.h:33 [inline]
> pc : _outw include/asm-generic/io.h:594 [inline]
> pc : logic_outw+0x54/0x218 lib/logic_pio.c:305
> lr : _outw include/asm-generic/io.h:593 [inline]
> lr : logic_outw+0x40/0x218 lib/logic_pio.c:305
> sp : ffff800083097a30
> x29: ffff800083097a30 x28: ffffba71ba86e130 x27: 1ffff00010612f93
> x26: ffff3bae63b3a420 x25: ffffba71bbf585d0 x24: 0000000000005ac1
> x23: 00000000000010c1 x22: ffff3baf0deb6488 x21: 0000000000000002
> x20: 00000000000010c1 x19: 0000000000ffbffe x18: 0000000000000000
> x17: 0000000000000000 x16: ffffba71b9f44b48 x15: 00000000200002c0
> x14: 0000000000000000 x13: 0000000000000000 x12: ffff6775ca80451f
> x11: 1fffe775ca80451e x10: ffff6775ca80451e x9 : ffffba71bb78cf2c
> x8 : 0000988a357fbae2 x7 : ffff3bae540228f7 x6 : 0000000000000001
> x5 : 1fffe775e2b43c78 x4 : dfff800000000000 x3 : ffffba71b9a00000
> x2 : ffff80008d22a000 x1 : ffffc58ec6600000 x0 : fffffbfffe8010c1
> Call trace:
>  _outw include/asm-generic/io.h:594 [inline]
>  logic_outw+0x54/0x218 lib/logic_pio.c:305
>  pci_resource_io drivers/pci/pci-sysfs.c:1157 [inline]
>  pci_write_resource_io drivers/pci/pci-sysfs.c:1191 [inline]
>  pci_write_resource_io+0x208/0x260 drivers/pci/pci-sysfs.c:1181
>  sysfs_kf_bin_write+0x188/0x210 fs/sysfs/file.c:158
>  kernfs_fop_write_iter+0x2e8/0x4b0 fs/kernfs/file.c:338
>  call_write_iter include/linux/fs.h:2085 [inline]
>  new_sync_write fs/read_write.c:493 [inline]
>  vfs_write+0x7bc/0xac8 fs/read_write.c:586
>  ksys_write+0x12c/0x270 fs/read_write.c:639
>  __do_sys_write fs/read_write.c:651 [inline]
>  __se_sys_write fs/read_write.c:648 [inline]
>  __arm64_sys_write+0x78/0xb8 fs/read_write.c:648
>  __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
>  invoke_syscall+0x8c/0x2e0 arch/arm64/kernel/syscall.c:51
>  el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:134
>  do_el0_svc+0x4c/0x70 arch/arm64/kernel/syscall.c:176
>  el0_svc+0x44/0x1d8 arch/arm64/kernel/entry-common.c:806
>  el0t_64_sync_handler+0x100/0x130 arch/arm64/kernel/entry-common.c:844
>  el0t_64_sync+0x3c8/0x3d0 arch/arm64/kernel/entry.S:757
> 
> Powerpc seems affected as well, so prohibit the unaligned access
> on non-x86 archs.
> 
> Fixes: 8633328be242 ("PCI: Allow read/write access to sysfs I/O port resources")
> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
> Signed-off-by: Ziming Du <duziming2@huawei.com>
> ---
>  drivers/pci/pci-sysfs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 7e697b82c5e1..6fa3c9d0e97e 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1141,6 +1141,13 @@ static int pci_mmap_resource_wc(struct file *filp, struct kobject *kobj,
>  	return pci_mmap_resource(kobj, attr, vma, 1);
>  }
>  
> +#if !defined(CONFIG_X86)
> +static bool is_unaligned(unsigned long port, size_t size)
> +{
> +	return port & (size - 1);
> +}
> +#endif
> +
>  static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
>  			       const struct bin_attribute *attr, char *buf,
>  			       loff_t off, size_t count, bool write)
> @@ -1158,6 +1165,11 @@ static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
>  	if (port + count - 1 > pci_resource_end(pdev, bar))
>  		return -EINVAL;
>  
> +#if !defined(CONFIG_X86)
> +	if (is_unaligned(port, count))
> +		return -EFAULT;
> +#endif
> +

This changes return value from -EINVAL -> -EFAULT for some values of count 
which seems not justified.

To me it's not clear why even x86 should allow unaligned access. This 
interface is very much geared towards natural alignment and sizing of the 
reads (e.g. count = 3 leads to -EINVAL), so it feels somewhat artificial 
to make x86 behave different here from the others.

>  	switch (count) {
>  	case 1:
>  		if (write)
> 

-- 
 i.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write()
  2025-12-16  8:39 ` [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write() Ziming Du
@ 2025-12-16 10:57   ` Ilpo Järvinen
  2025-12-17  9:33     ` duziming
  0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-12-16 10:57 UTC (permalink / raw)
  To: Ziming Du
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13

On Tue, 16 Dec 2025, Ziming Du wrote:

> When the value of ppos over the INT_MAX, the pos will be

is over

> set a negtive value which will be pass to get_user() or

set to a negative value which will be passed

> pci_user_write_config_dword(). And unexpected behavior

Please start the sentence with something else than And.

Hmm, the lines look rather short too, can you please reflow the changelog 
paragraphs to 75 characters.

> such as a softlock happens:
> 
>  watchdog: BUG: soft lockup - CPU#0 stuck for 130s! [syz.3.109:3444]
>  Modules linked in:
>  CPU: 0 PID: 3444 Comm: syz.3.109 Not tainted 6.6.0+ #33
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
>  RIP: 0010:_raw_spin_unlock_irq+0x17/0x30
>  Code: cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 e8 52 12 00 00 90 fb 65 ff 0d b1 a1 86 6d <74> 05 e9 42 52 00 00 0f 1f 44 00 00 c3 cc cc cc cc 0f 1f 84 00 00
>  RSP: 0018:ffff88816851fb50 EFLAGS: 00000246
>  RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff927daf9b
>  RDX: 0000000000000cfc RSI: 0000000000000046 RDI: ffffffff9a7c7400
>  RBP: 00000000818bb9dc R08: 0000000000000001 R09: ffffed102d0a3f59
>  R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000000
>  R13: ffff888102220000 R14: ffffffff926d3b10 R15: 00000000210bbb5f
>  FS:  00007ff2d4e56640(0000) GS:ffff8881f5c00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 00000000210bbb5b CR3: 0000000147374002 CR4: 0000000000772ef0
>  PKRU: 00000000
>  Call Trace:
>   <TASK>
>   pci_user_write_config_dword+0x126/0x1f0
>   ? __get_user_nocheck_8+0x20/0x20
>   proc_bus_pci_write+0x273/0x470
>   proc_reg_write+0x1b6/0x280
>   do_iter_write+0x48e/0x790
>   ? import_iovec+0x47/0x90
>   vfs_writev+0x125/0x4a0
>   ? futex_wake+0xed/0x500
>   ? __pfx_vfs_writev+0x10/0x10
>   ? userfaultfd_ioctl+0x131/0x1ae0
>   ? userfaultfd_ioctl+0x131/0x1ae0
>   ? do_futex+0x17e/0x220
>   ? __pfx_do_futex+0x10/0x10
>   ? __fget_files+0x193/0x2b0
>   __x64_sys_pwritev+0x1e2/0x2a0
>   ? __pfx___x64_sys_pwritev+0x10/0x10
>   do_syscall_64+0x59/0x110
>   entry_SYSCALL_64_after_hwframe+0x78/0xe2

Could you please trim the dump so it only contains things relevant to this 
issue () (also check trimming in the other patches).

> Fix this by use unsigned int for the pos.
> 
> 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 9348a0fb8084..dbec1d4209c9 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 pos = *ppos;
> +	unsigned int pos = *ppos;
>  	int size = dev->cfg_size;
>  	int cnt, ret;

So this still throws away some bits compared with the original ppos ?

-- 
 i.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write()
  2025-12-16 10:57   ` Ilpo Järvinen
@ 2025-12-17  9:33     ` duziming
  2025-12-17 10:19       ` Ilpo Järvinen
  0 siblings, 1 reply; 16+ messages in thread
From: duziming @ 2025-12-17  9:33 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13


在 2025/12/16 18:57, Ilpo Järvinen 写道:
> On Tue, 16 Dec 2025, Ziming Du wrote:
>
>> When the value of ppos over the INT_MAX, the pos will be
> is over
>
>> set a negtive value which will be pass to get_user() or
> set to a negative value which will be passed
>
>> pci_user_write_config_dword(). And unexpected behavior
> Please start the sentence with something else than And.
>
> Hmm, the lines look rather short too, can you please reflow the changelog
> paragraphs to 75 characters.

Thanks for the review. I'll reflow the changelog to 75-character lines 
and avoid

starting sentences with 'And' in the next revision.

>> such as a softlock happens:
>>
>>   watchdog: BUG: soft lockup - CPU#0 stuck for 130s! [syz.3.109:3444]
>>   Modules linked in:
>>   CPU: 0 PID: 3444 Comm: syz.3.109 Not tainted 6.6.0+ #33
>>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
>>   RIP: 0010:_raw_spin_unlock_irq+0x17/0x30
>>   Code: cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 e8 52 12 00 00 90 fb 65 ff 0d b1 a1 86 6d <74> 05 e9 42 52 00 00 0f 1f 44 00 00 c3 cc cc cc cc 0f 1f 84 00 00
>>   RSP: 0018:ffff88816851fb50 EFLAGS: 00000246
>>   RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff927daf9b
>>   RDX: 0000000000000cfc RSI: 0000000000000046 RDI: ffffffff9a7c7400
>>   RBP: 00000000818bb9dc R08: 0000000000000001 R09: ffffed102d0a3f59
>>   R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000000
>>   R13: ffff888102220000 R14: ffffffff926d3b10 R15: 00000000210bbb5f
>>   FS:  00007ff2d4e56640(0000) GS:ffff8881f5c00000(0000) knlGS:0000000000000000
>>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>   CR2: 00000000210bbb5b CR3: 0000000147374002 CR4: 0000000000772ef0
>>   PKRU: 00000000
>>   Call Trace:
>>    <TASK>
>>    pci_user_write_config_dword+0x126/0x1f0
>>    ? __get_user_nocheck_8+0x20/0x20
>>    proc_bus_pci_write+0x273/0x470
>>    proc_reg_write+0x1b6/0x280
>>    do_iter_write+0x48e/0x790
>>    ? import_iovec+0x47/0x90
>>    vfs_writev+0x125/0x4a0
>>    ? futex_wake+0xed/0x500
>>    ? __pfx_vfs_writev+0x10/0x10
>>    ? userfaultfd_ioctl+0x131/0x1ae0
>>    ? userfaultfd_ioctl+0x131/0x1ae0
>>    ? do_futex+0x17e/0x220
>>    ? __pfx_do_futex+0x10/0x10
>>    ? __fget_files+0x193/0x2b0
>>    __x64_sys_pwritev+0x1e2/0x2a0
>>    ? __pfx___x64_sys_pwritev+0x10/0x10
>>    do_syscall_64+0x59/0x110
>>    entry_SYSCALL_64_after_hwframe+0x78/0xe2
> Could you please trim the dump so it only contains things relevant to this
> issue () (also check trimming in the other patches).
Thanks for pointing that out, we'll make sure to only keep the relevant 
stacks in future patches.
>> Fix this by use unsigned int for the pos.
>>
>> 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 9348a0fb8084..dbec1d4209c9 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 pos = *ppos;
>> +	unsigned int pos = *ppos;
>>   	int size = dev->cfg_size;
>>   	int cnt, ret;
> So this still throws away some bits compared with the original ppos ?

The current approach may lose some precision compared to the original 
ppos, but a later check ensures  pos

remains valid—so any potential information loss is handled safely.

>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-16 10:43   ` Ilpo Järvinen
@ 2025-12-17  9:47     ` duziming
  2025-12-17 10:15       ` Ilpo Järvinen
  0 siblings, 1 reply; 16+ messages in thread
From: duziming @ 2025-12-17  9:47 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13


在 2025/12/16 18:43, Ilpo Järvinen 写道:
> On Tue, 16 Dec 2025, Ziming Du wrote:
>
>> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>>
>> Unaligned access is harmful for non-x86 archs such as arm64. When we
>> use pwrite or pread to access the I/O port resources with unaligned
>> offset, system will crash as follows:
>>
>> Unable to handle kernel paging request at virtual address fffffbfffe8010c1
>> Internal error: Oops: 0000000096000061 [#1] SMP
>> Modules linked in:
>> CPU: 1 PID: 44230 Comm: syz.1.10955 Not tainted 6.6.0+ #1
>> Hardware name: linux,dummy-virt (DT)
>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : __raw_writew arch/arm64/include/asm/io.h:33 [inline]
>> pc : _outw include/asm-generic/io.h:594 [inline]
>> pc : logic_outw+0x54/0x218 lib/logic_pio.c:305
>> lr : _outw include/asm-generic/io.h:593 [inline]
>> lr : logic_outw+0x40/0x218 lib/logic_pio.c:305
>> sp : ffff800083097a30
>> x29: ffff800083097a30 x28: ffffba71ba86e130 x27: 1ffff00010612f93
>> x26: ffff3bae63b3a420 x25: ffffba71bbf585d0 x24: 0000000000005ac1
>> x23: 00000000000010c1 x22: ffff3baf0deb6488 x21: 0000000000000002
>> x20: 00000000000010c1 x19: 0000000000ffbffe x18: 0000000000000000
>> x17: 0000000000000000 x16: ffffba71b9f44b48 x15: 00000000200002c0
>> x14: 0000000000000000 x13: 0000000000000000 x12: ffff6775ca80451f
>> x11: 1fffe775ca80451e x10: ffff6775ca80451e x9 : ffffba71bb78cf2c
>> x8 : 0000988a357fbae2 x7 : ffff3bae540228f7 x6 : 0000000000000001
>> x5 : 1fffe775e2b43c78 x4 : dfff800000000000 x3 : ffffba71b9a00000
>> x2 : ffff80008d22a000 x1 : ffffc58ec6600000 x0 : fffffbfffe8010c1
>> Call trace:
>>   _outw include/asm-generic/io.h:594 [inline]
>>   logic_outw+0x54/0x218 lib/logic_pio.c:305
>>   pci_resource_io drivers/pci/pci-sysfs.c:1157 [inline]
>>   pci_write_resource_io drivers/pci/pci-sysfs.c:1191 [inline]
>>   pci_write_resource_io+0x208/0x260 drivers/pci/pci-sysfs.c:1181
>>   sysfs_kf_bin_write+0x188/0x210 fs/sysfs/file.c:158
>>   kernfs_fop_write_iter+0x2e8/0x4b0 fs/kernfs/file.c:338
>>   call_write_iter include/linux/fs.h:2085 [inline]
>>   new_sync_write fs/read_write.c:493 [inline]
>>   vfs_write+0x7bc/0xac8 fs/read_write.c:586
>>   ksys_write+0x12c/0x270 fs/read_write.c:639
>>   __do_sys_write fs/read_write.c:651 [inline]
>>   __se_sys_write fs/read_write.c:648 [inline]
>>   __arm64_sys_write+0x78/0xb8 fs/read_write.c:648
>>   __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
>>   invoke_syscall+0x8c/0x2e0 arch/arm64/kernel/syscall.c:51
>>   el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:134
>>   do_el0_svc+0x4c/0x70 arch/arm64/kernel/syscall.c:176
>>   el0_svc+0x44/0x1d8 arch/arm64/kernel/entry-common.c:806
>>   el0t_64_sync_handler+0x100/0x130 arch/arm64/kernel/entry-common.c:844
>>   el0t_64_sync+0x3c8/0x3d0 arch/arm64/kernel/entry.S:757
>>
>> Powerpc seems affected as well, so prohibit the unaligned access
>> on non-x86 archs.
>>
>> Fixes: 8633328be242 ("PCI: Allow read/write access to sysfs I/O port resources")
>> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
>> Signed-off-by: Ziming Du <duziming2@huawei.com>
>> ---
>>   drivers/pci/pci-sysfs.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>> index 7e697b82c5e1..6fa3c9d0e97e 100644
>> --- a/drivers/pci/pci-sysfs.c
>> +++ b/drivers/pci/pci-sysfs.c
>> @@ -1141,6 +1141,13 @@ static int pci_mmap_resource_wc(struct file *filp, struct kobject *kobj,
>>   	return pci_mmap_resource(kobj, attr, vma, 1);
>>   }
>>   
>> +#if !defined(CONFIG_X86)
>> +static bool is_unaligned(unsigned long port, size_t size)
>> +{
>> +	return port & (size - 1);
>> +}
>> +#endif
>> +
>>   static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
>>   			       const struct bin_attribute *attr, char *buf,
>>   			       loff_t off, size_t count, bool write)
>> @@ -1158,6 +1165,11 @@ static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
>>   	if (port + count - 1 > pci_resource_end(pdev, bar))
>>   		return -EINVAL;
>>   
>> +#if !defined(CONFIG_X86)
>> +	if (is_unaligned(port, count))
>> +		return -EFAULT;
>> +#endif
>> +
> This changes return value from -EINVAL -> -EFAULT for some values of count
> which seems not justified.
>
> To me it's not clear why even x86 should allow unaligned access. This
> interface is very much geared towards natural alignment and sizing of the
> reads (e.g. count = 3 leads to -EINVAL), so it feels somewhat artificial
> to make x86 behave different here from the others.

Thanks for your review! We verify that when count = 3, the return value 
will not be

-EFAULT; It will only return -EFAULT in cases of unaligned access.

We conduct a POC on QEMU with the ARM architecture as follows:

#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>

int main()
{
         int fd = open("/sys/bus/pci/devices/0000:00:01.0/resource0", 
O_RDWR);
         char buf[] = "1233333";
         if (fd < 0) {
                 printf("open failed\n");
                 return 1;
         }

         pwrite(fd, buf, 2, 1);

         return 0;
}

On x86, this does not trigger a kernel panic.

>>   	switch (count) {
>>   	case 1:
>>   		if (write)
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-17  9:47     ` duziming
@ 2025-12-17 10:15       ` Ilpo Järvinen
  2025-12-18  8:03         ` duziming
  0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-12-17 10:15 UTC (permalink / raw)
  To: duziming
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13

[-- Attachment #1: Type: text/plain, Size: 6131 bytes --]

On Wed, 17 Dec 2025, duziming wrote:

> 
> 在 2025/12/16 18:43, Ilpo Järvinen 写道:
> > On Tue, 16 Dec 2025, Ziming Du wrote:
> > 
> > > From: Yongqiang Liu <liuyongqiang13@huawei.com>
> > > 
> > > Unaligned access is harmful for non-x86 archs such as arm64. When we
> > > use pwrite or pread to access the I/O port resources with unaligned
> > > offset, system will crash as follows:
> > > 
> > > Unable to handle kernel paging request at virtual address fffffbfffe8010c1
> > > Internal error: Oops: 0000000096000061 [#1] SMP
> > > Modules linked in:
> > > CPU: 1 PID: 44230 Comm: syz.1.10955 Not tainted 6.6.0+ #1
> > > Hardware name: linux,dummy-virt (DT)
> > > pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > pc : __raw_writew arch/arm64/include/asm/io.h:33 [inline]
> > > pc : _outw include/asm-generic/io.h:594 [inline]
> > > pc : logic_outw+0x54/0x218 lib/logic_pio.c:305
> > > lr : _outw include/asm-generic/io.h:593 [inline]
> > > lr : logic_outw+0x40/0x218 lib/logic_pio.c:305
> > > sp : ffff800083097a30
> > > x29: ffff800083097a30 x28: ffffba71ba86e130 x27: 1ffff00010612f93
> > > x26: ffff3bae63b3a420 x25: ffffba71bbf585d0 x24: 0000000000005ac1
> > > x23: 00000000000010c1 x22: ffff3baf0deb6488 x21: 0000000000000002
> > > x20: 00000000000010c1 x19: 0000000000ffbffe x18: 0000000000000000
> > > x17: 0000000000000000 x16: ffffba71b9f44b48 x15: 00000000200002c0
> > > x14: 0000000000000000 x13: 0000000000000000 x12: ffff6775ca80451f
> > > x11: 1fffe775ca80451e x10: ffff6775ca80451e x9 : ffffba71bb78cf2c
> > > x8 : 0000988a357fbae2 x7 : ffff3bae540228f7 x6 : 0000000000000001
> > > x5 : 1fffe775e2b43c78 x4 : dfff800000000000 x3 : ffffba71b9a00000
> > > x2 : ffff80008d22a000 x1 : ffffc58ec6600000 x0 : fffffbfffe8010c1
> > > Call trace:
> > >   _outw include/asm-generic/io.h:594 [inline]
> > >   logic_outw+0x54/0x218 lib/logic_pio.c:305
> > >   pci_resource_io drivers/pci/pci-sysfs.c:1157 [inline]
> > >   pci_write_resource_io drivers/pci/pci-sysfs.c:1191 [inline]
> > >   pci_write_resource_io+0x208/0x260 drivers/pci/pci-sysfs.c:1181
> > >   sysfs_kf_bin_write+0x188/0x210 fs/sysfs/file.c:158
> > >   kernfs_fop_write_iter+0x2e8/0x4b0 fs/kernfs/file.c:338
> > >   call_write_iter include/linux/fs.h:2085 [inline]
> > >   new_sync_write fs/read_write.c:493 [inline]
> > >   vfs_write+0x7bc/0xac8 fs/read_write.c:586
> > >   ksys_write+0x12c/0x270 fs/read_write.c:639
> > >   __do_sys_write fs/read_write.c:651 [inline]
> > >   __se_sys_write fs/read_write.c:648 [inline]
> > >   __arm64_sys_write+0x78/0xb8 fs/read_write.c:648
> > >   __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
> > >   invoke_syscall+0x8c/0x2e0 arch/arm64/kernel/syscall.c:51
> > >   el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:134
> > >   do_el0_svc+0x4c/0x70 arch/arm64/kernel/syscall.c:176
> > >   el0_svc+0x44/0x1d8 arch/arm64/kernel/entry-common.c:806
> > >   el0t_64_sync_handler+0x100/0x130 arch/arm64/kernel/entry-common.c:844
> > >   el0t_64_sync+0x3c8/0x3d0 arch/arm64/kernel/entry.S:757
> > > 
> > > Powerpc seems affected as well, so prohibit the unaligned access
> > > on non-x86 archs.
> > > 
> > > Fixes: 8633328be242 ("PCI: Allow read/write access to sysfs I/O port
> > > resources")
> > > Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
> > > Signed-off-by: Ziming Du <duziming2@huawei.com>
> > > ---
> > >   drivers/pci/pci-sysfs.c | 12 ++++++++++++
> > >   1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > > index 7e697b82c5e1..6fa3c9d0e97e 100644
> > > --- a/drivers/pci/pci-sysfs.c
> > > +++ b/drivers/pci/pci-sysfs.c
> > > @@ -1141,6 +1141,13 @@ static int pci_mmap_resource_wc(struct file *filp,
> > > struct kobject *kobj,
> > >   	return pci_mmap_resource(kobj, attr, vma, 1);
> > >   }
> > >   +#if !defined(CONFIG_X86)
> > > +static bool is_unaligned(unsigned long port, size_t size)
> > > +{
> > > +	return port & (size - 1);
> > > +}
> > > +#endif
> > > +
> > >   static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
> > >   			       const struct bin_attribute *attr, char *buf,
> > >   			       loff_t off, size_t count, bool write)
> > > @@ -1158,6 +1165,11 @@ static ssize_t pci_resource_io(struct file *filp,
> > > struct kobject *kobj,
> > >   	if (port + count - 1 > pci_resource_end(pdev, bar))
> > >   		return -EINVAL;
> > >   +#if !defined(CONFIG_X86)
> > > +	if (is_unaligned(port, count))
> > > +		return -EFAULT;
> > > +#endif
> > > +
> > This changes return value from -EINVAL -> -EFAULT for some values of count
> > which seems not justified.
> > 
> > To me it's not clear why even x86 should allow unaligned access. This
> > interface is very much geared towards natural alignment and sizing of the
> > reads (e.g. count = 3 leads to -EINVAL), so it feels somewhat artificial
> > to make x86 behave different here from the others.
> 
> Thanks for your review! We verify that when count = 3, the return value will
> not be
> 
> -EFAULT; It will only return -EFAULT in cases of unaligned access.

Oh, then there's even worse problem in your code as your is_aligned() 
assumes size is a power of two value.

Also, is_aligned() seems to be duplicating IS_ALIGNED() (your naming is 
very misleading as it's a prefixless name that overlaps with a generic 
macro with the very same name).

> We conduct a POC on QEMU with the ARM architecture as follows:
> 
> #include <stdio.h>
> #include <fcntl.h>
> #include <unistd.h>
> 
> int main()
> {
>         int fd = open("/sys/bus/pci/devices/0000:00:01.0/resource0", O_RDWR);
>         char buf[] = "1233333";
>         if (fd < 0) {
>                 printf("open failed\n");
>                 return 1;
>         }
> 
>         pwrite(fd, buf, 2, 1);
> 
>         return 0;
> }
> 
> On x86, this does not trigger a kernel panic.
> 
> > >   	switch (count) {
> > >   	case 1:
> > >   		if (write)
> > > 
> 

-- 
 i.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write()
  2025-12-17  9:33     ` duziming
@ 2025-12-17 10:19       ` Ilpo Järvinen
  2025-12-18  7:18         ` duziming
  0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-12-17 10:19 UTC (permalink / raw)
  To: duziming
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13

[-- Attachment #1: Type: text/plain, Size: 4288 bytes --]

On Wed, 17 Dec 2025, duziming wrote:
> 在 2025/12/16 18:57, Ilpo Järvinen 写道:
> > On Tue, 16 Dec 2025, Ziming Du wrote:
> > 
> > > When the value of ppos over the INT_MAX, the pos will be
> > is over
> > 
> > > set a negtive value which will be pass to get_user() or
> > set to a negative value which will be passed
> > 
> > > pci_user_write_config_dword(). And unexpected behavior
> > Please start the sentence with something else than And.
> > 
> > Hmm, the lines look rather short too, can you please reflow the changelog
> > paragraphs to 75 characters.
> 
> Thanks for the review. I'll reflow the changelog to 75-character lines and
> avoid
> 
> starting sentences with 'And' in the next revision.
> 
> > > such as a softlock happens:
> > > 
> > >   watchdog: BUG: soft lockup - CPU#0 stuck for 130s! [syz.3.109:3444]
> > >   Modules linked in:
> > >   CPU: 0 PID: 3444 Comm: syz.3.109 Not tainted 6.6.0+ #33
> > >   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> > >   RIP: 0010:_raw_spin_unlock_irq+0x17/0x30
> > >   Code: cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e
> > > fa 0f 1f 44 00 00 e8 52 12 00 00 90 fb 65 ff 0d b1 a1 86 6d <74> 05 e9 42
> > > 52 00 00 0f 1f 44 00 00 c3 cc cc cc cc 0f 1f 84 00 00
> > >   RSP: 0018:ffff88816851fb50 EFLAGS: 00000246
> > >   RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff927daf9b
> > >   RDX: 0000000000000cfc RSI: 0000000000000046 RDI: ffffffff9a7c7400
> > >   RBP: 00000000818bb9dc R08: 0000000000000001 R09: ffffed102d0a3f59
> > >   R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000000
> > >   R13: ffff888102220000 R14: ffffffff926d3b10 R15: 00000000210bbb5f
> > >   FS:  00007ff2d4e56640(0000) GS:ffff8881f5c00000(0000)
> > > knlGS:0000000000000000
> > >   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > >   CR2: 00000000210bbb5b CR3: 0000000147374002 CR4: 0000000000772ef0
> > >   PKRU: 00000000
> > >   Call Trace:
> > >    <TASK>
> > >    pci_user_write_config_dword+0x126/0x1f0
> > >    ? __get_user_nocheck_8+0x20/0x20
> > >    proc_bus_pci_write+0x273/0x470
> > >    proc_reg_write+0x1b6/0x280
> > >    do_iter_write+0x48e/0x790
> > >    ? import_iovec+0x47/0x90
> > >    vfs_writev+0x125/0x4a0
> > >    ? futex_wake+0xed/0x500
> > >    ? __pfx_vfs_writev+0x10/0x10
> > >    ? userfaultfd_ioctl+0x131/0x1ae0
> > >    ? userfaultfd_ioctl+0x131/0x1ae0
> > >    ? do_futex+0x17e/0x220
> > >    ? __pfx_do_futex+0x10/0x10
> > >    ? __fget_files+0x193/0x2b0
> > >    __x64_sys_pwritev+0x1e2/0x2a0
> > >    ? __pfx___x64_sys_pwritev+0x10/0x10
> > >    do_syscall_64+0x59/0x110
> > >    entry_SYSCALL_64_after_hwframe+0x78/0xe2
> > Could you please trim the dump so it only contains things relevant to this
> > issue () (also check trimming in the other patches).
> Thanks for pointing that out, we'll make sure to only keep the relevant stacks
> in future patches.
> > > Fix this by use unsigned int for the pos.
> > > 
> > > 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 9348a0fb8084..dbec1d4209c9 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 pos = *ppos;
> > > +	unsigned int pos = *ppos;
> > >   	int size = dev->cfg_size;
> > >   	int cnt, ret;
> > So this still throws away some bits compared with the original ppos ?
> 
> The current approach may lose some precision compared to the original ppos,
> but a later check ensures  pos
> 
> remains valid—so any potential information loss is handled safely.

That's somewhat odd definition of "valid" if a big ppos results in 
a smaller number after the precision loss that is smaller than size. 

-- 
 i.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write()
  2025-12-17 10:19       ` Ilpo Järvinen
@ 2025-12-18  7:18         ` duziming
  0 siblings, 0 replies; 16+ messages in thread
From: duziming @ 2025-12-18  7:18 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13


在 2025/12/17 18:19, Ilpo Järvinen 写道:
> On Wed, 17 Dec 2025, duziming wrote:
>> 在 2025/12/16 18:57, Ilpo Järvinen 写道:
>>> On Tue, 16 Dec 2025, Ziming Du wrote:
>>>
>>>> When the value of ppos over the INT_MAX, the pos will be
>>> is over
>>>
>>>> set a negtive value which will be pass to get_user() or
>>> set to a negative value which will be passed
>>>
>>>> pci_user_write_config_dword(). And unexpected behavior
>>> Please start the sentence with something else than And.
>>>
>>> Hmm, the lines look rather short too, can you please reflow the changelog
>>> paragraphs to 75 characters.
>> Thanks for the review. I'll reflow the changelog to 75-character lines and
>> avoid
>>
>> starting sentences with 'And' in the next revision.
>>
>>>> such as a softlock happens:
>>>>
>>>>    watchdog: BUG: soft lockup - CPU#0 stuck for 130s! [syz.3.109:3444]
>>>>    Modules linked in:
>>>>    CPU: 0 PID: 3444 Comm: syz.3.109 Not tainted 6.6.0+ #33
>>>>    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>>>> rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
>>>>    RIP: 0010:_raw_spin_unlock_irq+0x17/0x30
>>>>    Code: cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e
>>>> fa 0f 1f 44 00 00 e8 52 12 00 00 90 fb 65 ff 0d b1 a1 86 6d <74> 05 e9 42
>>>> 52 00 00 0f 1f 44 00 00 c3 cc cc cc cc 0f 1f 84 00 00
>>>>    RSP: 0018:ffff88816851fb50 EFLAGS: 00000246
>>>>    RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff927daf9b
>>>>    RDX: 0000000000000cfc RSI: 0000000000000046 RDI: ffffffff9a7c7400
>>>>    RBP: 00000000818bb9dc R08: 0000000000000001 R09: ffffed102d0a3f59
>>>>    R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000000
>>>>    R13: ffff888102220000 R14: ffffffff926d3b10 R15: 00000000210bbb5f
>>>>    FS:  00007ff2d4e56640(0000) GS:ffff8881f5c00000(0000)
>>>> knlGS:0000000000000000
>>>>    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>    CR2: 00000000210bbb5b CR3: 0000000147374002 CR4: 0000000000772ef0
>>>>    PKRU: 00000000
>>>>    Call Trace:
>>>>     <TASK>
>>>>     pci_user_write_config_dword+0x126/0x1f0
>>>>     ? __get_user_nocheck_8+0x20/0x20
>>>>     proc_bus_pci_write+0x273/0x470
>>>>     proc_reg_write+0x1b6/0x280
>>>>     do_iter_write+0x48e/0x790
>>>>     ? import_iovec+0x47/0x90
>>>>     vfs_writev+0x125/0x4a0
>>>>     ? futex_wake+0xed/0x500
>>>>     ? __pfx_vfs_writev+0x10/0x10
>>>>     ? userfaultfd_ioctl+0x131/0x1ae0
>>>>     ? userfaultfd_ioctl+0x131/0x1ae0
>>>>     ? do_futex+0x17e/0x220
>>>>     ? __pfx_do_futex+0x10/0x10
>>>>     ? __fget_files+0x193/0x2b0
>>>>     __x64_sys_pwritev+0x1e2/0x2a0
>>>>     ? __pfx___x64_sys_pwritev+0x10/0x10
>>>>     do_syscall_64+0x59/0x110
>>>>     entry_SYSCALL_64_after_hwframe+0x78/0xe2
>>> Could you please trim the dump so it only contains things relevant to this
>>> issue () (also check trimming in the other patches).
>> Thanks for pointing that out, we'll make sure to only keep the relevant stacks
>> in future patches.
>>>> Fix this by use unsigned int for the pos.
>>>>
>>>> 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 9348a0fb8084..dbec1d4209c9 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 pos = *ppos;
>>>> +	unsigned int pos = *ppos;
>>>>    	int size = dev->cfg_size;
>>>>    	int cnt, ret;
>>> So this still throws away some bits compared with the original ppos ?
>> The current approach may lose some precision compared to the original ppos,
>> but a later check ensures  pos
>>
>> remains valid—so any potential information loss is handled safely.
> That's somewhat odd definition of "valid" if a big ppos results in
> a smaller number after the precision loss that is smaller than size.

Oh, I get your concern now. In fact, in previous version, we fixed it 
like this :

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index dbec1d4209c9..200d42feafd8 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);
-       unsigned int pos = *ppos;
+       int pos = *ppos;
         int size = dev->cfg_size;
         int cnt, ret;

@@ -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;
         if (nbytes >= size)
                 nbytes = size;

In addition, we notice that in proc_bus_pci_read(), "unsigned int pos = 
*ppos" might also cause some issues.

>

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-17 10:15       ` Ilpo Järvinen
@ 2025-12-18  8:03         ` duziming
  0 siblings, 0 replies; 16+ messages in thread
From: duziming @ 2025-12-18  8:03 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: bhelgaas, linux-pci, LKML, chrisw, jbarnes, alex.williamson,
	liuyongqiang13


在 2025/12/17 18:15, Ilpo Järvinen 写道:
> On Wed, 17 Dec 2025, duziming wrote:
>
>> 在 2025/12/16 18:43, Ilpo Järvinen 写道:
>>> On Tue, 16 Dec 2025, Ziming Du wrote:
>>>
>>>> From: Yongqiang Liu <liuyongqiang13@huawei.com>
>>>>
>>>> Unaligned access is harmful for non-x86 archs such as arm64. When we
>>>> use pwrite or pread to access the I/O port resources with unaligned
>>>> offset, system will crash as follows:
>>>>
>>>> Unable to handle kernel paging request at virtual address fffffbfffe8010c1
>>>> Internal error: Oops: 0000000096000061 [#1] SMP
>>>> Modules linked in:
>>>> CPU: 1 PID: 44230 Comm: syz.1.10955 Not tainted 6.6.0+ #1
>>>> Hardware name: linux,dummy-virt (DT)
>>>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>>> pc : __raw_writew arch/arm64/include/asm/io.h:33 [inline]
>>>> pc : _outw include/asm-generic/io.h:594 [inline]
>>>> pc : logic_outw+0x54/0x218 lib/logic_pio.c:305
>>>> lr : _outw include/asm-generic/io.h:593 [inline]
>>>> lr : logic_outw+0x40/0x218 lib/logic_pio.c:305
>>>> sp : ffff800083097a30
>>>> x29: ffff800083097a30 x28: ffffba71ba86e130 x27: 1ffff00010612f93
>>>> x26: ffff3bae63b3a420 x25: ffffba71bbf585d0 x24: 0000000000005ac1
>>>> x23: 00000000000010c1 x22: ffff3baf0deb6488 x21: 0000000000000002
>>>> x20: 00000000000010c1 x19: 0000000000ffbffe x18: 0000000000000000
>>>> x17: 0000000000000000 x16: ffffba71b9f44b48 x15: 00000000200002c0
>>>> x14: 0000000000000000 x13: 0000000000000000 x12: ffff6775ca80451f
>>>> x11: 1fffe775ca80451e x10: ffff6775ca80451e x9 : ffffba71bb78cf2c
>>>> x8 : 0000988a357fbae2 x7 : ffff3bae540228f7 x6 : 0000000000000001
>>>> x5 : 1fffe775e2b43c78 x4 : dfff800000000000 x3 : ffffba71b9a00000
>>>> x2 : ffff80008d22a000 x1 : ffffc58ec6600000 x0 : fffffbfffe8010c1
>>>> Call trace:
>>>>    _outw include/asm-generic/io.h:594 [inline]
>>>>    logic_outw+0x54/0x218 lib/logic_pio.c:305
>>>>    pci_resource_io drivers/pci/pci-sysfs.c:1157 [inline]
>>>>    pci_write_resource_io drivers/pci/pci-sysfs.c:1191 [inline]
>>>>    pci_write_resource_io+0x208/0x260 drivers/pci/pci-sysfs.c:1181
>>>>    sysfs_kf_bin_write+0x188/0x210 fs/sysfs/file.c:158
>>>>    kernfs_fop_write_iter+0x2e8/0x4b0 fs/kernfs/file.c:338
>>>>    call_write_iter include/linux/fs.h:2085 [inline]
>>>>    new_sync_write fs/read_write.c:493 [inline]
>>>>    vfs_write+0x7bc/0xac8 fs/read_write.c:586
>>>>    ksys_write+0x12c/0x270 fs/read_write.c:639
>>>>    __do_sys_write fs/read_write.c:651 [inline]
>>>>    __se_sys_write fs/read_write.c:648 [inline]
>>>>    __arm64_sys_write+0x78/0xb8 fs/read_write.c:648
>>>>    __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
>>>>    invoke_syscall+0x8c/0x2e0 arch/arm64/kernel/syscall.c:51
>>>>    el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:134
>>>>    do_el0_svc+0x4c/0x70 arch/arm64/kernel/syscall.c:176
>>>>    el0_svc+0x44/0x1d8 arch/arm64/kernel/entry-common.c:806
>>>>    el0t_64_sync_handler+0x100/0x130 arch/arm64/kernel/entry-common.c:844
>>>>    el0t_64_sync+0x3c8/0x3d0 arch/arm64/kernel/entry.S:757
>>>>
>>>> Powerpc seems affected as well, so prohibit the unaligned access
>>>> on non-x86 archs.
>>>>
>>>> Fixes: 8633328be242 ("PCI: Allow read/write access to sysfs I/O port
>>>> resources")
>>>> Signed-off-by: Yongqiang Liu <liuyongqiang13@huawei.com>
>>>> Signed-off-by: Ziming Du <duziming2@huawei.com>
>>>> ---
>>>>    drivers/pci/pci-sysfs.c | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>>>> index 7e697b82c5e1..6fa3c9d0e97e 100644
>>>> --- a/drivers/pci/pci-sysfs.c
>>>> +++ b/drivers/pci/pci-sysfs.c
>>>> @@ -1141,6 +1141,13 @@ static int pci_mmap_resource_wc(struct file *filp,
>>>> struct kobject *kobj,
>>>>    	return pci_mmap_resource(kobj, attr, vma, 1);
>>>>    }
>>>>    +#if !defined(CONFIG_X86)
>>>> +static bool is_unaligned(unsigned long port, size_t size)
>>>> +{
>>>> +	return port & (size - 1);
>>>> +}
>>>> +#endif
>>>> +
>>>>    static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
>>>>    			       const struct bin_attribute *attr, char *buf,
>>>>    			       loff_t off, size_t count, bool write)
>>>> @@ -1158,6 +1165,11 @@ static ssize_t pci_resource_io(struct file *filp,
>>>> struct kobject *kobj,
>>>>    	if (port + count - 1 > pci_resource_end(pdev, bar))
>>>>    		return -EINVAL;
>>>>    +#if !defined(CONFIG_X86)
>>>> +	if (is_unaligned(port, count))
>>>> +		return -EFAULT;
>>>> +#endif
>>>> +
>>> This changes return value from -EINVAL -> -EFAULT for some values of count
>>> which seems not justified.
>>>
>>> To me it's not clear why even x86 should allow unaligned access. This
>>> interface is very much geared towards natural alignment and sizing of the
>>> reads (e.g. count = 3 leads to -EINVAL), so it feels somewhat artificial
>>> to make x86 behave different here from the others.
>> Thanks for your review! We verify that when count = 3, the return value will
>> not be
>>
>> -EFAULT; It will only return -EFAULT in cases of unaligned access.
> Oh, then there's even worse problem in your code as your is_aligned()
> assumes size is a power of two value.
>
> Also, is_aligned() seems to be duplicating IS_ALIGNED() (your naming is
> very misleading as it's a prefixless name that overlaps with a generic
> macro with the very same name).

Thanks for pointing that out. I'll update it to use the existing macro.

Our test shows the panic only triggers when count is 2 or 4, so we will 
scope**the fix to power of two

values and allow other cases to fall through to the default 
-EINVAL return without extra handling.

>> We conduct a POC on QEMU with the ARM architecture as follows:
>>
>> #include <stdio.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>>
>> int main()
>> {
>>          int fd = open("/sys/bus/pci/devices/0000:00:01.0/resource0", O_RDWR);
>>          char buf[] = "1233333";
>>          if (fd < 0) {
>>                  printf("open failed\n");
>>                  return 1;
>>          }
>>
>>          pwrite(fd, buf, 2, 1);
>>
>>          return 0;
>> }
>>
>> On x86, this does not trigger a kernel panic.
>>
>>>>    	switch (count) {
>>>>    	case 1:
>>>>    		if (write)
>>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-16  8:39 ` [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86 Ziming Du
  2025-12-16 10:43   ` Ilpo Järvinen
@ 2025-12-20 16:20   ` kernel test robot
  2025-12-22  5:01   ` kernel test robot
  2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2025-12-20 16:20 UTC (permalink / raw)
  To: Ziming Du, bhelgaas
  Cc: oe-kbuild-all, linux-pci, linux-kernel, chrisw, jbarnes,
	alex.williamson, liuyongqiang13, duziming2

Hi Ziming,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.19-rc1 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ziming-Du/PCI-sysfs-fix-null-pointer-dereference-during-PCI-hotplug/20251216-163452
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20251216083912.758219-3-duziming2%40huawei.com
patch subject: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
config: s390-randconfig-002-20251217 (https://download.01.org/0day-ci/archive/20251220/202512202328.VgDVWBKe-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251220/202512202328.VgDVWBKe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512202328.VgDVWBKe-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/pci-sysfs.c:1145:13: warning: 'is_unaligned' defined but not used [-Wunused-function]
    static bool is_unaligned(unsigned long port, size_t size)
                ^~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for CAN_DEV
   Depends on [n]: NETDEVICES [=n] && CAN [=y]
   Selected by [y]:
   - CAN [=y] && NET [=y]


vim +/is_unaligned +1145 drivers/pci/pci-sysfs.c

  1143	
  1144	#if !defined(CONFIG_X86)
> 1145	static bool is_unaligned(unsigned long port, size_t size)
  1146	{
  1147		return port & (size - 1);
  1148	}
  1149	#endif
  1150	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
  2025-12-16  8:39 ` [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86 Ziming Du
  2025-12-16 10:43   ` Ilpo Järvinen
  2025-12-20 16:20   ` kernel test robot
@ 2025-12-22  5:01   ` kernel test robot
  2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2025-12-22  5:01 UTC (permalink / raw)
  To: Ziming Du, bhelgaas
  Cc: oe-kbuild-all, linux-pci, linux-kernel, chrisw, jbarnes,
	alex.williamson, liuyongqiang13, duziming2

Hi Ziming,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.19-rc2 next-20251219]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ziming-Du/PCI-sysfs-fix-null-pointer-dereference-during-PCI-hotplug/20251216-163452
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20251216083912.758219-3-duziming2%40huawei.com
patch subject: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86
config: s390-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20251222/202512220635.gFQlRFAa-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251222/202512220635.gFQlRFAa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512220635.gFQlRFAa-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/pci-sysfs.c:1145:13: warning: 'is_unaligned' defined but not used [-Wunused-function]
    1145 | static bool is_unaligned(unsigned long port, size_t size)
         |             ^~~~~~~~~~~~


vim +/is_unaligned +1145 drivers/pci/pci-sysfs.c

  1143	
  1144	#if !defined(CONFIG_X86)
> 1145	static bool is_unaligned(unsigned long port, size_t size)
  1146	{
  1147		return port & (size - 1);
  1148	}
  1149	#endif
  1150	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug
  2025-12-16  8:39 ` [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug Ziming Du
@ 2025-12-23 16:55   ` Bjorn Helgaas
  2025-12-24  1:28     ` duziming
  0 siblings, 1 reply; 16+ messages in thread
From: Bjorn Helgaas @ 2025-12-23 16:55 UTC (permalink / raw)
  To: Ziming Du
  Cc: bhelgaas, linux-pci, linux-kernel, chrisw, jbarnes,
	alex.williamson, liuyongqiang13

Capitalize first word of subject to match history (see
"git log --oneline drivers/pci/pci-sysfs.c")

Drop "PCI" in "PCI hotplug" -- we already know this is PCI.

On Tue, Dec 16, 2025 at 04:39:10PM +0800, Ziming Du wrote:
> During the concurrent process of creating and rescanning in VF, the resource
> files for the same "pci_dev" may be created twice. The second creation attempt
> fails, resulting the "res_attr" in "pci_dev" to 'kfree', but the pointer is not
> set to NULL. This will subsequently lead to dereferencing a null pointer when
> removing the device.

Wrap this to fit in 75 columns so it still fits in 80 when git log
indents it.

Drop quotes around struct and member names, e.g., pci_dev, res_attr.

Drop '' around function names and add "()" after, e.g., kfree().

> When we perform the following operation:
> "echo $vfcount > /sys/class/net/"$pfname"/device/sriov_numvfs &
>  sleep 0.5
>  echo 1 > /sys/bus/pci/rescan
>  pci_remove "$pfname" "
> system will crash as follows:

Indent quoted material two spaces and drop the "" around it, e.g.,

When we perform the following operation:

  echo $vfcount > /sys/class/net/"$pfname"/device/sriov_numvfs &
  sleep 0.5
  ...

system will crash as follows.

> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=000020400d47b000
> 0000000000000000 pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000004 #1 SMP
> CPU: 115 PID: 13659 Comm: testEL_vf_resca Kdump: loaded Tainted: G W E 6.6.0 #9
> Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
> pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __pi_strlen+0x14/0x150
> lr : kernfs_name_hash+0x24/0xa8
> sp : ffff8001425c38f0
> x29: ffff8001425c38f0 x28: ffff204021a21540 x27: 0000000000000000
> x26: 0000000000000000 x25: 0000000000000000 x24: ffff20400f97fad0
> x23: ffff20403145a0c0 x22: 0000000000000000 x21: 0000000000000000
> x20: 0000000000000000 x19: 0000000000000000 x18: ffffffffffffffff
> x17: 2f35322f38302038 x16: 392e3020534f4942 x15: 00000000fffffffd
> x14: 0000000000000000 x13: 30643378302f3863 x12: 3378302b636e7973
> x11: 00000000ffff7fff x10: 0000000000000000 x9 : ffff800100594b3c
> x8 : 0101010101010101 x7 : 0000000000210d00 x6 : 67241f72241f7224
> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> __pi_strlen+0x14/0x150
> kernfs_find_ns+0x54/0x120
> kernfs_remove_by_name_ns+0x58/0xf0
> sysfs_remove_bin_file+0x24/0x38
> pci_remove_resource_files+0x44/0x90
> pci_remove_sysfs_dev_files+0x28/0x40
> pci_stop_bus_device+0xb8/0x118
> pci_stop_and_remove_bus_device+0x20/0x40
> pci_iov_remove_virtfn+0xb8/0x138
> sriov_disable+0xbc/0x190
> pci_disable_sriov+0x30/0x48
> hinic_pci_sriov_disable+0x54/0x138 [hinic]
> hinic_remove+0x140/0x290 [hinic]
> pci_device_remove+0x4c/0xf8
> device_remove+0x54/0x90
> device_release_driver_internal+0x1d4/0x238
> device_release_driver+0x20/0x38
> pci_stop_bus_device+0xa8/0x118
> pci_stop_and_remove_bus_device_locked+0x28/0x50
> remove_store+0x128/0x208

Indent this quoted material two spaces and remove parts that aren't
relevant.

> Fix this by set the pointer to NULL after releasing 'res_attr' immediately.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Ziming Du <duziming2@huawei.com>
> ---
>  drivers/pci/pci-sysfs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index c2df915ad2d2..7e697b82c5e1 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1222,12 +1222,14 @@ static void pci_remove_resource_files(struct pci_dev *pdev)
>  		if (res_attr) {
>  			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
>  			kfree(res_attr);
> +			pdev->res_attr[i] = NULL;
>  		}
>  
>  		res_attr = pdev->res_attr_wc[i];
>  		if (res_attr) {
>  			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
>  			kfree(res_attr);
> +			pdev->res_attr_wc[i] = NULL;
>  		}
>  	}
>  }
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug
  2025-12-23 16:55   ` Bjorn Helgaas
@ 2025-12-24  1:28     ` duziming
  0 siblings, 0 replies; 16+ messages in thread
From: duziming @ 2025-12-24  1:28 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: bhelgaas, linux-pci, linux-kernel, chrisw, jbarnes,
	alex.williamson, liuyongqiang13


在 2025/12/24 0:55, Bjorn Helgaas 写道:
> Capitalize first word of subject to match history (see
> "git log --oneline drivers/pci/pci-sysfs.c")
>
> Drop "PCI" in "PCI hotplug" -- we already know this is PCI.
>
> On Tue, Dec 16, 2025 at 04:39:10PM +0800, Ziming Du wrote:
>> During the concurrent process of creating and rescanning in VF, the resource
>> files for the same "pci_dev" may be created twice. The second creation attempt
>> fails, resulting the "res_attr" in "pci_dev" to 'kfree', but the pointer is not
>> set to NULL. This will subsequently lead to dereferencing a null pointer when
>> removing the device.
> Wrap this to fit in 75 columns so it still fits in 80 when git log
> indents it.
>
> Drop quotes around struct and member names, e.g., pci_dev, res_attr.
>
> Drop '' around function names and add "()" after, e.g., kfree().
>
>> When we perform the following operation:
>> "echo $vfcount > /sys/class/net/"$pfname"/device/sriov_numvfs &
>>   sleep 0.5
>>   echo 1 > /sys/bus/pci/rescan
>>   pci_remove "$pfname" "
>> system will crash as follows:
> Indent quoted material two spaces and drop the "" around it, e.g.,
>
> When we perform the following operation:
>
>    echo $vfcount > /sys/class/net/"$pfname"/device/sriov_numvfs &
>    sleep 0.5
>    ...
>
> system will crash as follows.
>
>> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>> Mem abort info:
>> ESR = 0x0000000096000004
>> EC = 0x25: DABT (current EL), IL = 32 bits
>> SET = 0, FnV = 0
>> EA = 0, S1PTW = 0
>> FSC = 0x04: level 0 translation fault
>> Data abort info:
>> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> user pgtable: 4k pages, 48-bit VAs, pgdp=000020400d47b000
>> 0000000000000000 pgd=0000000000000000, p4d=0000000000000000
>> Internal error: Oops: 0000000096000004 #1 SMP
>> CPU: 115 PID: 13659 Comm: testEL_vf_resca Kdump: loaded Tainted: G W E 6.6.0 #9
>> Hardware name: Huawei TaiShan 2280 V2/BC82AMDD, BIOS 0.98 08/25/2019
>> pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : __pi_strlen+0x14/0x150
>> lr : kernfs_name_hash+0x24/0xa8
>> sp : ffff8001425c38f0
>> x29: ffff8001425c38f0 x28: ffff204021a21540 x27: 0000000000000000
>> x26: 0000000000000000 x25: 0000000000000000 x24: ffff20400f97fad0
>> x23: ffff20403145a0c0 x22: 0000000000000000 x21: 0000000000000000
>> x20: 0000000000000000 x19: 0000000000000000 x18: ffffffffffffffff
>> x17: 2f35322f38302038 x16: 392e3020534f4942 x15: 00000000fffffffd
>> x14: 0000000000000000 x13: 30643378302f3863 x12: 3378302b636e7973
>> x11: 00000000ffff7fff x10: 0000000000000000 x9 : ffff800100594b3c
>> x8 : 0101010101010101 x7 : 0000000000210d00 x6 : 67241f72241f7224
>> x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
>> x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
>> Call trace:
>> __pi_strlen+0x14/0x150
>> kernfs_find_ns+0x54/0x120
>> kernfs_remove_by_name_ns+0x58/0xf0
>> sysfs_remove_bin_file+0x24/0x38
>> pci_remove_resource_files+0x44/0x90
>> pci_remove_sysfs_dev_files+0x28/0x40
>> pci_stop_bus_device+0xb8/0x118
>> pci_stop_and_remove_bus_device+0x20/0x40
>> pci_iov_remove_virtfn+0xb8/0x138
>> sriov_disable+0xbc/0x190
>> pci_disable_sriov+0x30/0x48
>> hinic_pci_sriov_disable+0x54/0x138 [hinic]
>> hinic_remove+0x140/0x290 [hinic]
>> pci_device_remove+0x4c/0xf8
>> device_remove+0x54/0x90
>> device_release_driver_internal+0x1d4/0x238
>> device_release_driver+0x20/0x38
>> pci_stop_bus_device+0xa8/0x118
>> pci_stop_and_remove_bus_device_locked+0x28/0x50
>> remove_store+0x128/0x208
> Indent this quoted material two spaces and remove parts that aren't
> relevant.
>
>> Fix this by set the pointer to NULL after releasing 'res_attr' immediately.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Ziming Du <duziming2@huawei.com>
>> ---
>>   drivers/pci/pci-sysfs.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>> index c2df915ad2d2..7e697b82c5e1 100644
>> --- a/drivers/pci/pci-sysfs.c
>> +++ b/drivers/pci/pci-sysfs.c
>> @@ -1222,12 +1222,14 @@ static void pci_remove_resource_files(struct pci_dev *pdev)
>>   		if (res_attr) {
>>   			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
>>   			kfree(res_attr);
>> +			pdev->res_attr[i] = NULL;
>>   		}
>>   
>>   		res_attr = pdev->res_attr_wc[i];
>>   		if (res_attr) {
>>   			sysfs_remove_bin_file(&pdev->dev.kobj, res_attr);
>>   			kfree(res_attr);
>> +			pdev->res_attr_wc[i] = NULL;
>>   		}
>>   	}
>>   }
>> -- 
>> 2.43.0
Thanks for your correction. We will include them in the v2 patch

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-12-24  1:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-16  8:39 [PATCH 0/3] Miscellaneous fixes for pci subsystem Ziming Du
2025-12-16  8:39 ` [PATCH 1/3] PCI/sysfs: fix null pointer dereference during PCI hotplug Ziming Du
2025-12-23 16:55   ` Bjorn Helgaas
2025-12-24  1:28     ` duziming
2025-12-16  8:39 ` [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on non-x86 Ziming Du
2025-12-16 10:43   ` Ilpo Järvinen
2025-12-17  9:47     ` duziming
2025-12-17 10:15       ` Ilpo Järvinen
2025-12-18  8:03         ` duziming
2025-12-20 16:20   ` kernel test robot
2025-12-22  5:01   ` kernel test robot
2025-12-16  8:39 ` [PATCH 3/3] PCI: Prevent overflow in proc_bus_pci_write() Ziming Du
2025-12-16 10:57   ` Ilpo Järvinen
2025-12-17  9:33     ` duziming
2025-12-17 10:19       ` Ilpo Järvinen
2025-12-18  7:18         ` duziming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox