2022-09-29 22:45:46

by Edgecombe, Rick P

[permalink] [raw]
Subject: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

From: Yu-cheng Yu <[email protected]>

Shadow Stack provides protection against function return address
corruption. It is active when the processor supports it, the kernel has
CONFIG_X86_SHADOW_STACK enabled, and the application is built for the
feature. This is only implemented for the 64-bit kernel. When it is
enabled, legacy non-Shadow Stack applications continue to work, but without
protection.

Signed-off-by: Yu-cheng Yu <[email protected]>
Co-developed-by: Rick Edgecombe <[email protected]>
Signed-off-by: Rick Edgecombe <[email protected]>
Cc: Kees Cook <[email protected]>

---

v2:
- Remove already wrong kernel size increase info (tlgx)
- Change prompt to remove "Intel" (tglx)
- Update line about what CPUs are supported (Dave)

Yu-cheng v25:
- Remove X86_CET and use X86_SHADOW_STACK directly.

Yu-cheng v24:
- Update for the splitting X86_CET to X86_SHADOW_STACK and X86_IBT.

arch/x86/Kconfig | 18 ++++++++++++++++++
arch/x86/Kconfig.assembler | 5 +++++
2 files changed, 23 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f9920f1341c8..b68eb75887b8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -26,6 +26,7 @@ config X86_64
depends on 64BIT
# Options that are inherently 64-bit kernel only:
select ARCH_HAS_GIGANTIC_PAGE
+ select ARCH_HAS_SHADOW_STACK
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
select ARCH_USE_CMPXCHG_LOCKREF
select HAVE_ARCH_SOFT_DIRTY
@@ -1936,6 +1937,23 @@ config X86_SGX

If unsure, say N.

+config ARCH_HAS_SHADOW_STACK
+ def_bool n
+
+config X86_SHADOW_STACK
+ prompt "X86 Shadow Stack"
+ def_bool n
+ depends on ARCH_HAS_SHADOW_STACK
+ select ARCH_USES_HIGH_VMA_FLAGS
+ help
+ Shadow Stack protection is a hardware feature that detects function
+ return address corruption. Today the kernel's support is limited to
+ virtualizing it in KVM guests.
+
+ CPUs supporting shadow stacks were first released in 2020.
+
+ If unsure, say N.
+
config EFI
bool "EFI runtime service support"
depends on ACPI
diff --git a/arch/x86/Kconfig.assembler b/arch/x86/Kconfig.assembler
index 26b8c08e2fc4..00c79dd93651 100644
--- a/arch/x86/Kconfig.assembler
+++ b/arch/x86/Kconfig.assembler
@@ -19,3 +19,8 @@ config AS_TPAUSE
def_bool $(as-instr,tpause %ecx)
help
Supported by binutils >= 2.31.1 and LLVM integrated assembler >= V7
+
+config AS_WRUSS
+ def_bool $(as-instr,wrussq %rax$(comma)(%rbx))
+ help
+ Supported by binutils >= 2.31 and LLVM integrated assembler
--
2.17.1


2022-10-03 13:52:49

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote:
> From: Yu-cheng Yu <[email protected]>
>
> Shadow Stack provides protection against function return address
> corruption. It is active when the processor supports it, the kernel has
> CONFIG_X86_SHADOW_STACK enabled, and the application is built for the
> feature. This is only implemented for the 64-bit kernel. When it is
> enabled, legacy non-Shadow Stack applications continue to work, but without
> protection.
>
> Signed-off-by: Yu-cheng Yu <[email protected]>
> Co-developed-by: Rick Edgecombe <[email protected]>
> Signed-off-by: Rick Edgecombe <[email protected]>
> Cc: Kees Cook <[email protected]>
>
> ---
>
> v2:
> - Remove already wrong kernel size increase info (tlgx)
> - Change prompt to remove "Intel" (tglx)
> - Update line about what CPUs are supported (Dave)
>
> Yu-cheng v25:
> - Remove X86_CET and use X86_SHADOW_STACK directly.
>
> Yu-cheng v24:
> - Update for the splitting X86_CET to X86_SHADOW_STACK and X86_IBT.
>
> arch/x86/Kconfig | 18 ++++++++++++++++++
> arch/x86/Kconfig.assembler | 5 +++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f9920f1341c8..b68eb75887b8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -26,6 +26,7 @@ config X86_64
> depends on 64BIT
> # Options that are inherently 64-bit kernel only:
> select ARCH_HAS_GIGANTIC_PAGE
> + select ARCH_HAS_SHADOW_STACK
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> select ARCH_USE_CMPXCHG_LOCKREF
> select HAVE_ARCH_SOFT_DIRTY
> @@ -1936,6 +1937,23 @@ config X86_SGX
>
> If unsure, say N.
>
> +config ARCH_HAS_SHADOW_STACK
> + def_bool n

Hm. Shouldn't ARCH_HAS_SHADOW_STACK definition be in arch/Kconfig, not
under arch/x86?

