On Thu, Sep 3, 2020 at 12:13 AM Yu, Yu-cheng <yu-cheng.yu@xxxxxxxxx> wrote: > On 9/2/2020 1:03 PM, Jann Horn wrote: > > On Tue, Aug 25, 2020 at 2:30 AM Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> wrote: > >> Add REGSET_CET64/REGSET_CET32 to get/set CET MSRs: > >> > >> IA32_U_CET (user-mode CET settings) and > >> IA32_PL3_SSP (user-mode Shadow Stack) > > [...] > >> diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c > > [...] > >> +int cetregs_get(struct task_struct *target, const struct user_regset *regset, > >> + struct membuf to) > >> +{ > >> + struct fpu *fpu = &target->thread.fpu; > >> + struct cet_user_state *cetregs; > >> + > >> + if (!boot_cpu_has(X86_FEATURE_SHSTK)) > >> + return -ENODEV; > >> + > >> + fpu__prepare_read(fpu); > >> + cetregs = get_xsave_addr(&fpu->state.xsave, XFEATURE_CET_USER); > >> + if (!cetregs) > >> + return -EFAULT; > > > > Can this branch ever be hit without a kernel bug? If yes, I think > > -EFAULT is probably a weird error code to choose here. If no, this > > should probably use WARN_ON(). Same thing in cetregs_set(). > > > > When a thread is not CET-enabled, its CET state does not exist. I > looked at EFAULT, and it means "Bad address". Maybe this can be ENODEV, > which means "No such device"? Yeah, I guess ENODEV might fit reasonably well.