On Mon, Feb 27, 2023 at 02:29:55PM -0800, Rick Edgecombe wrote:
> The only downside to not having a generic supervisor xfeature regset,
> is that apps need to be enlightened of any new supervisor xfeature
> exposed this way (i.e. they can't try to have generic save/restore
> logic). But maybe that is a good thing, because they have to think
> through each new xfeature instead of encountering issues when new a new
Remove the first "new".
> supervisor xfeature was added.
>
> By adding a shadow stack regset, it also has the effect of including the
> shadow stack state in a core dump, which could be useful for debugging.
>
> The shadow stack specific xstate includes the SSP, and the shadow stack
> and WRSS enablement status. Enabling shadow stack or wrss in the kernel
^^^^
"WRSS"
> involves more than just flipping the bit. The kernel is made aware that
> it has to do extra things when cloning or handling signals. That logic
> is triggered off of separate feature enablement state kept in the task
> struct. So the flipping on HW shadow stack enforcement without notifying
> the kernel to change its behavior would severely limit what an application
> could do without crashing, and the results would depend on kernel
> internal implementation details. There is also no known use for controlling
> this state via prtace today. So only expose the SSP, which is something
Unknown word [prtace] in commit message.
Suggestions: ['ptrace'
> that userspace already has indirect control over.
>
> Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>
> Tested-by: John Allen <john.allen@xxxxxxx>
> Tested-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Acked-by: Mike Rapoport (IBM) <rppt@xxxxxxxxxx>
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
I think your SOB should come last:
...
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
Co-developed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
Pls check whole set.
> +#ifdef CONFIG_X86_USER_SHADOW_STACK
> +int ssp_active(struct task_struct *target, const struct user_regset *regset)
> +{
> + if (target->thread.features & ARCH_SHSTK_SHSTK)
> + return regset->n;
> +
> + return 0;
> +}
> +
> +int ssp_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_USER_SHSTK))
check_for_deprecated_apis: WARNING: arch/x86/kernel/fpu/regset.c:193: Do not use boot_cpu_has() - use cpu_feature_enabled() instead.
Check your whole set pls.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
