diff mbox series

[RFC,RFT,v2,1/5] mm: Introduce ARCH_HAS_USER_SHADOW_STACK

Message ID 20231114-clone3-shadow-stack-v2-1-b613f8681155@kernel.org (mailing list archive)
State New
Headers show
Series fork: Support shadow stacks in clone3() | expand

Commit Message

Mark Brown Nov. 14, 2023, 8:05 p.m. UTC
Since multiple architectures have support for shadow stacks and we need to
select support for this feature in several places in the generic code
provide a generic config option that the architectures can select.

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/x86/Kconfig   | 1 +
 fs/proc/task_mmu.c | 2 +-
 include/linux/mm.h | 2 +-
 mm/Kconfig         | 6 ++++++
 4 files changed, 9 insertions(+), 2 deletions(-)

Comments

Rick Edgecombe Nov. 14, 2023, 11:22 p.m. UTC | #1
On Tue, 2023-11-14 at 20:05 +0000, Mark Brown wrote:
> +config ARCH_HAS_USER_SHADOW_STACK
> +       bool
> +       help
> +         The architecture has hardware support for userspace shadow
> call
> +          stacks (eg, x86 CET, arm64 GCS, RISC-V Zisslpcfi).

I feel like normally a patch like this should come with the second
feature that gets enabled. (i.e. arm or riscv). Especially since the
comment lists currently unsupported features. I think something like
this got nixed by an x86 maintainer earlier, but that was before these
other features were getting pushed.

I don't have a strong objection to having it ahead of the other
features though and it is nice to remove X86 defines out of core code.
Mark Brown Nov. 15, 2023, 2:55 p.m. UTC | #2
On Tue, Nov 14, 2023 at 11:22:16PM +0000, Edgecombe, Rick P wrote:
> On Tue, 2023-11-14 at 20:05 +0000, Mark Brown wrote:
> > +config ARCH_HAS_USER_SHADOW_STACK
> > +       bool
> > +       help
> > +         The architecture has hardware support for userspace shadow
> > call
> > +          stacks (eg, x86 CET, arm64 GCS, RISC-V Zisslpcfi).

> I feel like normally a patch like this should come with the second
> feature that gets enabled. (i.e. arm or riscv). Especially since the
> comment lists currently unsupported features. I think something like
> this got nixed by an x86 maintainer earlier, but that was before these
> other features were getting pushed.

> I don't have a strong objection to having it ahead of the other
> features though and it is nice to remove X86 defines out of core code.

