-Wnull-pointer-arithmetic in include/asm-generic/io.h

[nathanchance]

fs/btrfs has enabled W=1 for themselves (commit), which results in 1000+ warnings on architectures where PCI_IOBASE is not defined (as it evaluates to a NULL pointer). Example:

In file included from /builds/1niBayWPViIGDJ1NJeZ2RJH0siz/fs/btrfs/super.c:6:
In file included from /builds/1niBayWPViIGDJ1NJeZ2RJH0siz/include/linux/blkdev.h:26:
In file included from /builds/1niBayWPViIGDJ1NJeZ2RJH0siz/include/linux/scatterlist.h:9:
In file included from /builds/1niBayWPViIGDJ1NJeZ2RJH0siz/arch/s390/include/asm/io.h:80:
/builds/1niBayWPViIGDJ1NJeZ2RJH0siz/include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
        val = __raw_readb(PCI_IOBASE + addr);
                          ~~~~~~~~~~ ^
/builds/1niBayWPViIGDJ1NJeZ2RJH0siz/include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
        val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                        ~~~~~~~~~~ ^
/builds/1niBayWPViIGDJ1NJeZ2RJH0siz/include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                          ^
/builds/1niBayWPViIGDJ1NJeZ2RJH0siz/include/uapi/linux/swab.h:105:32: note: expanded from macro '__swab16'
        (__builtin_constant_p((__u16)(x)) ?     \
                                      ^

Full build log: https://builds.tuxbuild.com/1niBayWPViIGDJ1NJeZ2RJH0siz/build.log

Really ugly fix:

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index c6af40ce03be..2060e23e080d 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -444,6 +444,8 @@ static inline void writesq(volatile void __iomem *addr, const void *buffer,
 #define PCI_IOBASE ((void __iomem *)0)
 #endif

+#define PCI_IOBASE_ADD(val) ((void __iomem *)((uintptr_t)PCI_IOBASE + val))
+
 #ifndef IO_SPACE_LIMIT
 #define IO_SPACE_LIMIT 0xffff
 #endif
@@ -461,7 +463,7 @@ static inline u8 _inb(unsigned long addr)
    u8 val;

    __io_pbr();
-   val = __raw_readb(PCI_IOBASE + addr);
+   val = __raw_readb(PCI_IOBASE_ADD(addr));
    __io_par(val);
    return val;
 }
@@ -474,7 +476,7 @@ static inline u16 _inw(unsigned long addr)
    u16 val;

    __io_pbr();
-   val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
+   val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE_ADD(addr)));
    __io_par(val);
    return val;
 }
@@ -487,7 +489,7 @@ static inline u32 _inl(unsigned long addr)
    u32 val;

    __io_pbr();
-   val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
+   val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE_ADD(addr)));
    __io_par(val);
    return val;
 }
@@ -498,7 +500,7 @@ static inline u32 _inl(unsigned long addr)
 static inline void _outb(u8 value, unsigned long addr)
 {
    __io_pbw();
-   __raw_writeb(value, PCI_IOBASE + addr);
+   __raw_writeb(value, PCI_IOBASE_ADD(addr));
    __io_paw();
 }
 #endif
@@ -508,7 +510,7 @@ static inline void _outb(u8 value, unsigned long addr)
 static inline void _outw(u16 value, unsigned long addr)
 {
    __io_pbw();
-   __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
+   __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE_ADD(addr));
    __io_paw();
 }
 #endif
@@ -518,7 +520,7 @@ static inline void _outw(u16 value, unsigned long addr)
 static inline void _outl(u32 value, unsigned long addr)
 {
    __io_pbw();
-   __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
+   __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE_ADD(addr));
    __io_paw();
 }
 #endif
@@ -606,7 +608,7 @@ static inline void outl_p(u32 value, unsigned long addr)
 #define insb insb
 static inline void insb(unsigned long addr, void *buffer, unsigned int count)
 {
-   readsb(PCI_IOBASE + addr, buffer, count);
+   readsb(PCI_IOBASE_ADD(addr), buffer, count);
 }
 #endif

@@ -614,7 +616,7 @@ static inline void insb(unsigned long addr, void *buffer, unsigned int count)
 #define insw insw
 static inline void insw(unsigned long addr, void *buffer, unsigned int count)
 {
-   readsw(PCI_IOBASE + addr, buffer, count);
+   readsw(PCI_IOBASE_ADD(addr), buffer, count);
 }
 #endif

@@ -622,7 +624,7 @@ static inline void insw(unsigned long addr, void *buffer, unsigned int count)
 #define insl insl
 static inline void insl(unsigned long addr, void *buffer, unsigned int count)
 {
-   readsl(PCI_IOBASE + addr, buffer, count);
+   readsl(PCI_IOBASE_ADD(addr), buffer, count);
 }
 #endif

@@ -631,7 +633,7 @@ static inline void insl(unsigned long addr, void *buffer, unsigned int count)
 static inline void outsb(unsigned long addr, const void *buffer,
             unsigned int count)
 {
-   writesb(PCI_IOBASE + addr, buffer, count);
+   writesb(PCI_IOBASE_ADD(addr), buffer, count);
 }
 #endif

@@ -640,7 +642,7 @@ static inline void outsb(unsigned long addr, const void *buffer,
 static inline void outsw(unsigned long addr, const void *buffer,
             unsigned int count)
 {
-   writesw(PCI_IOBASE + addr, buffer, count);
+   writesw(PCI_IOBASE_ADD(addr), buffer, count);
 }
 #endif

@@ -649,7 +651,7 @@ static inline void outsw(unsigned long addr, const void *buffer,
 static inline void outsl(unsigned long addr, const void *buffer,
             unsigned int count)
 {
-   writesl(PCI_IOBASE + addr, buffer, count);
+   writesl(PCI_IOBASE_ADD(addr), buffer, count);
 }
 #endif

cc @arndb as I know you have fixed some of these in the past.

[arndb]

Any idea which architectures this happens on? There are two possible ways we might get around this:

a) a lot of architectures do not support I/O port accesses at all and should not define these as pointer dereferences in the first place

b) using a NULL pointer as the I/O port base is generally a bad idea as well, as it leads to a misplaced port number causing random data corruption, and they should define a proper base in virtual memory

[nathanchance]

@arndb I can for sure see this for s390. I have seen certain instances where riscv and powerpc generate warnings such as these but it looks like from files other than include/asm-generic/io.h.

[arndb]

s390 does not have any support for I/O ports, and just returns NULL from ioport_map(), while ignoring any IORESOURCE_IO BARs on a PCI device, so in that case, we could put anything in there. I would suggest adding a stub function that contains a WARN_ON(1) and just returns all-ones.

[nathanchance]

Hmmm I am not sure I understand the suggestion. All of these warnings are in the IO access functions, which are used even when ioport_map returns NULL, right?

[arndb]

The point is that actually calling any of these functions on s390 will result in undefined behavior because it actually does a NULL pointer dereference. What I would suggest doing instead is to rewrite the I/O port accessors to either get hidden, or to replace them with a function that does a WARN_ONCE() but does not try to pass the pointer into readl/writel

[nathanchance]

https://lore.kernel.org/r/20210421111759.2059976-1-schnelle@linux.ibm.com/

[nickdesaulniers]

v6: https://lore.kernel.org/lkml/20210510145234.594814-1-schnelle@linux.ibm.com/

[nickdesaulniers]

@arndb do you have cycles to rereview v6 above?

[nickdesaulniers]

I can see these in -next as: a5f7166b58cda2430123eb9bb96d60340e699ae4 78924148a3d22e030fe8f5c1a0ce10e177856423 5ae6eadfdaf431f47adbdf1754f3b5a5fd638de2 with @arndb 's SOB, though I'm not sure which tree this is flowing into -next via.

[nathanchance]

Niklas's solution was rejected at pull time: https://lore.kernel.org/r/CAK8P3a2oZ-+qd3Nhpy9VVXCJB3DU5N-y-ta2JpP0t6NHh=GVXw@mail.gmail.com/

[nathanchance]

This now affects Hexagon after commits 81c0386c1376 ("regmap: mmio: Support accelerared noinc operations") and f8f60615379c ("regmap/hexagon: Properly fix the generic IO helpers") :/

https://builds.tuxbuild.com/2DlDBj7YHaFv8PfY24gTGrHwKmq/build.log

[nickdesaulniers]

I think this is the latest series for this: https://lore.kernel.org/all/20230522105049.1467313-1-schnelle@linux.ibm.com/