Re: [PATCH v15 05/26] x86/cet/shstk: Add Kconfig option for user-mode Shadow Stack

Yu, Yu-cheng
2020-11-30T22:49:09Z

On 11/30/2020 10:15 AM, Borislav Petkov wrote:
> On Sat, Nov 28, 2020 at 08:23:59AM -0800, Yu, Yu-cheng wrote:
>> We have X86_BRANCH_TRACKING_USER too. My thought was, X86_CET means any of
>> kernel/user shadow stack/ibt.
>
> It is not about what it means - it is what you're going to use/need. You have
> ifdeffery both with X86_CET and X86_SHADOW_STACK_USER.
>
> This one
>
> +#ifdef CONFIG_X86_SHADOW_STACK_USER
> +#define DISABLE_SHSTK 0
> +#else
> +#define DISABLE_SHSTK (1 << (X86_FEATURE_SHSTK & 31))
> +#endif
>
> for example, is clearly wrong and wants to be #ifdef CONFIG_X86_CET, for
> example. Unless I'm missing something totally obvious.

Logically, enabling IBT without shadow stack does not make sense, but
these features have different CPUIDs, and CONFIG_X86_SHADOW_STACK_USER
and CONFIG_X86_BRANCH_TRACKING_USER can be selected separately.

Do we want to have only one selection for both features? In other
words, we turn on both or neither.

Thanks,
Yu-cheng

>
> In any case, you need to analyze what Kconfig defines the code will
> need and to what they belong and add only the minimal subset needed.
> Our Kconfig symbols space is already nuts so adding more needs to be
> absolutely justified.
>
> Thx.
>