On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu <yu-cheng...@intel.com> wrote:
>
> 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...@intel.com>
> ---
> 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..315ee3572664 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,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.

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?

Reply via email to