| Message ID | 20220929222936.14584-3-rick.p.edgecombe@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Shadowstacks for userspace | expand |
On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > 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 <yu-cheng.yu@intel.com> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Cc: Kees Cook <keescook@chromium.org> > > --- > > 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 >
On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > > 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 <yu-cheng.yu@intel.com> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Cc: Kees Cook <keescook@chromium.org> > > --- > > 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
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.
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.
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!
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.
On Thu, Sep 29, 2022 at 03:28:59PM -0700, Rick Edgecombe wrote: > From: Yu-cheng Yu <yu-cheng.yu@intel.com> > 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.
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 <yu-cheng.yu@intel.com> > > 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.
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.
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