2021-05-21 22:20:31

by Yu-cheng Yu

[permalink] [raw]
Subject: [PATCH v27 00/10] Control-flow Enforcement: Indirect Branch Tracking

Control-flow Enforcement (CET) is a new Intel processor feature that blocks
return/jump-oriented programming attacks. Details are in "Intel 64 and
IA-32 Architectures Software Developer's Manual" [1].

This is the second part of CET and enables Indirect Branch Tracking (IBT).
It is built on top of the shadow stack series.

Changes in v27:
- Use a ucontext flag to save/restore IBT status.
- Disable IBT support for IA32.
- Rebase to Linus tree v5.13-rc2.

[1] Intel 64 and IA-32 Architectures Software Developer's Manual:

https://software.intel.com/en-us/download/intel-64-and-ia-32-
architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4

[2] Indirect Branch Tracking patches v26:

https://lore.kernel.org/r/[email protected]/

H.J. Lu (3):
x86/cet/ibt: Update arch_prctl functions for Indirect Branch Tracking
x86/vdso: Insert endbr32/endbr64 to vDSO
x86/vdso/32: Add ENDBR to __kernel_vsyscall entry point

Yu-cheng Yu (7):
x86/cet/ibt: Add Kconfig option for Indirect Branch Tracking
x86/cet/ibt: Add user-mode Indirect Branch Tracking support
x86/cet/ibt: Handle signals for Indirect Branch Tracking
x86/cet/ibt: Disable IBT for ia32
x86/cet/ibt: Update ELF header parsing for Indirect Branch Tracking
x86/vdso: Introduce ENDBR macro
x86/vdso: Add ENDBR to __vdso_sgx_enter_enclave

arch/x86/Kconfig | 19 +++++
arch/x86/entry/vdso/Makefile | 4 +
arch/x86/entry/vdso/vdso32/system_call.S | 2 +
arch/x86/entry/vdso/vsgx.S | 4 +
arch/x86/ia32/ia32_signal.c | 22 +++++-
arch/x86/include/asm/cet.h | 13 ++++
arch/x86/include/asm/disabled-features.h | 8 +-
arch/x86/include/asm/elf.h | 13 +++-
arch/x86/include/asm/vdso.h | 20 ++++-
arch/x86/include/uapi/asm/ucontext.h | 5 ++
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/cet_prctl.c | 5 ++
arch/x86/kernel/ibt.c | 95 ++++++++++++++++++++++++
arch/x86/kernel/process_64.c | 8 ++
arch/x86/kernel/signal.c | 6 ++
15 files changed, 219 insertions(+), 6 deletions(-)
create mode 100644 arch/x86/kernel/ibt.c

--
2.21.0


2021-05-21 22:20:56

by Yu-cheng Yu

[permalink] [raw]
Subject: [PATCH v27 05/10] x86/cet/ibt: Update ELF header parsing for Indirect Branch Tracking

An ELF file's .note.gnu.property indicates features the file supports.
The property is parsed at loading time and passed to arch_setup_elf_
property(). Update it for Indirect Branch Tracking.

Signed-off-by: Yu-cheng Yu <[email protected]>
Cc: Kees Cook <[email protected]>
---
v27:
- Remove selecting of ARCH_USE_GNU_PROPERTY and ARCH_BINFMT_ELF_STATE,
since they are already selected by X86_64.

arch/x86/kernel/process_64.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 1742c16945ef..607b782afe2c 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -860,6 +860,14 @@ int arch_setup_elf_property(struct arch_elf_state *state)
if (state->gnu_property & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
r = shstk_setup();
}
+
+ if (r < 0)
+ return r;
+
+ if (cpu_feature_enabled(X86_FEATURE_IBT)) {
+ if (state->gnu_property & GNU_PROPERTY_X86_FEATURE_1_IBT)
+ r = ibt_setup();
+ }
#endif

return r;
--
2.21.0

2021-05-21 22:21:03

by Yu-cheng Yu

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

From: "H.J. Lu" <[email protected]>

Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect Branch
Tracking.

Signed-off-by: H.J. Lu <[email protected]>
Signed-off-by: Yu-cheng Yu <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---
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;
+
return copy_to_user(ubuf, buf, sizeof(buf));
}

@@ -46,6 +49,8 @@ int prctl_cet(int option, u64 arg2)
return -EINVAL;
if (arg2 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
shstk_disable();
+ if (arg2 & GNU_PROPERTY_X86_FEATURE_1_IBT)
+ ibt_disable();
return 0;

case ARCH_X86_CET_LOCK:
--
2.21.0

2021-07-20 02:05:31

by Edgecombe, Rick P

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

On Fri, 2021-05-21 at 15:15 -0700, Yu-cheng Yu wrote:
> From: "H.J. Lu" <[email protected]>
>
> Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect
> Branch
> Tracking.
>
> Signed-off-by: H.J. Lu <[email protected]>
> Signed-off-by: Yu-cheng Yu <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> ---
>  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;

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

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?

>         return copy_to_user(ubuf, buf, sizeof(buf));
>  }
>  
> @@ -46,6 +49,8 @@ int prctl_cet(int option, u64 arg2)
>                         return -EINVAL;
>                 if (arg2 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
>                         shstk_disable();
> +               if (arg2 & GNU_PROPERTY_X86_FEATURE_1_IBT)
> +                       ibt_disable();
>                 return 0;
>  
>         case ARCH_X86_CET_LOCK:

2021-07-20 17:17:11

by Yu-cheng Yu

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

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" <[email protected]>
>>
>> Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect
>> Branch
>> Tracking.
>>
>> Signed-off-by: H.J. Lu <[email protected]>
>> Signed-off-by: Yu-cheng Yu <[email protected]>
>> Reviewed-by: Kees Cook <[email protected]>
>> ---
>>  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

2021-07-20 19:51:34

by Edgecombe, Rick P

[permalink] [raw]
Subject: 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:
> > On Fri, 2021-05-21 at 15:15 -0700, Yu-cheng Yu wrote:
> > > From: "H.J. Lu" <[email protected]>
> > >
> > > Update ARCH_X86_CET_STATUS and ARCH_X86_CET_DISABLE for Indirect
> > > Branch
> > > Tracking.
> > >
> > > Signed-off-by: H.J. Lu <[email protected]>
> > > Signed-off-by: Yu-cheng Yu <[email protected]>
> > > Reviewed-by: Kees Cook <[email protected]>
> > > ---
> > >   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.

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

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