Control-flow Enforcement (CET) is a new Intel processor feature that blocks
return/jump-oriented programming attacks. Details are in "Intel 64 and
IA-32 Architectures Software Developer's Manual" [1].
This is the second part of CET and enables Indirect Branch Tracking (IBT).
It is built on top of the shadow stack series.
Changes in v13:
- Drop the patch that disables vsyscall emulation when CET is enabled, and
re-introduce an earlier patch that fixes shadow stack and IBT for the
emulation code.
- Remove "INTEL" string from CET Kconfig options, update help text, and
change IBT default configuration to N.
[1] Intel 64 and IA-32 Architectures Software Developer's Manual:
https://software.intel.com/en-us/download/intel-64-and-ia-32-
architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4
[2] Indirect Branch Tracking patches v12.
https://lkml.kernel.org/r/20200918192312.25978-1-yu-cheng.yu@intel.com/
H.J. Lu (3):
x86/cet/ibt: Update arch_prctl functions for Indirect Branch Tracking
x86/vdso/32: Add ENDBR32 to __kernel_vsyscall entry point
x86/vdso: Insert endbr32/endbr64 to vDSO
Yu-cheng Yu (5):
x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
x86/cet/ibt: User-mode Indirect Branch Tracking support
x86/cet/ibt: Handle signals for Indirect Branch Tracking
x86/cet/ibt: ELF header parsing for Indirect Branch Tracking
x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch Tracking for
vsyscall emulation
arch/x86/Kconfig | 21 +++++++
arch/x86/entry/vdso/Makefile | 4 ++
arch/x86/entry/vdso/vdso32/system_call.S | 3 +
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 +
arch/x86/include/asm/cet.h | 3 +
arch/x86/include/asm/disabled-features.h | 8 ++-
arch/x86/kernel/cet.c | 60 ++++++++++++++++++-
arch/x86/kernel/cet_prctl.c | 8 ++-
arch/x86/kernel/cpu/common.c | 17 ++++++
arch/x86/kernel/fpu/signal.c | 8 ++-
arch/x86/kernel/process_64.c | 8 +++
.../arch/x86/include/asm/disabled-features.h | 8 ++-
14 files changed, 184 insertions(+), 8 deletions(-)
--
2.21.0
From: "H.J. Lu" <redacted>
When Indirect Branch Tracking (IBT) is enabled, vDSO functions may be
called indirectly, and must have ENDBR32 or ENDBR64 as the first
instruction. The compiler must support -fcf-protection=branch so that it
can be used to compile vDSO.
Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
---
v12:
- Replace object file list with $(vobjs) $(vobjs32).
arch/x86/entry/vdso/Makefile | 4 ++++
1 file changed, 4 insertions(+)
Introduce Kconfig option X86_BRANCH_TRACKING_USER.
Indirect Branch Tracking (IBT) provides protection against CALL-/JMP-
oriented programming attacks. It is active when the kernel has this
feature enabled, and the processor and the application support it.
When this feature is enabled, legacy non-IBT applications continue to
work, but without IBT protection.
Signed-off-by: Yu-cheng Yu <redacted>
---
v13:
- Update help text, and change default to N.
- Change X86_INTEL_* to X86_*.
v10:
- Change build-time CET check to config depends on.
arch/x86/Kconfig | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
@@ -536,6 +536,23 @@ static __init int setup_disable_shstk(char *s)__setup("no_user_shstk",setup_disable_shstk);#endif+#ifdef CONFIG_X86_BRANCH_TRACKING_USER+static__initintsetup_disable_ibt(char*s)+{+/* require an exact match without trailing characters */+if(s[0]!='\0')+return0;++if(!boot_cpu_has(X86_FEATURE_IBT))+return1;++setup_clear_cpu_cap(X86_FEATURE_IBT);+pr_info("x86: 'no_user_ibt' specified, disabling user Branch Tracking\n");+return1;+}+__setup("no_user_ibt",setup_disable_ibt);+#endif+/**SomeCPUfeaturesdependonhigherCPUIDlevels,whichmaynotalways*beavailableduetoCPUIDlevelcappingorbrokenvirtualization
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 <redacted>
---
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(+)
@@ -286,6 +289,37 @@ 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){+structcet_user_state*cet;+structfpu*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){+fpregs_unlock();+gotosigsegv;+}++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+returntrue;sigsegv:
An indirect CALL/JMP moves the indirect branch tracking (IBT) state machine
to WAIT_ENDBR status until the instruction reaches an ENDBR opcode. If the
CALL/JMP does not reach an ENDBR opcode, the processor raises a control-
protection fault. WAIT_ENDBR status can be read from MSR_IA32_U_CET.
WAIT_ENDBR is cleared for signal handling, and restored for sigreturn.
IBT state machine is described in Intel SDM Vol. 1, Sec. 18.3.
Signed-off-by: Yu-cheng Yu <redacted>
---
v9:
- Fix missing WAIT_ENDBR in signal handling.
arch/x86/kernel/cet.c | 27 +++++++++++++++++++++++++--
arch/x86/kernel/fpu/signal.c | 8 +++++---
2 files changed, 30 insertions(+), 5 deletions(-)
@@ -577,7 +579,7 @@ static unsigned long fpu__alloc_sigcontext_ext(unsigned long sp)*sigcontext_extisat:fpu+fpu_user_xstate_size+*FP_XSTATE_MAGIC2_SIZE,thenalignedto8.*/-if(cet->shstk_size)+if(cet->shstk_size||cet->ibt_enabled)sp-=(sizeof(structsc_ext)+8);returnsp;
From: Andy Lutomirski <luto@kernel.org> Date: 2020-09-25 16:18:57
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu [off-list ref] wrote:
From: "H.J. Lu" <redacted>
When Indirect Branch Tracking (IBT) is enabled, vDSO functions may be
called indirectly, and must have ENDBR32 or ENDBR64 as the first
instruction. The compiler must support -fcf-protection=branch so that it
can be used to compile vDSO.
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu [off-list ref] wrote:
quoted
From: "H.J. Lu" <redacted>
When Indirect Branch Tracking (IBT) is enabled, vDSO functions may be
called indirectly, and must have ENDBR32 or ENDBR64 as the first
instruction. The compiler must support -fcf-protection=branch so that it
can be used to compile vDSO.
From: Andy Lutomirski <luto@kernel.org> Date: 2020-09-25 16:31:57
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu [off-list ref] wrote:
quoted hunk
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 <redacted>
---
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(+)
@@ -286,6 +289,37 @@ 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){+structcet_user_state*cet;+structfpu*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){+fpregs_unlock();+gotosigsegv;
I *think* your patchset tries to keep cet.shstk_size and
cet.ibt_enabled in sync with the MSR, in which case it should be
impossible to get here, but a comment and a warning would be much
better than a random sigsegv.
Shouldn't we have a get_xsave_addr_or_allocate() that will never
return NULL but instead will mark the state as in use and set up the
init state if the feature was previously not in use?
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu [off-list ref] wrote:
quoted
[...]
quoted
@@ -286,6 +289,37 @@ 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) {+ fpregs_unlock();+ goto sigsegv;
I *think* your patchset tries to keep cet.shstk_size and
cet.ibt_enabled in sync with the MSR, in which case it should be
impossible to get here, but a comment and a warning would be much
better than a random sigsegv.
Yes, it should be impossible to get here. I will add a comment and a
warning, but still do sigsegv. Should this happen, and the function
return, the app gets a control-protection fault. Why not let it fail early?
Shouldn't we have a get_xsave_addr_or_allocate() that will never
return NULL but instead will mark the state as in use and set up the
init state if the feature was previously not in use?
We already have a static __raw_xsave_addr(), which returns a pointer to
the requested xstate. Maybe we can export __raw_xsave_addr(), if that
is needed.
From: Andy Lutomirski <luto@amacapital.net> Date: 2020-09-25 16:51:19
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] wrote:
On 9/25/2020 9:31 AM, Andy Lutomirski wrote:
quoted
quoted
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu [off-list ref] wrote:
[...]
quoted
quoted
@@ -286,6 +289,37 @@ 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) {+ fpregs_unlock();+ goto sigsegv;
I *think* your patchset tries to keep cet.shstk_size and
cet.ibt_enabled in sync with the MSR, in which case it should be
impossible to get here, but a comment and a warning would be much
better than a random sigsegv.
Yes, it should be impossible to get here. I will add a comment and a warning, but still do sigsegv. Should this happen, and the function return, the app gets a control-protection fault. Why not let it fail early?
I’m okay with either approach as long as we get a comment and warning.
quoted
Shouldn't we have a get_xsave_addr_or_allocate() that will never
return NULL but instead will mark the state as in use and set up the
init state if the feature was previously not in use?
We already have a static __raw_xsave_addr(), which returns a pointer to the requested xstate. Maybe we can export __raw_xsave_addr(), if that is needed.
I don’t think that’s what we want in general — we want the whole construct of initializing the state if needed.
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] wrote:
On 9/25/2020 9:31 AM, Andy Lutomirski wrote:
quoted
quoted
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu [off-list ref] wrote:
[...]
quoted
quoted
@@ -286,6 +289,37 @@ 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) {+ fpregs_unlock();+ goto sigsegv;
I *think* your patchset tries to keep cet.shstk_size and
cet.ibt_enabled in sync with the MSR, in which case it should be
impossible to get here, but a comment and a warning would be much
better than a random sigsegv.
Yes, it should be impossible to get here. I will add a comment and a warning, but still do sigsegv. Should this happen, and the function return, the app gets a control-protection fault. Why not let it fail early?
I’m okay with either approach as long as we get a comment and warning.
Here is the updated patch. I can also re-send the whole series as v14. Thanks!
======
From 09803e66dca38d7784e32687d0693550948199ed Mon Sep 17 00:00:00 2001
From: Yu-cheng Yu <redacted>
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 <redacted>
---
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(+)
@@ -286,6 +289,42 @@ 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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisisanunlikelycasewherethetaskis+*CET-enabled,butCETxstateisinINIT.+*/+WARN_ONCE(1,"CET is enabled, but no xstates");+fpregs_unlock();+gotosigsegv;+}++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+returntrue;sigsegv:
From: Andy Lutomirski <luto@kernel.org> Date: 2020-09-28 17:37:59
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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.
Also, can you point me to where any of these canonicality rules are
documented in the SDM? I looked and I can't find them.
This reminds me: this code in extable.c needs to change.
__visible bool ex_handler_fprestore(const struct exception_table_entry *fixup,
struct pt_regs *regs, int trapnr,
unsigned long error_code,
unsigned long fault_addr)
{
regs->ip = ex_fixup_addr(fixup);
WARN_ONCE(1, "Bad FPU state detected at %pB, reinitializing
FPU registers.",
(void *)instruction_pointer(regs));
__copy_kernel_to_fpregs(&init_fpstate, -1);
Now that we have supervisor states like CET, this is buggy. This
should do something intelligent like initializing all the *user* state
and trying again. If that succeeds, a signal should be sent rather
than just corrupting the task. And if it fails, then perhaps some
actual intelligence is needed. We certainly should not just disable
CET because something is wrong with the CET MSRs.
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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.
The code now checks if shadow stack is on (yes, it should check SHSTK_EN
bit, I will fix it.), then adds 8 to user_ssp. If the result is
canonical, then it sets the corresponding xstate.
If the resulting address is not canonical, the kernel does not know what
the address should be either. I think the best action to take is doing
nothing about the shadow stack pointer, and let the application return
and get a control protection fault. The application should have not got
into such situation in the first place; if it does, it should fault.
Also, can you point me to where any of these canonicality rules are
documented in the SDM? I looked and I can't find them.
The SDM is not very explicit. It should have been.
This reminds me: this code in extable.c needs to change.
__visible bool ex_handler_fprestore(const struct exception_table_entry *fixup,
struct pt_regs *regs, int trapnr,
unsigned long error_code,
unsigned long fault_addr)
{
regs->ip = ex_fixup_addr(fixup);
WARN_ONCE(1, "Bad FPU state detected at %pB, reinitializing
FPU registers.",
(void *)instruction_pointer(regs));
__copy_kernel_to_fpregs(&init_fpstate, -1);
Now that we have supervisor states like CET, this is buggy. This
should do something intelligent like initializing all the *user* state
and trying again. If that succeeds, a signal should be sent rather
than just corrupting the task. And if it fails, then perhaps some
actual intelligence is needed. We certainly should not just disable
CET because something is wrong with the CET MSRs.
Yes, but it needs more thought. Maybe a separate patch and more discussion?
Yu-cheng
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/+WARN_ONCE(1,"CET is enabled, but no xstates");+fpregs_unlock();+gotosigsegv;+}++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+returntrue;sigsegv:
From: Andy Lutomirski <luto@kernel.org> Date: 2020-09-29 19:57:49
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted hunk
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
--Andy
From: Andy Lutomirski <luto@kernel.org> Date: 2020-09-29 20:01:10
On Tue, Sep 29, 2020 at 12:57 PM Andy Lutomirski [off-list ref] wrote:
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
quoted
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
On Tue, Sep 29, 2020 at 12:57 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
quoted
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Yu-cheng
From: Andy Lutomirski <luto@kernel.org> Date: 2020-09-30 23:44:56
On Wed, Sep 30, 2020 at 3:33 PM Yu, Yu-cheng [off-list ref] wrote:
On 9/29/2020 1:00 PM, Andy Lutomirski wrote:
quoted
On Tue, Sep 29, 2020 at 12:57 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
quoted
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
On Wed, Sep 30, 2020 at 4:44 PM Andy Lutomirski [off-list ref] wrote:
On Wed, Sep 30, 2020 at 3:33 PM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/29/2020 1:00 PM, Andy Lutomirski wrote:
quoted
On Tue, Sep 29, 2020 at 12:57 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
quoted
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
--
H.J.
From: Andy Lutomirski <luto@kernel.org> Date: 2020-10-01 01:10:37
On Wed, Sep 30, 2020 at 6:01 PM H.J. Lu [off-list ref] wrote:
On Wed, Sep 30, 2020 at 4:44 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Sep 30, 2020 at 3:33 PM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/29/2020 1:00 PM, Andy Lutomirski wrote:
quoted
On Tue, Sep 29, 2020 at 12:57 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
quoted
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
Let me say this one more time:
If we have a per-process vsyscall disable control and a per-process
CET control, we are going to keep those settings orthogonal. I'm
willing to entertain an option in which enabling SHSTK without also
disabling vsyscalls is disallowed, We are *not* going to have any CET
flags magically disable vsyscalls, though, and we are not going to
have a situation where disabling vsyscalls on process startup requires
enabling SHSTK.
Any possible static vsyscall controls (and CET controls, for that
matter) also need to come with some explanation of whether they are
properties set on the ELF loader, the ELF program being loaded, or
both. And this explanation needs to cover what happens when old
binaries link against new libc versions and vice versa. A new
CET-enabled binary linked against old libc running on a new kernel
that is expected to work on a non-CET CPU MUST work on a CET CPU, too.
Right now, literally the only thing preventing vsyscall emulation from
coexisting with SHSTK is that the implementation eeds work.
So your proposal is rejected. Sorry.
On Wed, Sep 30, 2020 at 6:10 PM Andy Lutomirski [off-list ref] wrote:
On Wed, Sep 30, 2020 at 6:01 PM H.J. Lu [off-list ref] wrote:
quoted
On Wed, Sep 30, 2020 at 4:44 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Wed, Sep 30, 2020 at 3:33 PM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/29/2020 1:00 PM, Andy Lutomirski wrote:
quoted
On Tue, Sep 29, 2020 at 12:57 PM Andy Lutomirski [off-list ref] wrote:
quoted
On Tue, Sep 29, 2020 at 11:37 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/28/2020 10:37 AM, Andy Lutomirski wrote:
quoted
On Mon, Sep 28, 2020 at 9:59 AM Yu-cheng Yu [off-list ref] wrote:
quoted
On Fri, 2020-09-25 at 09:51 -0700, Andy Lutomirski wrote:
quoted
quoted
On Sep 25, 2020, at 9:48 AM, Yu, Yu-cheng [off-list ref] 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");
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 <redacted>
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 <redacted>
---
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(+)
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
Can the comment explain better, please? I would say something like:
If the kernel thinks this task has CET enabled (because
tsk->thread.cet has one of the features enabled), then the
corresponding bits must also be set in the CET XSAVES region. If the
CET XSAVES region is in the INIT state, then the kernel's concept of
the task's CET state is corrupt.
quoted
+ 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;
+ }
This makes so sense to me. Also, the vsyscall emulation code is
intended to be as rigid as possible to minimize the chance that it
gets used as an exploit gadget. So we should not silently corrupt
anything. Moreover, this code seems quite dangerous -- you've created
a gadget that does RET without actually verifying the SHSTK token. If
SHSTK and some form of strong indirect branch/call CFI is in use, then
the existance of a CFI-bypassing return primitive at a fixed address
seems quite problematic.
So I think you need to write a function that reasonably accurately
emulates a usermode RET.
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
Let me say this one more time:
If we have a per-process vsyscall disable control and a per-process
CET control, we are going to keep those settings orthogonal. I'm
willing to entertain an option in which enabling SHSTK without also
disabling vsyscalls is disallowed, We are *not* going to have any CET
flags magically disable vsyscalls, though, and we are not going to
have a situation where disabling vsyscalls on process startup requires
enabling SHSTK.
Any possible static vsyscall controls (and CET controls, for that
matter) also need to come with some explanation of whether they are
properties set on the ELF loader, the ELF program being loaded, or
Kernel enables CET on CET processors only if ld.so is CET enabled.
Kernel passes control to CET enabled ld.so with CET enabled and
ld.so will check if CET should be disabled because of legacy program
or dependency libraries.
both. And this explanation needs to cover what happens when old
binaries link against new libc versions and vice versa. A new
CET-enabled binary linked against old libc running on a new kernel
that is expected to work on a non-CET CPU MUST work on a CET CPU, too.
Since kernel doesn't enable CET on ld.so from the old libc, the new
CET-enabled binary will start with CET disabled regardless whatever the
processor the binary runs on.
Right now, literally the only thing preventing vsyscall emulation from
coexisting with SHSTK is that the implementation eeds work.
So your proposal is rejected. Sorry.
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
[...]
quoted
quoted
quoted
quoted
quoted
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
Let me say this one more time:
If we have a per-process vsyscall disable control and a per-process
CET control, we are going to keep those settings orthogonal. I'm
willing to entertain an option in which enabling SHSTK without also
disabling vsyscalls is disallowed, We are *not* going to have any CET
flags magically disable vsyscalls, though, and we are not going to
have a situation where disabling vsyscalls on process startup requires
enabling SHSTK.
Any possible static vsyscall controls (and CET controls, for that
matter) also need to come with some explanation of whether they are
properties set on the ELF loader, the ELF program being loaded, or
both. And this explanation needs to cover what happens when old
binaries link against new libc versions and vice versa. A new
CET-enabled binary linked against old libc running on a new kernel
that is expected to work on a non-CET CPU MUST work on a CET CPU, too.
Right now, literally the only thing preventing vsyscall emulation from
coexisting with SHSTK is that the implementation eeds work.
So your proposal is rejected. Sorry.
I think, even with shadow stack/ibt enabled, we can still allow XONLY
without too much mess.
What about this?
Thanks,
Yu-cheng
======
@@ -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){+structcet_user_state*cet;+structfpu*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){+/*+*Thisshouldnothappen.Thetaskis+*CET-enabled,butCETxstateisinINIT.+*/
[...]
quoted
quoted
quoted
quoted
quoted
quoted
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
Let me say this one more time:
If we have a per-process vsyscall disable control and a per-process
CET control, we are going to keep those settings orthogonal. I'm
willing to entertain an option in which enabling SHSTK without also
disabling vsyscalls is disallowed, We are *not* going to have any CET
flags magically disable vsyscalls, though, and we are not going to
have a situation where disabling vsyscalls on process startup requires
enabling SHSTK.
Any possible static vsyscall controls (and CET controls, for that
matter) also need to come with some explanation of whether they are
properties set on the ELF loader, the ELF program being loaded, or
both. And this explanation needs to cover what happens when old
binaries link against new libc versions and vice versa. A new
CET-enabled binary linked against old libc running on a new kernel
that is expected to work on a non-CET CPU MUST work on a CET CPU, too.
Right now, literally the only thing preventing vsyscall emulation from
coexisting with SHSTK is that the implementation eeds work.
So your proposal is rejected. Sorry.
I think, even with shadow stack/ibt enabled, we can still allow XONLY
without too much mess.
What about this?
Thanks,
Yu-cheng
======
I don't get it.
First, you can't do any of this based on config -- it must be runtime.
Second, and more importantly, I don't see how XONLY helps at all. The
(non-executable) text that's exposed to user code in EMULATE mode is
trivial to get right with CET -- your code already handles it. It's
the emulation code (that runs identically in EMULATE and XONLY mode)
that's tricky.
On Thu, Oct 1, 2020 at 9:51 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/30/2020 6:10 PM, Andy Lutomirski wrote:
quoted
On Wed, Sep 30, 2020 at 6:01 PM H.J. Lu [off-list ref] wrote:
quoted
On Wed, Sep 30, 2020 at 4:44 PM Andy Lutomirski [off-list ref] wrote:
[...]
quoted
quoted
quoted
quoted
quoted
quoted
quoted
From 09803e66dca38d7784e32687d0693550948199ed Mon Sep 17 00:00:00 2001
From: Yu-cheng Yu <redacted>
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 <redacted>
[...]
quoted
quoted
quoted
quoted
quoted
quoted
quoted
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
Let me say this one more time:
If we have a per-process vsyscall disable control and a per-process
CET control, we are going to keep those settings orthogonal. I'm
willing to entertain an option in which enabling SHSTK without also
disabling vsyscalls is disallowed, We are *not* going to have any CET
flags magically disable vsyscalls, though, and we are not going to
have a situation where disabling vsyscalls on process startup requires
enabling SHSTK.
Any possible static vsyscall controls (and CET controls, for that
matter) also need to come with some explanation of whether they are
properties set on the ELF loader, the ELF program being loaded, or
both. And this explanation needs to cover what happens when old
binaries link against new libc versions and vice versa. A new
CET-enabled binary linked against old libc running on a new kernel
that is expected to work on a non-CET CPU MUST work on a CET CPU, too.
Right now, literally the only thing preventing vsyscall emulation from
coexisting with SHSTK is that the implementation eeds work.
So your proposal is rejected. Sorry.
I think, even with shadow stack/ibt enabled, we can still allow XONLY
without too much mess.
What about this?
Thanks,
Yu-cheng
======
I don't get it.
First, you can't do any of this based on config -- it must be runtime.
Second, and more importantly, I don't see how XONLY helps at all. The
(non-executable) text that's exposed to user code in EMULATE mode is
trivial to get right with CET -- your code already handles it. It's
the emulation code (that runs identically in EMULATE and XONLY mode)
that's tricky.
Hi,
There has been some ambiguity in my previous proposals. To make things
clear, I created a patch for arch_prctl(VSYSCALL_CTL), which controls
the TIF_VSYSCALL_DISABLE flag. It is entirely orthogonal to shadow
stack or IBT. On top of the patch, we can do SET_PERSONALITY2() to
disable vsyscall, e.g.
======
======
The is the patch.
From a124b81086122495d6837f26df99db619cd5402a Mon Sep 17 00:00:00 2001
From: Yu-cheng Yu <redacted>
Date: Mon, 5 Oct 2020 12:10:26 -0700
Subject: [PATCH 34/45] x86/vsyscall/64: Introduce arch_prctl(VSYCALL_CTL)
Vsyscall emulation provides compatibility to older applications. Newer
applications use the vDSO interface and do not use vsyscalls, and it is
desirable to have a per-task control of vsyscall.
One use case of the interface is when shadow stack and/or indirect branch
tracking is enabled and vsyscall emulation needs to cancel out the control-
flow protection. The cancelling code, if implemented, could become a back
door for evading the protection. Disabling vsyscall eliminates the risk.
Introduce arch_prctl(VSYSCALL_CTL), which sets/clears TIF_VSYSCALL_DISABLE
flag. When the flag is set, vsyscall is disabled.
Signed-off-by: Yu-cheng Yu <redacted>
---
arch/x86/entry/vsyscall/vsyscall_64.c | 3 +++
arch/x86/include/asm/thread_info.h | 2 ++
arch/x86/include/uapi/asm/prctl.h | 1 +
arch/x86/kernel/process_64.c | 17 +++++++++++++++++
tools/arch/x86/include/uapi/asm/prctl.h | 1 +
5 files changed, 24 insertions(+)
@@ -127,6 +127,9 @@ bool emulate_vsyscall(unsigned long error_code,longret;unsignedlongorig_dx;+if(test_thread_flag(TIF_VSYSCALL_DISABLE))+returnfalse;+/* Write faults or kernel-privilege faults never get fixed up. */if((error_code&(X86_PF_WRITE|X86_PF_USER))!=X86_PF_USER)returnfalse;
diff --git a/arch/x86/include/asm/thread_info.h
b/arch/x86/include/asm/thread_info.h
index 267701ae3d86..c0cce3401c0f 100644
From: Andy Lutomirski <luto@kernel.org> Date: 2020-10-09 17:43:08
On Tue, Oct 6, 2020 at 12:09 PM Yu, Yu-cheng [off-list ref] wrote:
On 10/1/2020 10:26 AM, Andy Lutomirski wrote:
quoted
On Thu, Oct 1, 2020 at 9:51 AM Yu, Yu-cheng [off-list ref] wrote:
quoted
On 9/30/2020 6:10 PM, Andy Lutomirski wrote:
quoted
On Wed, Sep 30, 2020 at 6:01 PM H.J. Lu [off-list ref] wrote:
quoted
On Wed, Sep 30, 2020 at 4:44 PM Andy Lutomirski [off-list ref] wrote:
[...]
quoted
quoted
quoted
quoted
quoted
quoted
quoted
From 09803e66dca38d7784e32687d0693550948199ed Mon Sep 17 00:00:00 2001
From: Yu-cheng Yu <redacted>
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 <redacted>
[...]
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
For what it's worth, I think there is an alternative. If you all
(userspace people, etc) can come up with a credible way for a user
program to statically declare that it doesn't need vsyscalls, then we
could make SHSTK depend on *that*, and we could avoid this mess. This
breaks orthogonality, but it's probably a decent outcome.
Would an arch_prctl(DISABLE_VSYSCALL) work? The kernel then sets a
thread flag, and in emulate_vsyscall(), checks the flag.
When CET is enabled, ld-linux will do DISABLE_VSYSCALL.
How is that?
Backwards, no? Presumably vsyscall needs to be disabled before or
concurrently with CET being enabled, not after.
I think the solution of making vsyscall emulation work correctly with
CET is going to be better and possibly more straightforward.
We can do
1. Add ARCH_X86_DISABLE_VSYSCALL to disable the vsyscall page.
2. If CPU supports CET and the program is CET enabled:
a. Disable the vsyscall page.
b. Pass control to user.
c. Enable the vsyscall page when ARCH_X86_CET_DISABLE is called.
So when control is passed from kernel to user, the vsyscall page is
disabled if the program
is CET enabled.
Let me say this one more time:
If we have a per-process vsyscall disable control and a per-process
CET control, we are going to keep those settings orthogonal. I'm
willing to entertain an option in which enabling SHSTK without also
disabling vsyscalls is disallowed, We are *not* going to have any CET
flags magically disable vsyscalls, though, and we are not going to
have a situation where disabling vsyscalls on process startup requires
enabling SHSTK.
Any possible static vsyscall controls (and CET controls, for that
matter) also need to come with some explanation of whether they are
properties set on the ELF loader, the ELF program being loaded, or
both. And this explanation needs to cover what happens when old
binaries link against new libc versions and vice versa. A new
CET-enabled binary linked against old libc running on a new kernel
that is expected to work on a non-CET CPU MUST work on a CET CPU, too.
Right now, literally the only thing preventing vsyscall emulation from
coexisting with SHSTK is that the implementation eeds work.
So your proposal is rejected. Sorry.
I think, even with shadow stack/ibt enabled, we can still allow XONLY
without too much mess.
What about this?
Thanks,
Yu-cheng
======
I don't get it.
First, you can't do any of this based on config -- it must be runtime.
Second, and more importantly, I don't see how XONLY helps at all. The
(non-executable) text that's exposed to user code in EMULATE mode is
trivial to get right with CET -- your code already handles it. It's
the emulation code (that runs identically in EMULATE and XONLY mode)
that's tricky.
Hi,
There has been some ambiguity in my previous proposals. To make things
clear, I created a patch for arch_prctl(VSYSCALL_CTL), which controls
the TIF_VSYSCALL_DISABLE flag. It is entirely orthogonal to shadow
stack or IBT. On top of the patch, we can do SET_PERSONALITY2() to
disable vsyscall, e.g.
This is not what "orthogonal" means. If the bits were orthogonal, the
logic would be:
if (gnu_property & DISABLE_VSYSCALL)
disable vsyscall;
if (gnu_property & SHSTK)
enable SHSTK;
if (gnu_property & IBT);
enable IBT;
and, if necessarily (although I still think it would be preferable not
to do this):
if ((gnu_property & (DISABLE_VSYSCALL | SHSTK)) == SHSTK)
return -EINVAL;
As far as I'm concerned, you have two choices:
a) Make SHSTK work *correctly* with vsyscall emulation.
b) Add a high quality mechanism to disable vsyscall emulation and make
SHSTK depend on that.
As far as I'm concerned, (a) is preferable. Ideally we'd get (a)
*and* a high quality vsyscall emulation disable mechanism with no
dependencies.
@@ -127,6 +127,9 @@ bool emulate_vsyscall(unsigned long error_code,longret;unsignedlongorig_dx;+if(test_thread_flag(TIF_VSYSCALL_DISABLE))+returnfalse;+
This needs to be per-mm, not per-thread. There's a patch floating
around that gets us about a quarter of the way there. I'm not
convinced that CET should wait for this to finish.