[PATCH 0/3] Remove obsolete RELOC_HIDE() macro from compiler-gcc.h

[PATCH 0/3] Remove obsolete RELOC_HIDE() macro from compiler-gcc.h

[Uros Bizjak]

The GCC specific macro was historically used to workaround very old
compiler bugs (including pre-4.1 ppc64 GCC). These compilers are long
obsolete.

The generic RELOC_HIDE() macro should be used instead.

The removal causes GCC to detect several reads from addresses below 4kB in
the boot code. GCC treats absolute addresses smaller than min-pagesize param
(defaulting to 4kB) as assumed results of pointer arithmetics from NULL.
The following code, when compiled with -O2 -Warray-bounds (included in -Wall):

  int foo (void) { return *(int *)0x123; }

will emit a rather cryptic warning:

warning: array subscript 0 is outside array bounds of ‘int[0]’ [-Warray-bounds=]
    1 | int foo (void) { return *(int *)0x123; }
      |                         ^~~~~~~~~~~~~
cc1: note: source object is likely at address zero

To solve the issue, replace open-coded inline assembly used for
FS/GS memory accesses in arch/x86/boot/boot.h with segment-qualified
pointer dereferences. The compiler allows pointer arithmetic from
NULL in __seg_fs and __seg_gs named address spaces.

The early boot environment does not guarantee any minimum page size,
so explicitly setting the minimum page size to zero by adding
--param=min-pagesize=0 to the compiler flags when building the x86
boot code with GCC inhibits warnings for addresses below 4kB.

The removal of the GCC specific macro results in the
following code size reduction:

     text    data     bss     dec     hex filename
  28526453        4823511  737108 34087072        20820a0 vmlinux-old.o
  28520945        4823463  737108 34081516        2080aec vmlinux-new.o

  ./bloat-o-meter vmlinux-old.o vmlinux-new.o
  add/remove: 4/14 grow/shrink: 189/674 up/down: 4433/-7865 (-3432)
  ...
  Total: Before=24103512, After=24100080, chg -0.01%

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>

Uros Bizjak (3):
  x86/boot: replace FS/GS inline asm with segment-qualified accesses
  x86/boot: disable GCC min-pagesize assumption in boot code
  compiler-gcc: Remove obsolete RELOC_HIDE() macro

 arch/x86/boot/Makefile       |  3 ++
 arch/x86/boot/boot.h         | 58 +++++++++++-------------------------
 include/linux/compiler-gcc.h | 25 ----------------
 3 files changed, 21 insertions(+), 65 deletions(-)

-- 
2.52.0

[PATCH 1/3] x86/boot: replace FS/GS inline asm with segment-qualified accesses

[Uros Bizjak]

GCC treats absolute addresses smaller than min-pagesize param
(defaulting to 4kB) as assumed results of pointer arithmetics from
NULL. The following code, when compiled with -O2 -Warray-bounds
(included in -Wall):

  int foo (void) { return *(int *)0x123; }

will emit a rather cryptic warning:

warning: array subscript 0 is outside array bounds of ‘int[0]’ [-Warray-bounds=]
    1 | int foo (void) { return *(int *)0x123; }
      |                         ^~~~~~~~~~~~~
cc1: note: source object is likely at address zero

Currently, the warning is supressed by the GCC specific RELOC_HIDE()
macro that obfuscates arithmetic on a variable address so that GCC
doesn't recognize the original var, and make assumptions about it.

The GCC specific RELOC_HIDE() macro was introduced to work around
certain ppc64 specific compiler bug in pre-4.1 GCC. This bug was
fixed long ago, and replacing GCC specific macro with a generic one
triggers the above warning in vga_recalc_vertical().

To solve the issue, replace open-coded inline assembly used for
FS/GS memory accesses in arch/x86/boot/boot.h with segment-qualified
pointer dereferences. The compiler allows pointer arithmetic from
NULL in __seg_fs and __seg_gs named address spaces. Using __seg_fs
and __seg_gs also simplifies the code, improves readability,
and allows the compiler to reason better about the memory accesses.

Explicit "memory" clobbers are also added to FS/GS segment register
updates and repe cmpsb helpers to prevent incorrect reordering.

No functional changes intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/boot/boot.h | 58 ++++++++++++++------------------------------
 1 file changed, 18 insertions(+), 40 deletions(-)

diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 8e3eab34dff4..6f39048f0481 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -53,7 +53,7 @@ static inline u16 ds(void)
 
 static inline void set_fs(u16 seg)
 {
-	asm volatile("movw %0,%%fs" : : "rm" (seg));
+	asm volatile("movw %0,%%fs" : : "rm" (seg) : "memory");
 }
 static inline u16 fs(void)
 {
@@ -64,7 +64,7 @@ static inline u16 fs(void)
 
 static inline void set_gs(u16 seg)
 {
-	asm volatile("movw %0,%%gs" : : "rm" (seg));
+	asm volatile("movw %0,%%gs" : : "rm" (seg) : "memory");
 }
 static inline u16 gs(void)
 {
@@ -77,78 +77,54 @@ typedef unsigned int addr_t;
 
 static inline u8 rdfs8(addr_t addr)
 {
-	u8 *ptr = (u8 *)absolute_pointer(addr);
-	u8 v;
-	asm volatile("movb %%fs:%1,%0" : "=q" (v) : "m" (*ptr));
-	return v;
+	return *(__seg_fs u8 *)(__force addr_t)absolute_pointer(addr);
 }
 static inline u16 rdfs16(addr_t addr)
 {
-	u16 *ptr = (u16 *)absolute_pointer(addr);
-	u16 v;
-	asm volatile("movw %%fs:%1,%0" : "=r" (v) : "m" (*ptr));
-	return v;
+	return *(__seg_fs u16 *)(__force addr_t)absolute_pointer(addr);
 }
 static inline u32 rdfs32(addr_t addr)
 {
-	u32 *ptr = (u32 *)absolute_pointer(addr);
-	u32 v;
-	asm volatile("movl %%fs:%1,%0" : "=r" (v) : "m" (*ptr));
-	return v;
+	return *(__seg_fs u32 *)(__force addr_t)absolute_pointer(addr);
 }
 
 static inline void wrfs8(u8 v, addr_t addr)
 {
-	u8 *ptr = (u8 *)absolute_pointer(addr);
-	asm volatile("movb %1,%%fs:%0" : "+m" (*ptr) : "qi" (v));
+	*(__seg_fs u8 *)(__force addr_t)absolute_pointer(addr) = v;
 }
 static inline void wrfs16(u16 v, addr_t addr)
 {
-	u16 *ptr = (u16 *)absolute_pointer(addr);
-	asm volatile("movw %1,%%fs:%0" : "+m" (*ptr) : "ri" (v));
+	*(__seg_fs u16 *)(__force addr_t)absolute_pointer(addr) = v;
 }
 static inline void wrfs32(u32 v, addr_t addr)
 {
-	u32 *ptr = (u32 *)absolute_pointer(addr);
-	asm volatile("movl %1,%%fs:%0" : "+m" (*ptr) : "ri" (v));
+	*(__seg_fs u32 *)(__force addr_t)absolute_pointer(addr) = v;
 }
 
 static inline u8 rdgs8(addr_t addr)
 {
-	u8 *ptr = (u8 *)absolute_pointer(addr);
-	u8 v;
-	asm volatile("movb %%gs:%1,%0" : "=q" (v) : "m" (*ptr));
-	return v;
+	return *(__seg_gs u8 *)(__force addr_t)absolute_pointer(addr);
 }
 static inline u16 rdgs16(addr_t addr)
 {
-	u16 *ptr = (u16 *)absolute_pointer(addr);
-	u16 v;
-	asm volatile("movw %%gs:%1,%0" : "=r" (v) : "m" (*ptr));
-	return v;
+	return *(__seg_gs u16 *)(__force addr_t)absolute_pointer(addr);
 }
 static inline u32 rdgs32(addr_t addr)
 {
-	u32 *ptr = (u32 *)absolute_pointer(addr);
-	u32 v;
-	asm volatile("movl %%gs:%1,%0" : "=r" (v) : "m" (*ptr));
-	return v;
+	return *(__seg_gs u32 *)(__force addr_t)absolute_pointer(addr);
 }
 
 static inline void wrgs8(u8 v, addr_t addr)
 {
-	u8 *ptr = (u8 *)absolute_pointer(addr);
-	asm volatile("movb %1,%%gs:%0" : "+m" (*ptr) : "qi" (v));
+	*(__seg_gs u8 *)(__force addr_t)absolute_pointer(addr) = v;
 }
 static inline void wrgs16(u16 v, addr_t addr)
 {
-	u16 *ptr = (u16 *)absolute_pointer(addr);
-	asm volatile("movw %1,%%gs:%0" : "+m" (*ptr) : "ri" (v));
+	*(__seg_gs u16 *)(__force addr_t)absolute_pointer(addr) = v;
 }
 static inline void wrgs32(u32 v, addr_t addr)
 {
-	u32 *ptr = (u32 *)absolute_pointer(addr);
-	asm volatile("movl %1,%%gs:%0" : "+m" (*ptr) : "ri" (v));
+	*(__seg_gs u32 *)(__force addr_t)absolute_pointer(addr) = v;
 }
 
 /* Note: these only return true/false, not a signed return value! */
@@ -156,14 +132,16 @@ static inline bool memcmp_fs(const void *s1, addr_t s2, size_t len)
 {
 	bool diff;
 	asm volatile("fs repe cmpsb"
-		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
+		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
+		     : : "memory");
 	return diff;
 }
 static inline bool memcmp_gs(const void *s1, addr_t s2, size_t len)
 {
 	bool diff;
 	asm volatile("gs repe cmpsb"
-		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
+		     : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
+		     : : "memory");
 	return diff;
 }
 
-- 
2.52.0

[PATCH 2/3] x86/boot: disable GCC min-pagesize assumption in boot code

[Uros Bizjak]

GCC treats absolute addresses smaller than min-pagesize param
(defaulting to 4kB) as assumed results of pointer arithmetics from
NULL. The following code, when compiled with -O2 -Warray-bounds
(included in -Wall):

  int foo (void) { return *(int *)0x123; }

will emit a rather cryptic warning:

warning: array subscript 0 is outside array bounds of ‘int[0]’ [-Warray-bounds=]
    1 | int foo (void) { return *(int *)0x123; }
      |                         ^~~~~~~~~~~~~
cc1: note: source object is likely at address zero

Currently, the warning is supressed by the GCC specific RELOC_HIDE()
macro that obfuscates arithmetic on a variable address so that GCC
doesn't recognize the original var, and make assumptions about it.

The GCC specific RELOC_HIDE() macro was introduced to work around
certain ppc64 specific compiler bug in pre-4.1 GCC. This bug was
fixed long ago, and replacing GCC specific macro with a generic one
triggers the above warning in copy_boot_params().

The early boot environment does not guarantee any minimum page size,
so explicitly setting the minimum page size to zero by adding
--param=min-pagesize=0 to the compiler flags when building the x86
boot code with GCC inhibits warnings for addresses below 4kB.

The option is guarded by CONFIG_CC_IS_GCC since it is GCC-specific.

No functional changes intended.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
 arch/x86/boot/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 3f9fb3698d66..372e67d2a855 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -55,6 +55,9 @@ KBUILD_CFLAGS	:= $(REALMODE_CFLAGS) -D_SETUP
 KBUILD_AFLAGS	:= $(KBUILD_CFLAGS) -D__ASSEMBLY__
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables
 KBUILD_CFLAGS	+= $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
+ifdef CONFIG_CC_IS_GCC
+KBUILD_CFLAGS	+= $(call cc-option,--param=min-pagesize=0)
+endif
 
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
-- 
2.52.0

[PATCH 3/3] compiler-gcc: Remove obsolete RELOC_HIDE() macro

[Uros Bizjak]

Remove the RELOC_HIDE() macro from include/linux/compiler-gcc.h.

The GCC specific macro was historically used to workaround very old
compiler bugs (including pre-4.1 ppc64 GCC). These compilers are long
obsolete.

The generic RELOC_HIDE() macro should be used instead.

The removal of the GCC specific macro results in the
following code size reduction:

     text    data     bss     dec     hex filename
  28526453        4823511  737108 34087072        20820a0 vmlinux-old.o
  28520945        4823463  737108 34081516        2080aec vmlinux-new.o

  ./bloat-o-meter vmlinux-old.o vmlinux-new.o
  add/remove: 4/14 grow/shrink: 189/674 up/down: 4433/-7865 (-3432)
  ...
  Total: Before=24103512, After=24100080, chg -0.01%

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 include/linux/compiler-gcc.h | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 5de824a0b3d7..081e658754b9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -10,31 +10,6 @@
 		     + __GNUC_MINOR__ * 100	\
 		     + __GNUC_PATCHLEVEL__)
 
-/*
- * This macro obfuscates arithmetic on a variable address so that gcc
- * shouldn't recognize the original var, and make assumptions about it.
- *
- * This is needed because the C standard makes it undefined to do
- * pointer arithmetic on "objects" outside their boundaries and the
- * gcc optimizers assume this is the case. In particular they
- * assume such arithmetic does not wrap.
- *
- * A miscompilation has been observed because of this on PPC.
- * To work around it we hide the relationship of the pointer and the object
- * using this macro.
- *
- * Versions of the ppc64 compiler before 4.1 had a bug where use of
- * RELOC_HIDE could trash r30. The bug can be worked around by changing
- * the inline assembly constraint from =g to =r, in this particular
- * case either is valid.
- */
-#define RELOC_HIDE(ptr, off)						\
-({									\
-	unsigned long __ptr;						\
-	__asm__ ("" : "=r"(__ptr) : "0"(ptr));				\
-	(typeof(ptr)) (__ptr + (off));					\
-})
-
 #if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
 #define __latent_entropy __attribute__((latent_entropy))
 #endif
-- 
2.52.0