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

[Yu, Yu-cheng]

On 7/19/2021 11:21 AM, Edgecombe, Rick P wrote:
> On Fri, 2021-05-21 at 15:15 -0700, Yu-cheng Yu wrote:
> > From: "H.J. Lu" <hjl.tools@xxxxxxxxx>
> >
> > Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect
> > Branch
> > Tracking.
> >
> > Signed-off-by: H.J. Lu <hjl.tools@xxxxxxxxx>
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
> > Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > ---
> >   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.

> 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.

> 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.

Thanks,
Yu-cheng