| Message ID | 20200205181935.3712-6-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control-flow Enforcement: Shadow Stack | expand |
On Wed, Feb 05, 2020 at 10:19:13AM -0800, Yu-cheng Yu wrote: > Introduce Kconfig option: X86_INTEL_SHADOW_STACK_USER. > > Shadow Stack (SHSTK) provides protection against function return address > corruption. It is active when the kernel has this feature enabled, and > both the processor and the application support it. When this feature is > enabled, legacy non-SHSTK applications continue to work, but without SHSTK > protection. > > The user-mode SHSTK protection is only implemented for the 64-bit kernel. > IA32 applications are supported under the compatibility mode. > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > --- > arch/x86/Kconfig | 22 ++++++++++++++++++++++ > arch/x86/Makefile | 7 +++++++ > 2 files changed, 29 insertions(+) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 5e8949953660..6c34b701c588 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1974,6 +1974,28 @@ config X86_INTEL_TSX_MODE_AUTO > side channel attacks- equals the tsx=auto command line parameter. > endchoice > > +config X86_INTEL_CET > + def_bool n > + > +config ARCH_HAS_SHSTK > + def_bool n > + > +config X86_INTEL_SHADOW_STACK_USER > + prompt "Intel Shadow Stack for user-mode" > + def_bool n > + depends on CPU_SUP_INTEL && X86_64 > + select ARCH_USES_HIGH_VMA_FLAGS > + select X86_INTEL_CET > + select ARCH_HAS_SHSTK > + ---help--- > + Shadow Stack (SHSTK) provides protection against program > + stack corruption. It is active when the kernel has this > + feature enabled, and the processor and the application > + support it. When this feature is enabled, legacy non-SHSTK > + applications continue to work, but without SHSTK protection. > + > + If unsure, say y. > + > config EFI > bool "EFI runtime service support" > depends on ACPI > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > index 94df0868804b..c34f5befa4c8 100644 > --- a/arch/x86/Makefile > +++ b/arch/x86/Makefile > @@ -149,6 +149,13 @@ ifdef CONFIG_X86_X32 > endif > export CONFIG_X86_X32_ABI > > +# Check assembler Shadow Stack suppot > +ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER > + ifeq ($(call as-instr, saveprevssp, y),) This test needs to happen in the Kconfig rather than the Makefile; the CONFIG should be unavailable if AS doesn't support the feature. -Kees > + $(error CONFIG_X86_INTEL_SHADOW_STACK_USER not supported by the assembler) > + endif > +endif > + > # > # If the function graph tracer is used with mcount instead of fentry, > # '-maccumulate-outgoing-args' is needed to prevent a GCC bug > -- > 2.21.0 >
On 2/5/20 10:19 AM, Yu-cheng Yu wrote: > Introduce Kconfig option: X86_INTEL_SHADOW_STACK_USER. > > Shadow Stack (SHSTK) provides protection against function return address > corruption. It is active when the kernel has this feature enabled, and > both the processor and the application support it. When this feature is > enabled, legacy non-SHSTK applications continue to work, but without SHSTK > protection. > > The user-mode SHSTK protection is only implemented for the 64-bit kernel. > IA32 applications are supported under the compatibility mode. I think what you're trying to say here is that the hardware supports shadow stacks with 32-bit kernels. However, this series does not include that support and we have no plans to add it. Right? I'll let others weigh in, but I rather dislike the use of acronyms here. I'd much rather see the english "shadow stack" everywhere than SHSTK. > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 5e8949953660..6c34b701c588 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1974,6 +1974,28 @@ config X86_INTEL_TSX_MODE_AUTO > side channel attacks- equals the tsx=auto command line parameter. > endchoice > > +config X86_INTEL_CET > + def_bool n > + > +config ARCH_HAS_SHSTK > + def_bool n > + > +config X86_INTEL_SHADOW_STACK_USER > + prompt "Intel Shadow Stack for user-mode" Nit: this whole thing is to support more than a single stack. I'd make this plural at least in the text: "shadow stacks". > + def_bool n > + depends on CPU_SUP_INTEL && X86_64 > + select ARCH_USES_HIGH_VMA_FLAGS > + select X86_INTEL_CET > + select ARCH_HAS_SHSTK > + ---help--- > + Shadow Stack (SHSTK) provides protection against program > + stack corruption. It is active when the kernel has this > + feature enabled, and the processor and the application > + support it. When this feature is enabled, legacy non-SHSTK > + applications continue to work, but without SHSTK protection. > + > + If unsure, say y. This is missing a *lot* of information. What matters to someone turning this on? 1. It's a hardware feature. This only matters if you have the right hardware 2. It's a security hardening feature. You dance around this, but need to come out and say it. 3. Apps must be enabled to use it. You get no protection "for free" on old userspace. 4. The hardware supports user and kernel, but this option is for userspace only.
On 2/5/20 10:19 AM, Yu-cheng Yu wrote: > +# Check assembler Shadow Stack suppot ^ support > +ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER > + ifeq ($(call as-instr, saveprevssp, y),) > + $(error CONFIG_X86_INTEL_SHADOW_STACK_USER not supported by the assembler) > + endif > +endif Is this *just* looking for instruction support in the assembler? We usually just .byte them, like this for pkeys: asm volatile(".byte 0x0f,0x01,0xee\n\t" : "=a" (pkru), "=d" (edx) : "c" (ecx)); That way everybody with old toolchains can still build the kernel (and run/test code with your config option on, btw...).
> On 2/5/20 10:19 AM, Yu-cheng Yu wrote: > > Introduce Kconfig option: X86_INTEL_SHADOW_STACK_USER. > > > > Shadow Stack (SHSTK) provides protection against function return address > > corruption. It is active when the kernel has this feature enabled, and > > both the processor and the application support it. When this feature is > > enabled, legacy non-SHSTK applications continue to work, but without SHSTK > > protection. > > > > The user-mode SHSTK protection is only implemented for the 64-bit kernel. > > IA32 applications are supported under the compatibility mode. > > I think what you're trying to say here is that the hardware supports > shadow stacks with 32-bit kernels. However, this series does not > include that support and we have no plans to add it. > > Right? > > I'll let others weigh in, but I rather dislike the use of acronyms here. > I'd much rather see the english "shadow stack" everywhere than SHSTK. For the record, I like "shadow stack" better, too.
On Wed, Feb 26, 2020 at 10:05 AM Dave Hansen <dave.hansen@intel.com> wrote: > > On 2/5/20 10:19 AM, Yu-cheng Yu wrote: > > +# Check assembler Shadow Stack suppot > > ^ support > > > +ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER > > + ifeq ($(call as-instr, saveprevssp, y),) > > + $(error CONFIG_X86_INTEL_SHADOW_STACK_USER not supported by the assembler) > > + endif > > +endif > > Is this *just* looking for instruction support in the assembler? > > We usually just .byte them, like this for pkeys: > > asm volatile(".byte 0x0f,0x01,0xee\n\t" > : "=a" (pkru), "=d" (edx) > : "c" (ecx)); > > That way everybody with old toolchains can still build the kernel (and > run/test code with your config option on, btw...). CET requires a complete new OS image from kernel, toolchain, run-time. CET enabled kernel without the rest of updated OS won't give you CET at all.
On 2/26/20 5:02 PM, H.J. Lu wrote: >> That way everybody with old toolchains can still build the kernel (and >> run/test code with your config option on, btw...). > CET requires a complete new OS image from kernel, toolchain, run-time. > CET enabled kernel without the rest of updated OS won't give you CET > at all. If you require a new toolchain, nobody even builds your fancy feature. Probably including 0day and all of the lazy maintainers with crufty old distros. The point isn't to actually run CET at all. The point is to get as many people as possible testing as much of it as possible. Testing includes compile testing, static analysis and bloat watching. It also includes functional and performance testing when you've got the feature compiled in but unavailable at runtime. Did this hurt anything even when I'm not using it?
On Wed, Feb 26, 2020 at 5:16 PM Dave Hansen <dave.hansen@intel.com> wrote: > > On 2/26/20 5:02 PM, H.J. Lu wrote: > >> That way everybody with old toolchains can still build the kernel (and > >> run/test code with your config option on, btw...). > > CET requires a complete new OS image from kernel, toolchain, run-time. > > CET enabled kernel without the rest of updated OS won't give you CET > > at all. > > If you require a new toolchain, nobody even builds your fancy feature. > Probably including 0day and all of the lazy maintainers with crufty old > distros. GCC 8 or above is needed since vDSO must be compiled with --fcf-protection=branch. > The point isn't to actually run CET at all. The point is to get as many > people as possible testing as much of it as possible. Testing includes > compile testing, static analysis and bloat watching. It also includes > functional and performance testing when you've got the feature compiled > in but unavailable at runtime. Did this hurt anything even when I'm not > using it? > I will leave the CET toolchain issue to Yu-cheng.
> On Feb 26, 2020, at 6:11 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Feb 26, 2020 at 5:16 PM Dave Hansen <dave.hansen@intel.com> wrote: >> >> On 2/26/20 5:02 PM, H.J. Lu wrote: >>>> That way everybody with old toolchains can still build the kernel (and >>>> run/test code with your config option on, btw...). >>> CET requires a complete new OS image from kernel, toolchain, run-time. >>> CET enabled kernel without the rest of updated OS won't give you CET >>> at all. >> >> If you require a new toolchain, nobody even builds your fancy feature. >> Probably including 0day and all of the lazy maintainers with crufty old >> distros. > > GCC 8 or above is needed since vDSO must be compiled with > --fcf-protection=branch. Fair enough. I don’t particularly want to carry a gross hack to add the ENDBRANCHes without compiler support.
On 2/26/20 7:57 PM, Andy Lutomirski wrote: >> GCC 8 or above is needed since vDSO must be compiled with >> --fcf-protection=branch. > Fair enough. I don’t particularly want to carry a gross hack to add > the ENDBRANCHes without compiler support. Yeah, that's not worth it. But my main issue the shadow stack instructions: >> +ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER >> + ifeq ($(call as-instr, saveprevssp, y),) >> + $(error CONFIG_X86_INTEL_SHADOW_STACK_USER not supported by the assembler) >> + endif >> +endif Which are unrelated to ENDBRANCH. But, in any case, let's say Kconfig says we should try to use IBT, but we get to building the vDSO and don't have the right toolchain. Do we just stop the build? Or do we let the build go on and then decline to let folks enable IBT at runtime?
On Wed, 2020-02-26 at 09:03 -0800, Dave Hansen wrote: > On 2/5/20 10:19 AM, Yu-cheng Yu wrote: > > Introduce Kconfig option: X86_INTEL_SHADOW_STACK_USER. > > > > Shadow Stack (SHSTK) provides protection against function return address > > corruption. It is active when the kernel has this feature enabled, and > > both the processor and the application support it. When this feature is > > enabled, legacy non-SHSTK applications continue to work, but without SHSTK > > protection. > > > > The user-mode SHSTK protection is only implemented for the 64-bit kernel. > > IA32 applications are supported under the compatibility mode. > > I think what you're trying to say here is that the hardware supports > shadow stacks with 32-bit kernels. However, this series does not > include that support and we have no plans to add it. > > Right? Yes. > > I'll let others weigh in, but I rather dislike the use of acronyms here. > I'd much rather see the english "shadow stack" everywhere than SHSTK. I will change to shadow stack. > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 5e8949953660..6c34b701c588 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -1974,6 +1974,28 @@ config X86_INTEL_TSX_MODE_AUTO > > side channel attacks- equals the tsx=auto command line parameter. > > endchoice > > > > +config X86_INTEL_CET > > + def_bool n > > + > > +config ARCH_HAS_SHSTK > > + def_bool n > > + > > +config X86_INTEL_SHADOW_STACK_USER > > + prompt "Intel Shadow Stack for user-mode" > > Nit: this whole thing is to support more than a single stack. I'd make > this plural at least in the text: "shadow stacks". OK. > > > + def_bool n > > + depends on CPU_SUP_INTEL && X86_64 > > + select ARCH_USES_HIGH_VMA_FLAGS > > + select X86_INTEL_CET > > + select ARCH_HAS_SHSTK > > + ---help--- > > + Shadow Stack (SHSTK) provides protection against program > > + stack corruption. It is active when the kernel has this > > + feature enabled, and the processor and the application > > + support it. When this feature is enabled, legacy non-SHSTK > > + applications continue to work, but without SHSTK protection. > > + > > + If unsure, say y. > > This is missing a *lot* of information. > > What matters to someone turning this on? > > 1. It's a hardware feature. This only matters if you have the right > hardware > 2. It's a security hardening feature. You dance around this, but need > to come out and say it. > 3. Apps must be enabled to use it. You get no protection "for free" on > old userspace. > 4. The hardware supports user and kernel, but this option is for > userspace only. I will update the help text. Yu-cheng
On Wed, 2020-02-26 at 10:05 -0800, Dave Hansen wrote: > On 2/5/20 10:19 AM, Yu-cheng Yu wrote: > > +# Check assembler Shadow Stack suppot > > ^ support > > > +ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER > > + ifeq ($(call as-instr, saveprevssp, y),) > > + $(error CONFIG_X86_INTEL_SHADOW_STACK_USER not supported by the assembler) > > + endif > > +endif > > Is this *just* looking for instruction support in the assembler? > > We usually just .byte them, like this for pkeys: > > asm volatile(".byte 0x0f,0x01,0xee\n\t" > : "=a" (pkru), "=d" (edx) > : "c" (ecx)); > > That way everybody with old toolchains can still build the kernel (and > run/test code with your config option on, btw...). We used to do this for CET instructions, but after adding kernel-mode instructions and inserting ENDBR's, the code becomes cluttered. I also found an earlier discussion on the ENDBR: https://lore.kernel.org/lkml/CALCETrVRH8LeYoo7V1VBPqg4WS0Enxtizt=T7dPvgoeWfJrdzA@mail.gmail.com/ It makes sense to let the user know early on that the system cannot support CET and cannot build a CET-enabled kernel. One thing we can do is to disable CET in Kconfig and not in kernel build, which I will do in the next version. Yu-cheng
On 3/6/20 10:37 AM, Yu-cheng Yu wrote: > We used to do this for CET instructions, but after adding kernel-mode > instructions and inserting ENDBR's, the code becomes cluttered. I also > found an earlier discussion on the ENDBR: > > https://lore.kernel.org/lkml/CALCETrVRH8LeYoo7V1VBPqg4WS0Enxtizt=T7dPvgoeWfJrdzA@mail.gmail.com/ > > It makes sense to let the user know early on that the system cannot support > CET and cannot build a CET-enabled kernel. > > One thing we can do is to disable CET in Kconfig and not in kernel > build, which I will do in the next version. I'll go on the record and say I think we should allow building CET-enabled kernels on old toolchains. We need it for build test coverage. We can spit out a warning, but we need to allow building it. Andy L, do you have any heartburn with that?
On Fri, 2020-03-06 at 11:02 -0800, Dave Hansen wrote: > On 3/6/20 10:37 AM, Yu-cheng Yu wrote: > > We used to do this for CET instructions, but after adding kernel-mode > > instructions and inserting ENDBR's, the code becomes cluttered. I also > > found an earlier discussion on the ENDBR: > > > > https://lore.kernel.org/lkml/CALCETrVRH8LeYoo7V1VBPqg4WS0Enxtizt=T7dPvgoeWfJrdzA@mail.gmail.com/ > > > > It makes sense to let the user know early on that the system cannot support > > CET and cannot build a CET-enabled kernel. > > > > One thing we can do is to disable CET in Kconfig and not in kernel > > build, which I will do in the next version. > > I'll go on the record and say I think we should allow building > CET-enabled kernels on old toolchains. We need it for build test > coverage. We can spit out a warning, but we need to allow building it. The build test will go through (assembler or .byte), once the opcode patch is applied [1]. Also, when we enable kernel-mode CET, it is difficult to build IBT code without the right tool chain. Yu-cheng [1] opcode patch: https://lore.kernel.org/lkml/20200204171425.28073-1-yu-cheng.yu@intel.com/
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 5e8949953660..6c34b701c588 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1974,6 +1974,28 @@ config X86_INTEL_TSX_MODE_AUTO side channel attacks- equals the tsx=auto command line parameter. endchoice +config X86_INTEL_CET + def_bool n + +config ARCH_HAS_SHSTK + def_bool n + +config X86_INTEL_SHADOW_STACK_USER + prompt "Intel Shadow Stack for user-mode" + def_bool n + depends on CPU_SUP_INTEL && X86_64 + select ARCH_USES_HIGH_VMA_FLAGS + select X86_INTEL_CET + select ARCH_HAS_SHSTK + ---help--- + Shadow Stack (SHSTK) provides protection against program + stack corruption. It is active when the kernel has this + feature enabled, and the processor and the application + support it. When this feature is enabled, legacy non-SHSTK + applications continue to work, but without SHSTK protection. + + If unsure, say y. + config EFI bool "EFI runtime service support" depends on ACPI diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 94df0868804b..c34f5befa4c8 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -149,6 +149,13 @@ ifdef CONFIG_X86_X32 endif export CONFIG_X86_X32_ABI +# Check assembler Shadow Stack suppot +ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER + ifeq ($(call as-instr, saveprevssp, y),) + $(error CONFIG_X86_INTEL_SHADOW_STACK_USER not supported by the assembler) + endif +endif + # # If the function graph tracer is used with mcount instead of fentry, # '-maccumulate-outgoing-args' is needed to prevent a GCC bug
Introduce Kconfig option: X86_INTEL_SHADOW_STACK_USER. Shadow Stack (SHSTK) provides protection against function return address corruption. It is active when the kernel has this feature enabled, and both the processor and the application support it. When this feature is enabled, legacy non-SHSTK applications continue to work, but without SHSTK protection. The user-mode SHSTK protection is only implemented for the 64-bit kernel. IA32 applications are supported under the compatibility mode. Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> --- arch/x86/Kconfig | 22 ++++++++++++++++++++++ arch/x86/Makefile | 7 +++++++ 2 files changed, 29 insertions(+)