x86/vdso: Remove retpoline flags

From: Borislav Petkov <[email protected]>

So this does not fix an existing problem but a hypothetical one. The below fires on an old Frankenstein distro kernel and I'm sending it only because it is obviously The Right Thing(tm). And in case we change things in the vdso in the future and thus manage to generate an indirect call for whatever reason...

So on those older vdsos, the compiler can generate indirect calls to the retpoline thunks, which manifests itself as:

  /usr/lib64/gcc/x86_64-suse-linux/4.8/../../../../x86_64-suse-linux/bin/ld: arch/x86/vdso/vclock_gettime.o: \
	  relocation R_X86_64_PC32 against undefined symbol `__x86_indirect_thunk_rax' can not be used when \
	  making a shared object; recompile with -fPIC

/usr/lib64/gcc/x86_64-suse-linux/4.8/../../../../x86_64-suse-linux/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status
nm: 'arch/x86/vdso/vdso.so.dbg': No such file
objcopy: 'arch/x86/vdso/vdso.so.dbg': No such file
make[2]: *** [arch/x86/vdso/vdso.so] Error 1
make[1]: *** [arch/x86/vdso] Error 2
make[1]: *** Waiting for unfinished jobs....

However, since the vdso is not part of the kernel, it cannot reference kernel symbols so we better not build with with retpoline CFLAGS either.

Thus, export RETPOLINE_CFLAGS to the lower level Makefiles so that they can be filtered out in vdso's Makefile.

---
 arch/x86/Makefile            | 6 ++++--
 arch/x86/entry/vdso/Makefile | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

index fad55160dcb9..fe7adbc6a380 100644
@@ -232,9 +232,11 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 
 # Avoid indirect branches in kernel to deal with Spectre
 ifdef CONFIG_RETPOLINE
-    RETPOLINE_CFLAGS += $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
+    RETPOLINE_CFLAGS := $(call cc-option,-mindirect-branch=thunk-extern -mindirect-branch-register)
     ifneq ($(RETPOLINE_CFLAGS),)
-        KBUILD_CFLAGS += $(RETPOLINE_CFLAGS) -DRETPOLINE
+        RETPOLINE_CFLAGS += -DRETPOLINE
+	KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
+	export RETPOLINE_CFLAGS
     endif
 endif
 

index 1943aebadede..991ae9af5a00 100644
@@ -30,6 +30,11 @@ vdso_img-$(VDSO32-y)		+= 32
 
 obj-$(VDSO32-y)			+= vdso32-setup.o
 
+
+ifdef CONFIG_RETPOLINE
+	KBUILD_CFLAGS := $(filter-out $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS))
+endif
+
 vobjs := $(foreach F,$(vobjs-y),$(obj)/$F)
 
 $(obj)/vdso.o: $(obj)/vdso.so

2.13.0

Hmm. I'm okay with this, but I'd also be okay doing nothing and figuring out WTF happened if an upstream kernel fails to build like this.

--Andy

Oh sure, I'm sending it only as an FYI to show that something like this *might* happen so that we're aware. I've taken it into our trees where the 3.0 vdso code generates an indirect call to the thunk:

        .loc 1 41 0
        movq    -10489696, %rax # MEM[(const struct vsyscall_gtod_data *)-10489728B].clock.vread, MEM[(const struct vsyscall_gtod_data *)-10489728B].clock.vread
        call    __x86_indirect_thunk_rax

Without the retpoline flags, the code looks like this:

 170:   4c 8b 34 25 88 f0 5f    mov    0xffffffffff5ff088,%r14
 177:   ff 
 178:   44 8b 2c 25 90 f0 5f    mov    0xffffffffff5ff090,%r13d
 17f:   ff 
 180:   ff 14 25 a0 f0 5f ff    callq  *0xffffffffff5ff0a0		<---
 187:   4c 8b 0c 25 a8 f0 5f    mov    0xffffffffff5ff0a8,%r9
 18e:   ff 
 18f:   4c 8b 04 25 b0 f0 5f    mov    0xffffffffff5ff0b0,%r8

which is:

        movl    -10489712, %r12d        # MEM[(const struct vsyscall_gtod_data *)-10489728B].wall_time_nsec,
.LVL46:
.LBB125:
.LBB126:
        .loc 1 41 0
        call    *-10489696      # MEM[(const struct vsyscall_gtod_data *)-10489728B].clock.vread		<---
.LVL47:
        movq    -10489688, %r9  # MEM[(const struct vsyscall_gtod_data *)-10489728B].clock.cycle_last, D.23457
        movq    -10489680, %r8  # MEM[(const struct vsyscall_gtod_data *)-10489728B].clock.mask, D.23457

notrace static inline long vgetns(void)

{
        long v;
        cycles_t (*vread)(void);
        vread = gtod->clock.vread;
        v = (vread() - gtod->clock.cycle_last) & gtod->clock.mask;
	^^^^^^^^^^^^

so it has been converted to an absolute memory reference in that CALL - nothing funky through a register.

Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.