Also, I think "def_bool n" has the same meaning as just "bool", no?

> +
> +config X86_SHADOW_STACK
> + prompt "X86 Shadow Stack"
> + def_bool n

Maybe just

bool "X86 Shadow Stack"

?

> + depends on ARCH_HAS_SHADOW_STACK
> + select ARCH_USES_HIGH_VMA_FLAGS
> + help
> + Shadow Stack protection is a hardware feature that detects function
> + return address corruption. Today the kernel's support is limited to
> + virtualizing it in KVM guests.
> +
> + CPUs supporting shadow stacks were first released in 2020.
> +
> + If unsure, say N.
> +
> config EFI
> bool "EFI runtime service support"
> depends on ACPI
> diff --git a/arch/x86/Kconfig.assembler b/arch/x86/Kconfig.assembler
> index 26b8c08e2fc4..00c79dd93651 100644
> --- a/arch/x86/Kconfig.assembler
> +++ b/arch/x86/Kconfig.assembler
> @@ -19,3 +19,8 @@ config AS_TPAUSE
> def_bool $(as-instr,tpause %ecx)
> help
> Supported by binutils >= 2.31.1 and LLVM integrated assembler >= V7
> +
> +config AS_WRUSS
> + def_bool $(as-instr,wrussq %rax$(comma)(%rbx))
> + help
> + Supported by binutils >= 2.31 and LLVM integrated assembler
> --
> 2.17.1
>

--
Kiryl Shutsemau / Kirill A. Shutemov

2022-10-03 18:13:23

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote:
> From: Yu-cheng Yu <[email protected]>
>
> Shadow Stack provides protection against function return address
> corruption. It is active when the processor supports it, the kernel has
> CONFIG_X86_SHADOW_STACK enabled, and the application is built for the
> feature. This is only implemented for the 64-bit kernel. When it is
> enabled, legacy non-Shadow Stack applications continue to work, but without
> protection.
>
> Signed-off-by: Yu-cheng Yu <[email protected]>
> Co-developed-by: Rick Edgecombe <[email protected]>
> Signed-off-by: Rick Edgecombe <[email protected]>
> Cc: Kees Cook <[email protected]>
>
> ---
>
> v2:
> - Remove already wrong kernel size increase info (tlgx)
> - Change prompt to remove "Intel" (tglx)
> - Update line about what CPUs are supported (Dave)
>
> Yu-cheng v25:
> - Remove X86_CET and use X86_SHADOW_STACK directly.
>
> Yu-cheng v24:
> - Update for the splitting X86_CET to X86_SHADOW_STACK and X86_IBT.
>
> arch/x86/Kconfig | 18 ++++++++++++++++++
> arch/x86/Kconfig.assembler | 5 +++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f9920f1341c8..b68eb75887b8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -26,6 +26,7 @@ config X86_64
> depends on 64BIT
> # Options that are inherently 64-bit kernel only:
> select ARCH_HAS_GIGANTIC_PAGE
> + select ARCH_HAS_SHADOW_STACK
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> select ARCH_USE_CMPXCHG_LOCKREF
> select HAVE_ARCH_SOFT_DIRTY
> @@ -1936,6 +1937,23 @@ config X86_SGX
>
> If unsure, say N.
>
> +config ARCH_HAS_SHADOW_STACK
> + def_bool n
> +
> +config X86_SHADOW_STACK
> + prompt "X86 Shadow Stack"
> + def_bool n

I hope we can switch this to "default y" soon, given it's a hardware
feature that is disabled at runtime when not available.

> + depends on ARCH_HAS_SHADOW_STACK

Doesn't this depend on AS_WRUSS too?

> + select ARCH_USES_HIGH_VMA_FLAGS
> + help
> + Shadow Stack protection is a hardware feature that detects function
> + return address corruption. Today the kernel's support is limited to
> + virtualizing it in KVM guests.
> +
> + CPUs supporting shadow stacks were first released in 2020.
> +
> + If unsure, say N.
> +
> config EFI
> bool "EFI runtime service support"
> depends on ACPI
> diff --git a/arch/x86/Kconfig.assembler b/arch/x86/Kconfig.assembler
> index 26b8c08e2fc4..00c79dd93651 100644
> --- a/arch/x86/Kconfig.assembler
> +++ b/arch/x86/Kconfig.assembler
> @@ -19,3 +19,8 @@ config AS_TPAUSE
> def_bool $(as-instr,tpause %ecx)
> help
> Supported by binutils >= 2.31.1 and LLVM integrated assembler >= V7
> +
> +config AS_WRUSS
> + def_bool $(as-instr,wrussq %rax$(comma)(%rbx))
> + help
> + Supported by binutils >= 2.31 and LLVM integrated assembler

Otherwise, I don't see anything else using OCNFIG_AS_WRUSS:

$ git grep AS_WRUSS
arch/x86/Kconfig.assembler:config AS_WRUSS

-Kees

--
Kees Cook

