* [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer
@ 2022-12-05 8:30 Song Chen
2022-12-05 10:04 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Song Chen @ 2022-12-05 8:30 UTC (permalink / raw)
To: rostedt, mhiramat, arnd
Cc: linux-kernel, linux-trace-kernel, linux-arch, Song Chen
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@189.cn>
Reported-by: kernel test robot <lkp@intel.com>
---
include/asm-generic/io.h | 45 +++++++++++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 12 deletions(-)
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index a68f8fbf423b..394538fd2585 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -542,9 +542,10 @@ static inline void writesq(volatile void __iomem *addr, const void *buffer,
static inline u8 _inb(unsigned long addr)
{
u8 val;
+ void *pci_iobase = PCI_IOBASE;
__io_pbr();
- val = __raw_readb(PCI_IOBASE + addr);
+ val = __raw_readb(pci_iobase + addr);
__io_par(val);
return val;
}
@@ -555,9 +556,10 @@ static inline u8 _inb(unsigned long addr)
static inline u16 _inw(unsigned long addr)
{
u16 val;
+ void *pci_iobase = PCI_IOBASE;
__io_pbr();
- val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
+ val = __le16_to_cpu((__le16 __force)__raw_readw(pci_iobase + addr));
__io_par(val);
return val;
}
@@ -568,9 +570,10 @@ static inline u16 _inw(unsigned long addr)
static inline u32 _inl(unsigned long addr)
{
u32 val;
+ void *pci_iobase = PCI_IOBASE;
__io_pbr();
- val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
+ val = __le32_to_cpu((__le32 __force)__raw_readl(pci_iobase + addr));
__io_par(val);
return val;
}
@@ -580,8 +583,10 @@ static inline u32 _inl(unsigned long addr)
#define _outb _outb
static inline void _outb(u8 value, unsigned long addr)
{
+ void *pci_iobase = PCI_IOBASE;
+
__io_pbw();
- __raw_writeb(value, PCI_IOBASE + addr);
+ __raw_writeb(value, pci_iobase + addr);
__io_paw();
}
#endif
@@ -590,8 +595,10 @@ static inline void _outb(u8 value, unsigned long addr)
#define _outw _outw
static inline void _outw(u16 value, unsigned long addr)
{
+ void *pci_iobase = PCI_IOBASE;
+
__io_pbw();
- __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
+ __raw_writew((u16 __force)cpu_to_le16(value), pci_iobase + addr);
__io_paw();
}
#endif
@@ -600,8 +607,10 @@ static inline void _outw(u16 value, unsigned long addr)
#define _outl _outl
static inline void _outl(u32 value, unsigned long addr)
{
+ void *pci_iobase = PCI_IOBASE;
+
__io_pbw();
- __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
+ __raw_writel((u32 __force)cpu_to_le32(value), pci_iobase + addr);
__io_paw();
}
#endif
@@ -689,7 +698,9 @@ 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);
+ void *pci_iobase = PCI_IOBASE;
+
+ readsb(pci_iobase + addr, buffer, count);
}
#endif
@@ -697,7 +708,9 @@ 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);
+ void *pci_iobase = PCI_IOBASE;
+
+ readsw(pci_iobase + addr, buffer, count);
}
#endif
@@ -705,7 +718,9 @@ 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);
+ void *pci_iobase = PCI_IOBASE;
+
+ readsl(pci_iobase + addr, buffer, count);
}
#endif
@@ -714,7 +729,9 @@ 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);
+ void *pci_iobase = PCI_IOBASE;
+
+ writesb(pci_iobase + addr, buffer, count);
}
#endif
@@ -723,7 +740,9 @@ 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);
+ void *pci_iobase = PCI_IOBASE;
+
+ writesw(pci_iobase + addr, buffer, count);
}
#endif
@@ -732,7 +751,9 @@ 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);
+ void *pci_iobase = PCI_IOBASE;
+
+ writesl(pci_iobase + addr, buffer, count);
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer
2022-12-05 8:30 [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer Song Chen
@ 2022-12-05 10:04 ` Arnd Bergmann
2022-12-06 6:01 ` Song Chen
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2022-12-05 10:04 UTC (permalink / raw)
To: Song Chen, Steven Rostedt, Masami Hiramatsu, Niklas Schnelle
Cc: linux-kernel, linux-trace-kernel, Linux-Arch
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@189.cn>
> Reported-by: kernel test robot <lkp@intel.com>
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@linux.ibm.com/
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer
2022-12-05 10:04 ` Arnd Bergmann
@ 2022-12-06 6:01 ` Song Chen
2022-12-06 14:22 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Song Chen @ 2022-12-06 6:01 UTC (permalink / raw)
To: Arnd Bergmann, Steven Rostedt, Masami Hiramatsu, Niklas Schnelle
Cc: linux-kernel, linux-trace-kernel, Linux-Arch
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@189.cn>
>> Reported-by: kernel test robot <lkp@intel.com>
>
> 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@linux.ibm.com/
>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer
2022-12-06 6:01 ` Song Chen
@ 2022-12-06 14:22 ` Arnd Bergmann
0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2022-12-06 14:22 UTC (permalink / raw)
To: Song Chen, Steven Rostedt, Masami Hiramatsu, Niklas Schnelle
Cc: linux-kernel, linux-trace-kernel, Linux-Arch
On Tue, Dec 6, 2022, at 07:01, Song Chen wrote:
> 在 2022/12/5 18:04, Arnd Bergmann 写道:
>> On Mon, Dec 5, 2022, at 09:30, Song Chen wrote:
>>
>> We have discussed this bit multiple times, and Niklas Schnelle
>> last posted his series to fix this as an RFC in [1].
>>
>
> 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.
hexagon/riscv/s390 should not implement inb(), there is no reason
for that because no hardware uses it. Half of the other architectures
that currently implement inb() should not do so either.
> Introducing HAS_IOPORT to trace seems no necessary and too much impact.
I don't think that trace has anything to do with it, the asm-generic
header should just not provde the inb() interface on architectures
that don't use it.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-06 14:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-05 8:30 [PATCH v3 3/4] include/asm-generic/io.h: remove performing pointer arithmetic on a null pointer Song Chen
2022-12-05 10:04 ` Arnd Bergmann
2022-12-06 6:01 ` Song Chen
2022-12-06 14:22 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox