diff mbox

[5/7] x86: Insert endbr32/endbr64 to vDSO

Message ID 20180607143855.3681-6-yu-cheng.yu@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yu-cheng Yu June 7, 2018, 2:38 p.m. UTC
From: "H.J. Lu" <hjl.tools@gmail.com>

When Intel indirect branch tracking is enabled, functions in vDSO which
may be called indirectly should have endbr32 or endbr64 as the first
instruction.  We try to compile vDSO with -fcf-protection=branch -mibt
if possible.  Otherwise, we insert endbr32 or endbr64 by hand to assembly
codes generated by the compiler.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 arch/x86/entry/vdso/.gitignore |  4 ++++
 arch/x86/entry/vdso/Makefile   | 34 ++++++++++++++++++++++++++++++++++
 arch/x86/entry/vdso/endbr.sh   | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+)
 create mode 100644 arch/x86/entry/vdso/endbr.sh

Comments

Andy Lutomirski June 7, 2018, 8:50 p.m. UTC | #1
On Thu, Jun 7, 2018 at 7:42 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> When Intel indirect branch tracking is enabled, functions in vDSO which
> may be called indirectly should have endbr32 or endbr64 as the first
> instruction.  We try to compile vDSO with -fcf-protection=branch -mibt
> if possible.  Otherwise, we insert endbr32 or endbr64 by hand to assembly
> codes generated by the compiler.

Wow, that's... a genuine abomination.  Do we really need to support
CET on kernels built with old toolchains?

--Andy
H.J. Lu June 7, 2018, 10:03 p.m. UTC | #2
On Thu, Jun 7, 2018 at 1:50 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Thu, Jun 7, 2018 at 7:42 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>>
>> When Intel indirect branch tracking is enabled, functions in vDSO which
>> may be called indirectly should have endbr32 or endbr64 as the first
>> instruction.  We try to compile vDSO with -fcf-protection=branch -mibt
>> if possible.  Otherwise, we insert endbr32 or endbr64 by hand to assembly
>> codes generated by the compiler.
>
> Wow, that's... a genuine abomination.  Do we really need to support
> CET on kernels built with old toolchains?
>

Yes.  GCC 7 should be able to build CET kernel.
Andy Lutomirski June 7, 2018, 11 p.m. UTC | #3
On Thu, Jun 7, 2018 at 3:03 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jun 7, 2018 at 1:50 PM, Andy Lutomirski <luto@kernel.org> wrote:
> > On Thu, Jun 7, 2018 at 7:42 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >>
> >> From: "H.J. Lu" <hjl.tools@gmail.com>
> >>
> >> When Intel indirect branch tracking is enabled, functions in vDSO which
> >> may be called indirectly should have endbr32 or endbr64 as the first
> >> instruction.  We try to compile vDSO with -fcf-protection=branch -mibt
> >> if possible.  Otherwise, we insert endbr32 or endbr64 by hand to assembly
> >> codes generated by the compiler.
> >
> > Wow, that's... a genuine abomination.  Do we really need to support
> > CET on kernels built with old toolchains?
> >
>
> Yes.  GCC 7 should be able to build CET kernel.
>

Why?  Presumably people running distros that use CET are going to have
kernels build with a CET-supporting compiler.

If we really really need this patch, then I want some kind of
assurance that selftests will catch the failure if something breaks it
or a new vDSO entry point is added.  But my inclination is to NAK this
patch and let the distros carry it if they really really want it.  As
it stands, this sucks for maintainability.
H.J. Lu June 8, 2018, 12:31 a.m. UTC | #4
On Thu, Jun 7, 2018 at 4:00 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Thu, Jun 7, 2018 at 3:03 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Thu, Jun 7, 2018 at 1:50 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> > On Thu, Jun 7, 2018 at 7:42 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>> >>
>> >> From: "H.J. Lu" <hjl.tools@gmail.com>
>> >>
>> >> When Intel indirect branch tracking is enabled, functions in vDSO which
>> >> may be called indirectly should have endbr32 or endbr64 as the first
>> >> instruction.  We try to compile vDSO with -fcf-protection=branch -mibt
>> >> if possible.  Otherwise, we insert endbr32 or endbr64 by hand to assembly
>> >> codes generated by the compiler.
>> >
>> > Wow, that's... a genuine abomination.  Do we really need to support
>> > CET on kernels built with old toolchains?
>> >
>>
>> Yes.  GCC 7 should be able to build CET kernel.
>>
>
> Why?  Presumably people running distros that use CET are going to have
> kernels build with a CET-supporting compiler.
>

Good point.  It was needed before GCC 8 was released.   We can drop
arch/x86/entry/vdso/endbr.sh now.
Andy Lutomirski June 8, 2018, 12:47 a.m. UTC | #5
On Thu, Jun 7, 2018 at 5:31 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jun 7, 2018 at 4:00 PM, Andy Lutomirski <luto@kernel.org> wrote:
> > On Thu, Jun 7, 2018 at 3:03 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Thu, Jun 7, 2018 at 1:50 PM, Andy Lutomirski <luto@kernel.org> wrote:
> >> > On Thu, Jun 7, 2018 at 7:42 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >> >>
> >> >> From: "H.J. Lu" <hjl.tools@gmail.com>
> >> >>
> >> >> When Intel indirect branch tracking is enabled, functions in vDSO which
> >> >> may be called indirectly should have endbr32 or endbr64 as the first
> >> >> instruction.  We try to compile vDSO with -fcf-protection=branch -mibt
> >> >> if possible.  Otherwise, we insert endbr32 or endbr64 by hand to assembly
> >> >> codes generated by the compiler.
> >> >
> >> > Wow, that's... a genuine abomination.  Do we really need to support
> >> > CET on kernels built with old toolchains?
> >> >
> >>
> >> Yes.  GCC 7 should be able to build CET kernel.
> >>
> >
> > Why?  Presumably people running distros that use CET are going to have
> > kernels build with a CET-supporting compiler.
> >
>
> Good point.  It was needed before GCC 8 was released.   We can drop
> arch/x86/entry/vdso/endbr.sh now.
>

Phew!  We'll still need a patch to prevent configuring CET on a
compiler that can't support it.
diff mbox

Patch

diff --git a/arch/x86/entry/vdso/.gitignore b/arch/x86/entry/vdso/.gitignore
index aae8ffdd5880..552941fdfae0 100644
--- a/arch/x86/entry/vdso/.gitignore
+++ b/arch/x86/entry/vdso/.gitignore
@@ -5,3 +5,7 @@  vdso32-sysenter-syms.lds
 vdso32-int80-syms.lds
 vdso-image-*.c
 vdso2c
+vclock_gettime.S
+vgetcpu.S
+vclock_gettime.asm
+vgetcpu.asm
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index d998a487c9b1..cccafc36a831 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -118,6 +118,40 @@  $(obj)/%-x32.o: $(obj)/%.o FORCE
 
 targets += vdsox32.lds $(vobjx32s-y)
 
+ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
+  ifeq ($(call cc-option-yn, -fcf-protection=branch -mibt), y)
+    $(obj)/vclock_gettime.o $(obj)/vgetcpu.o $(obj)/vdso32/vclock_gettime.o: KBUILD_CFLAGS += -fcf-protection=branch -mibt
+  else
+    endbr := $(srctree)/$(src)/endbr.sh
+    quiet_cmd_endbr = ENDBR $@
+	  cmd_endbr = $(CONFIG_SHELL) '$(endbr)' $< $@
+
+    quiet_cmd_cc_asm_c = CC $(quiet_modtag) $@
+	  cmd_cc_asm_c = $(CC) $(c_flags) $(DISABLE_LTO) -S -o $@ $<
+
+$(obj)/%.asm: $(src)/%.c
+	$(call if_changed_dep,cc_asm_c)
+
+$(obj)/vclock_gettime.S: $(obj)/vclock_gettime.asm
+	$(call if_changed,endbr)
+
+$(obj)/vgetcpu.S: $(obj)/vgetcpu.asm
+	$(call if_changed,endbr)
+
+$(obj)/vclock_gettime.o: $(obj)/vclock_gettime.S
+	 $(call if_changed_rule,as_o_S)
+
+$(obj)/vgetcpu.o: $(obj)/vgetcpu.S
+	 $(call if_changed_rule,as_o_S)
+
+$(obj)/vdso32/vclock_gettime.S: $(obj)/vdso32/vclock_gettime.asm
+	$(call if_changed,endbr)
+
+$(obj)/vdso32/vclock_gettime.o: $(obj)/vdso32/vclock_gettime.S
+	 $(call if_changed_rule,as_o_S)
+  endif
+endif
+
 $(obj)/%.so: OBJCOPYFLAGS := -S
 $(obj)/%.so: $(obj)/%.so.dbg
 	$(call if_changed,objcopy)
diff --git a/arch/x86/entry/vdso/endbr.sh b/arch/x86/entry/vdso/endbr.sh
new file mode 100644
index 000000000000..983dbec182f2
--- /dev/null
+++ b/arch/x86/entry/vdso/endbr.sh
@@ -0,0 +1,32 @@ 
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+in="$1"
+out="$2"
+
+case "$in" in
+*/vdso32/*)
+    endbr=".byte 0xf3,0x0f,0x1e,0xfb"
+    ;;
+*)
+    endbr=".byte 0xf3,0x0f,0x1e,0xfa"
+    ;;
+esac
+
+need_endbr=no
+while IFS= read line ; do
+   case "$line" in
+   __vdso_clock_gettime:|__vdso_getcpu:|__vdso_gettimeofday:|__vdso_time:)
+	need_endbr=yes
+	;;
+   "	."*)
+	;;
+   "	"*)
+	if [ "$need_endbr" = "yes" ]; then
+	    need_endbr=no
+	    echo "	$endbr"
+	fi
+	;;
+   esac
+   echo "$line"
+done < "$in" > "$out"