* [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
* 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
* [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
* 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 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 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
* [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 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 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
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