Re: [PATCH] PCI/sysfs: Prohibit unaligned access to I/O port

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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

On Wed, 8 Apr 2026, Ilpo Järvinen wrote:

> On Wed, 8 Apr 2026, Ziming Du wrote:
> 
> > 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
> > 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
> >  vfs_write+0x7bc/0xac8 fs/read_write.c:586
> >  ksys_write+0x12c/0x270 fs/read_write.c:639
> >  __arm64_sys_write+0x78/0xb8 fs/read_write.c:648
> > 
> > Although x86 might handle unaligned I/O accesses by splitting cycles,
> > this approach is still limited because PCI device registers typically
> > expect natural alignment. A global prohibition of unaligned accesses
> > ensures consistent behavior across all architectures and prevents
> > unexpected hardware side effects.
> > 
> > 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>
> > Link: https://lore.kernel.org/all/20260116081723.1603603-1-duziming2@huawei.com/
> > Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/pci/pci-sysfs.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > index 16eaaf749ba97..c88910bcad262 100644
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -31,6 +31,7 @@
> >  #include <linux/of.h>
> >  #include <linux/aperture.h>
> >  #include <linux/unaligned.h>
> > +#include <linux/align.h>
> 
> Not exactly the best place for adding this but since these headers are not 
> yet in the alphabetical order, not an end of the world (I'd have put it 
> first to avoid having to reorder it when sorting the headers).
> 
> >  #include "pci.h"
> >  
> >  #ifndef ARCH_PCI_DEV_GROUPS
> > @@ -1157,6 +1158,9 @@ static ssize_t pci_resource_io(struct file *filp, struct kobject *kobj,
> >  	if (port + count - 1 > pci_resource_end(pdev, bar))
> >  		return -EINVAL;
> >  
> > +	if (!IS_ALIGNED(port, count))
> 


> Hmm, your commit message talks about "offset" but this doesn't check "off" 
> but "count", aren't those two different things?
> 
> ...What's also odd is that you seem to have this same thing already in v1 
> so I'm not following why this ends up solving the mentioned issue.

Nevermind this comment. I was just badly confused myself ('port' is 
assigned 'off' outside the visible context, and my 'count' vs 'off' 
related talk is just plain wrong).

Except for the header order mentioned above, this seems fine,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>


-- 
 i.