2022-10-03 20:21:25

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Mon, 2022-10-03 at 10:25 -0700, Kees Cook wrote:
> > +config X86_SHADOW_STACK
> > + prompt "X86 Shadow Stack"
> > + def_bool n
>
> I hope we can switch this to "default y" soon, given it's a hardware
> feature that is disabled at runtime when not available.

Hmm, yes. Not sure on this. I'm inclined to leave it as is for now.

>
> > + depends on ARCH_HAS_SHADOW_STACK
>
> Doesn't this depend on AS_WRUSS too?

Yes, this got messed up when this patch went to and from the CET KVM
series.

Thanks!

2022-10-03 20:28:46

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On 9/29/22 15:28, Rick Edgecombe wrote:
> +config X86_SHADOW_STACK
> + prompt "X86 Shadow Stack"
> + def_bool n
> + depends on ARCH_HAS_SHADOW_STACK
> + select ARCH_USES_HIGH_VMA_FLAGS
> + help
> + Shadow Stack protection is a hardware feature that detects function
> + return address corruption. Today the kernel's support is limited to
> + virtualizing it in KVM guests.
> +

Is this help text up to date? It seems a bit at odds with the series title.

2022-10-03 20:30:25

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Mon, 2022-10-03 at 16:40 +0300, Kirill A . Shutemov wrote:
> Hm. Shouldn't ARCH_HAS_SHADOW_STACK definition be in arch/Kconfig,
> not
> under arch/x86?
>
> Also, I think "def_bool n" has the same meaning as just "bool", no?
>
> > +
> > +config X86_SHADOW_STACK
> > + prompt "X86 Shadow Stack"
> > + def_bool n
>
> Maybe just
>
> bool "X86 Shadow Stack"
>
> ?
Yep, will change it. Thanks.

2022-10-03 20:48:42

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Mon, 2022-10-03 at 12:42 -0700, Dave Hansen wrote:
> On 9/29/22 15:28, Rick Edgecombe wrote:
> > +config X86_SHADOW_STACK
> > + prompt "X86 Shadow Stack"
> > + def_bool n
> > + depends on ARCH_HAS_SHADOW_STACK
> > + select ARCH_USES_HIGH_VMA_FLAGS
> > + help
> > + Shadow Stack protection is a hardware feature that detects
> > function
> > + return address corruption. Today the kernel's support is
> > limited to
> > + virtualizing it in KVM guests.
> > +
>
> Is this help text up to date? It seems a bit at odds with the series
> title.

Arg, yes. This patch got screwed up when I converted it back and forth
for the KVM series.

2022-10-12 20:37:15

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote:
> From: Yu-cheng Yu <[email protected]>

> Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

Please remove all "CET", "cet", etc strings from the text as that is
confusing. We should use either shadow stack or IBT and not CET.

> +config ARCH_HAS_SHADOW_STACK

Do I see it correctly that this thing is needed only once in
show_smap_vma_flags()?

If so, can we do a arch_show_smap_vma_flags(), call it at the end of
former function and avoid adding yet another Kconfig symbol?

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-10-13 01:43:46

by Edgecombe, Rick P

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Wed, 2022-10-12 at 22:04 +0200, Borislav Petkov wrote:
> On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote:
> > From: Yu-cheng Yu <[email protected]>
> > Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for
> > Shadow Stack
>
> Please remove all "CET", "cet", etc strings from the text as that is
> confusing. We should use either shadow stack or IBT and not CET.

Good point, I'll remove it. Thanks.

>
> > +config ARCH_HAS_SHADOW_STACK
>
> Do I see it correctly that this thing is needed only once in
> show_smap_vma_flags()?
>
> If so, can we do a arch_show_smap_vma_flags(), call it at the end of
> former function and avoid adding yet another Kconfig symbol?

Yea, I was thinking to maybe just change it to
CONFIG_X86_USER_SHADOW_STACK in show_smap_vma_flags(). In that function
there is already CONFIG_ARM64_BTI and CONFIG_ARM64_MTE.

I'm not sure if there is any aversion to having arch CONFIGs in core
code, but it's kind of nice to have all of the potentially conflicting
strings in once place.

2022-10-13 09:29:07

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v2 02/39] x86/cet/shstk: Add Kconfig option for Shadow Stack

On Thu, Oct 13, 2022 at 12:31:38AM +0000, Edgecombe, Rick P wrote:
> Yea, I was thinking to maybe just change it to
> CONFIG_X86_USER_SHADOW_STACK in show_smap_vma_flags(). In that function
> there is already CONFIG_ARM64_BTI and CONFIG_ARM64_MTE.

I was thinking exactly the same thing. :-)

> I'm not sure if there is any aversion to having arch CONFIGs in core
> code, but it's kind of nice to have all of the potentially conflicting
> strings in once place.

Yeah, ok.

I guess you can do the CONFIG_X86_USER_SHADOW_STACK thing for the sake
of simplicity. We have *waaay* too many Kconfig symbols and we should
introduce only the least amount of new ones.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette