Re: [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer

[Song Chen]

Hi,

在 2022/12/5 18:04, Arnd Bergmann 写道:
> On Mon, Dec 5, 2022, at 09:30, Song Chen wrote:
> > kernel test robot reports below warnings:
> >
> >     In file included from kernel/trace/trace_events_synth.c:18:
> >     In file included from include/linux/trace_events.h:9:
> >     In file included from include/linux/hardirq.h:11:
> >     In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
> >     In file included from include/asm-generic/hardirq.h:17:
> >     In file included from include/linux/irq.h:20:
> >     In file included from include/linux/io.h:13:
> >     In file included from arch/hexagon/include/asm/io.h:334:
> >     include/asm-generic/io.h:547:31: warning: performing pointer arithmetic
> > 	on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> >             val = __raw_readb(PCI_IOBASE + addr);
> >                               ~~~~~~~~~~ ^
> >     include/asm-generic/io.h:560: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));
> >                                                             ~~~~~~~~~~ ^
> >     include/uapi/linux/byteorder/little_endian.h:37:51: note:
> > 		expanded from macro '__le16_to_cpu'
> >     #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
> >
> > The reason could be constant literal zero converted to any pointer type decays
> > into the null pointer constant.
> >
> > I'm not sure why those warnings are only triggered when building hexagon instead
> > of x86 or arm, but anyway, i found a work around:
> >
> > 	void *pci_iobase = PCI_IOBASE;
> > 	val = __raw_readb(pci_iobase + addr);
> >
> > The pointer is not evaluated at compile time, so the warnings are removed.
> >
> > Signed-off-by: Song Chen <chensong_2000@xxxxxx>
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
>
> The code is still wrong, you just hide the warning, so no, this is
> not a correct fix. When PCI_IOBASE is NULL, any call to
> inb() etc is a NULL pointer dereference that immediately crashes
> the kernel, so the correct solution is to not allow building code
> that uses port I/O on kernels that are configured not to
> support port I/O.
>
> We have discussed this bit multiple times, and Niklas Schnelle
> last posted his series to fix this as an RFC in [1].
>
>        Arnd
>
> [1] https://lore.kernel.org/lkml/20220429135108.2781579-1-schnelle@xxxxxxxxxxxxx/

Trace triggers the warning accidentally by including io.h indirectly 
because of the absence of PCI_IOBASE in hexagon. So what trace can do in 
this case is either to suppress warning or just ignore it, the warning 
will go away as long as hexagon has put PCI_IOBASE in place or 
implemented its own inb() etc, i think they will do it sooner or later.

Introducing HAS_IOPORT to trace seems no necessary and too much impact.

/Song