On 9/25/2020 9:31 AM, Andy Lutomirski wrote:
On Fri, Sep 25, 2020 at 7:58 AM Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> wrote:
[...]
@@ -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.
