Re: [PATCH v27 06/10] x86/cet/ibt: Update arch_prctl functions for Indirect Branch Tracking

On Tue, 2021-07-20 at 10:09 -0700, Yu, Yu-cheng wrote:

On 7/19/2021 11:21 AM, Edgecombe, Rick P wrote:

quoted

On Fri, 2021-05-21 at 15:15 -0700, Yu-cheng Yu wrote:

quoted

From: "H.J. Lu" <redacted>

Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect
Branch
Tracking.

Signed-off-by: H.J. Lu <redacted>
Signed-off-by: Yu-cheng Yu <redacted>
Reviewed-by: Kees Cook <redacted>
---
  arch/x86/kernel/cet_prctl.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/cet_prctl.c

b/arch/x86/kernel/cet_prctl.c
index b426d200e070..bd3c80d402e7 100644

--- a/arch/x86/kernel/cet_prctl.c
+++ b/arch/x86/kernel/cet_prctl.c

@@ -22,6 +22,9 @@ static int cet_copy_status_to_user(struct

thread_shstk *shstk, u64 __user *ubuf)
                 buf[2] = shstk->size;
         }
  
+       if (shstk->ibt)
+               buf[0] |= GNU_PROPERTY_X86_FEATURE_1_IBT;
+

Can you have IBT enabled but not shadow stack via kernel
parameters?
Outside this diff it has:
if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
        return -ENOTSUPP;

If shadow stack is disabled by the kernel parameter, IBT is also
disabled.

Thanks for the clarification.

quoted

So if "no_user_shstk" is set, this can't be used for IBT. But the
kernel would attempt to enable IBT.

It will not.

Oh yea, I see the cpuid_deps part now. Sorry.

quoted

Also if so, the CR4 bit enabling logic needs adjusting in this IBT
series. If not, we should probably mention this in the docs and
enforce
it. It would then follow the logic in Kconfig, so maybe the
simplest.
Like maybe instead of no_user_shstk, just no_user_cet?

If shadow stack is disabled (from either Kconfig or kernel 
command-line), then IBT is also disabled.  However, we still need two
kernel parameters because no_user_ibt can be useful sometimes.  I
will 
add a sentence in the document to indicate that IBT depends on shadow
stack.

Yea, no_user_ibt seems useful. I meant that renaming no_user_shstk to
no_user_cet (or similar) would be more clear and self documenting,
since it intends to disable all user cet features and not just
shadowstack. And leaving no_user_ibt as is. Documentation works as well
though. Not major in any case.