Re: [PATCH v13 8/8] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch Tracking for vsyscall emulation

From: Yu, Yu-cheng
Date: Tue Sep 29 2020 - 14:37:23 EST

On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> wrote:

On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng <yu-cheng.yu@xxxxxxxxx> wrote:
+
+ cet = get_xsave_addr(&fpu->state.xsave, XFEATURE_CET_USER);
+ if (!cet) {
+ /*
+ * This is an unlikely case where the task is
+ * CET-enabled, but CET xstate is in INIT.
+ */
+ WARN_ONCE(1, "CET is enabled, but no xstates");

"unlikely" doesn't really cover this.

+ fpregs_unlock();
+ goto sigsegv;
+ }
+
+ if (cet->user_ssp && ((cet->user_ssp + 8) < TASK_SIZE_MAX))
+ cet->user_ssp += 8;

This looks buggy. The condition should be "if SHSTK is on, then add 8
to user_ssp". If the result is noncanonical, then some appropriate
exception should be generated, probably by the FPU restore code -- see
below. You should be checking the SHSTK_EN bit, not SSP.

Updated. Is this OK? I will resend the whole series later.

Thanks,
Yu-cheng

======

From 09803e66dca38d7784e32687d0693550948199ed Mon Sep 17 00:00:00 2001
From: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
Date: Thu, 29 Nov 2018 14:15:38 -0800
Subject: [PATCH v13 8/8] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch
Tracking for vsyscall emulation

Vsyscall entry points are effectively branch targets. Mark them with
ENDBR64 opcodes. When emulating the RET instruction, unwind shadow stack
and reset IBT state machine.

Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
---
v13:
- Check shadow stack address is canonical.
- Change from writing to MSRs to writing to CET xstate.

arch/x86/entry/vsyscall/vsyscall_64.c | 34 +++++++++++++++++++++++
arch/x86/entry/vsyscall/vsyscall_emu_64.S | 9 ++++++
arch/x86/entry/vsyscall/vsyscall_trace.h | 1 +
3 files changed, 44 insertions(+)

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index 44c33103a955..30b166091d46 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -38,6 +38,9 @@
#include <asm/fixmap.h>
#include <asm/traps.h>
#include <asm/paravirt.h>
+#include <asm/fpu/xstate.h>
+#include <asm/fpu/types.h>
+#include <asm/fpu/internal.h>

#define CREATE_TRACE_POINTS
#include "vsyscall_trace.h"
@@ -286,6 +289,44 @@ bool emulate_vsyscall(unsigned long error_code,
/* Emulate a ret instruction. */
regs->ip = caller;
regs->sp += 8;
+
+#ifdef CONFIG_X86_CET
+ if (tsk->thread.cet.shstk_size || tsk->thread.cet.ibt_enabled) {
+ struct cet_user_state *cet;
+ struct fpu *fpu;
+
+ fpu = &tsk->thread.fpu;
+ fpregs_lock();
+
+ if (!test_thread_flag(TIF_NEED_FPU_LOAD)) {
+ copy_fpregs_to_fpstate(fpu);
+ set_thread_flag(TIF_NEED_FPU_LOAD);
+ }
+
+ cet = get_xsave_addr(&fpu->state.xsave, XFEATURE_CET_USER);
+ if (!cet) {
+ /*
+ * This should not happen. The task is
+ * CET-enabled, but CET xstate is in INIT.
+ */
+ WARN_ONCE(1, "CET is enabled, but no xstates");
+ fpregs_unlock();
+ goto sigsegv;
+ }
+
+ if (cet->user_cet & CET_SHSTK_EN) {
+ if (cet->user_ssp && (cet->user_ssp + 8 < TASK_SIZE_MAX))
+ cet->user_ssp += 8;
+ }
+
+ if (cet->user_cet & CET_ENDBR_EN)
+ cet->user_cet &= ~CET_WAIT_ENDBR;
+
+ __fpu_invalidate_fpregs_state(fpu);
+ fpregs_unlock();
+ }
+#endif
+
return true;

sigsegv:
diff --git a/arch/x86/entry/vsyscall/vsyscall_emu_64.S b/arch/x86/entry/vsyscall/vsyscall_emu_64.S
index 2e203f3a25a7..b2fa92104cdb 100644
--- a/arch/x86/entry/vsyscall/vsyscall_emu_64.S
+++ b/arch/x86/entry/vsyscall/vsyscall_emu_64.S
@@ -17,16 +17,25 @@ __PAGE_ALIGNED_DATA
.type __vsyscall_page, @object
__vsyscall_page:

+#ifdef CONFIG_X86_BRANCH_TRACKING_USER
+ endbr64
+#endif
mov $__NR_gettimeofday, %rax
syscall
ret

.balign 1024, 0xcc
+#ifdef CONFIG_X86_BRANCH_TRACKING_USER
+ endbr64
+#endif
mov $__NR_time, %rax
syscall
ret

.balign 1024, 0xcc
+#ifdef CONFIG_X86_BRANCH_TRACKING_USER
+ endbr64
+#endif
mov $__NR_getcpu, %rax
syscall
ret
diff --git a/arch/x86/entry/vsyscall/vsyscall_trace.h b/arch/x86/entry/vsyscall/vsyscall_trace.h
index 3c3f9765a85c..7aa2101ada44 100644
--- a/arch/x86/entry/vsyscall/vsyscall_trace.h
+++ b/arch/x86/entry/vsyscall/vsyscall_trace.h
@@ -25,6 +25,7 @@ TRACE_EVENT(emulate_vsyscall,
#endif

#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
#define TRACE_INCLUDE_PATH ../../arch/x86/entry/vsyscall/
#define TRACE_INCLUDE_FILE vsyscall_trace
#include <trace/define_trace.h>
--
2.21.0