Messages in this thread |  | | Subject | Re: [PATCH v13 8/8] x86/vsyscall/64: Fixup Shadow Stack and Indirect Branch Tracking for vsyscall emulation | From | "Yu, Yu-cheng" <> | Date | Fri, 25 Sep 2020 09:47:55 -0700 |
| |
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@intel.com> 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.
|  |