| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0d723f79-6330-4cb8-95bb-aeaebaf820dd@huawei.com>
Date: Thu, 18 Dec 2025 16:03:48 +0800
From: duziming <duziming2@...wei.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: <bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>, LKML
<linux-kernel@...r.kernel.org>, <chrisw@...hat.com>,
<jbarnes@...tuousgeek.org>, <alex.williamson@...hat.com>,
<liuyongqiang13@...wei.com>
Subject: Re: [PATCH 2/3] PCI/sysfs: Prohibit unaligned access to I/O port on
non-x86
在 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@...wei.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@...wei.com>
>>>> Signed-off-by: Ziming Du <duziming2@...wei.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)
>>>>
Powered by blists - more mailing lists