Given that the GCS patches are on the list and relatively
uncontroversial it does seem reasonable to carry this - I'm only able to
test this in the context of having both serieses applied!
David Hildenbrand Nov. 15, 2023, 3:12 p.m. UTC | #3
On 14.11.23 21:05, Mark Brown wrote:
> Since multiple architectures have support for shadow stacks and we need to
> select support for this feature in several places in the generic code
> provide a generic config option that the architectures can select.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   arch/x86/Kconfig   | 1 +
>   fs/proc/task_mmu.c | 2 +-
>   include/linux/mm.h | 2 +-
>   mm/Kconfig         | 6 ++++++
>   4 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 3762f41bb092..14b7703a9a2b 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1952,6 +1952,7 @@ config X86_USER_SHADOW_STACK
>   	depends on AS_WRUSS
>   	depends on X86_64
>   	select ARCH_USES_HIGH_VMA_FLAGS
> +	select ARCH_HAS_USER_SHADOW_STACK
>   	select X86_CET
>   	help
>   	  Shadow stack protection is a hardware feature that detects function
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index ef2eb12906da..f0a904aeee8e 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -699,7 +699,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
>   #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
>   		[ilog2(VM_UFFD_MINOR)]	= "ui",
>   #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
> -#ifdef CONFIG_X86_USER_SHADOW_STACK
> +#ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
>   		[ilog2(VM_SHADOW_STACK)] = "ss",
>   #endif
>   	};
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 418d26608ece..10462f354614 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -341,7 +341,7 @@ extern unsigned int kobjsize(const void *objp);
>   #endif
>   #endif /* CONFIG_ARCH_HAS_PKEYS */
>   
> -#ifdef CONFIG_X86_USER_SHADOW_STACK
> +#ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
>   /*
>    * VM_SHADOW_STACK should not be set with VM_SHARED because of lack of
>    * support core mm.
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 89971a894b60..b8638da636e1 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1270,6 +1270,12 @@ config LOCK_MM_AND_FIND_VMA
>   	bool
>   	depends on !STACK_GROWSUP
>   
> +config ARCH_HAS_USER_SHADOW_STACK
> +	bool
> +	help
> +	  The architecture has hardware support for userspace shadow call
> +          stacks (eg, x86 CET, arm64 GCS, RISC-V Zisslpcfi).
> +

Probably less controversial if we start with one example we have in 
place: "e.g., x86 CET". That should be sufficient to understand what 
this is about :)

Acked-by: David Hildenbrand <david@redhat.com>
Deepak Gupta Nov. 15, 2023, 3:36 p.m. UTC | #4
On Tue, Nov 14, 2023 at 08:05:54PM +0000, Mark Brown wrote:
>Since multiple architectures have support for shadow stacks and we need to
>select support for this feature in several places in the generic code
>provide a generic config option that the architectures can select.
>
>Suggested-by: David Hildenbrand <david@redhat.com>
>Signed-off-by: Mark Brown <broonie@kernel.org>
>---
> arch/x86/Kconfig   | 1 +
> fs/proc/task_mmu.c | 2 +-
> include/linux/mm.h | 2 +-
> mm/Kconfig         | 6 ++++++
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index 3762f41bb092..14b7703a9a2b 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -1952,6 +1952,7 @@ config X86_USER_SHADOW_STACK
> 	depends on AS_WRUSS
> 	depends on X86_64
> 	select ARCH_USES_HIGH_VMA_FLAGS
>+	select ARCH_HAS_USER_SHADOW_STACK
> 	select X86_CET
> 	help
> 	  Shadow stack protection is a hardware feature that detects function
>diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>index ef2eb12906da..f0a904aeee8e 100644
>--- a/fs/proc/task_mmu.c
>+++ b/fs/proc/task_mmu.c
>@@ -699,7 +699,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
> 		[ilog2(VM_UFFD_MINOR)]	= "ui",
> #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
>-#ifdef CONFIG_X86_USER_SHADOW_STACK
>+#ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
> 		[ilog2(VM_SHADOW_STACK)] = "ss",
> #endif
> 	};
>diff --git a/include/linux/mm.h b/include/linux/mm.h
>index 418d26608ece..10462f354614 100644
>--- a/include/linux/mm.h
>+++ b/include/linux/mm.h
>@@ -341,7 +341,7 @@ extern unsigned int kobjsize(const void *objp);
> #endif
> #endif /* CONFIG_ARCH_HAS_PKEYS */
>
>-#ifdef CONFIG_X86_USER_SHADOW_STACK
>+#ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
> /*
>  * VM_SHADOW_STACK should not be set with VM_SHARED because of lack of
>  * support core mm.
>diff --git a/mm/Kconfig b/mm/Kconfig
>index 89971a894b60..b8638da636e1 100644
>--- a/mm/Kconfig
>+++ b/mm/Kconfig
>@@ -1270,6 +1270,12 @@ config LOCK_MM_AND_FIND_VMA
> 	bool
> 	depends on !STACK_GROWSUP
>
>+config ARCH_HAS_USER_SHADOW_STACK
>+	bool
>+	help
>+	  The architecture has hardware support for userspace shadow call
>+          stacks (eg, x86 CET, arm64 GCS, RISC-V Zisslpcfi).

Minor correction for future version.RISC-V choose to split extension [1, 2].
It's now:

Zicfiss - CFI using shadow stack
Zicfilp - CFI using landing pad

So for shadow stack purposes, we can start saying "RISC-V Zicfiss"

[1] - https://lists.riscv.org/g/tech-ss-lp-cfi/message/55
[2] - https://github.com/riscv/riscv-cfi/pull/126

>+
> source "mm/damon/Kconfig"
>
> endmenu
>
>-- 
>2.30.2
>
diff mbox series

Patch

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3762f41bb092..14b7703a9a2b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1952,6 +1952,7 @@  config X86_USER_SHADOW_STACK
 	depends on AS_WRUSS
 	depends on X86_64
 	select ARCH_USES_HIGH_VMA_FLAGS
+	select ARCH_HAS_USER_SHADOW_STACK
 	select X86_CET
 	help
 	  Shadow stack protection is a hardware feature that detects function
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index ef2eb12906da..f0a904aeee8e 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -699,7 +699,7 @@  static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
 #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
 		[ilog2(VM_UFFD_MINOR)]	= "ui",
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
-#ifdef CONFIG_X86_USER_SHADOW_STACK
+#ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
 		[ilog2(VM_SHADOW_STACK)] = "ss",
 #endif
 	};
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 418d26608ece..10462f354614 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -341,7 +341,7 @@  extern unsigned int kobjsize(const void *objp);
 #endif
 #endif /* CONFIG_ARCH_HAS_PKEYS */
 
-#ifdef CONFIG_X86_USER_SHADOW_STACK
+#ifdef CONFIG_ARCH_HAS_USER_SHADOW_STACK
 /*
  * VM_SHADOW_STACK should not be set with VM_SHARED because of lack of
  * support core mm.
diff --git a/mm/Kconfig b/mm/Kconfig
index 89971a894b60..b8638da636e1 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1270,6 +1270,12 @@  config LOCK_MM_AND_FIND_VMA
 	bool
 	depends on !STACK_GROWSUP
 
+config ARCH_HAS_USER_SHADOW_STACK
+	bool
+	help
+	  The architecture has hardware support for userspace shadow call
+          stacks (eg, x86 CET, arm64 GCS, RISC-V Zisslpcfi).
+
 source "mm/damon/Kconfig"
 
 endmenu