diff mbox series

[v10,12/40] mm: Define VM_SHADOW_STACK for arm64 when we support GCS

Message ID 20240801-arm64-gcs-v10-12-699e2bd2190b@kernel.org (mailing list archive)
State New, archived
Headers show
Series arm64/gcs: Provide support for GCS in userspace | expand

Commit Message

Mark Brown Aug. 1, 2024, 12:06 p.m. UTC
Use VM_HIGH_ARCH_5 for guarded control stack pages.

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/filesystems/proc.rst |  2 +-
 include/linux/mm.h                 | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Edgecombe, Rick P Aug. 15, 2024, 3:20 p.m. UTC | #1
On Thu, 2024-08-01 at 13:06 +0100, Mark Brown wrote:
> Use VM_HIGH_ARCH_5 for guarded control stack pages.

FYI - If you want to have more complete guard gaps, you need to do this for arm
too:
https://lore.kernel.org/linux-mm/20240326021656.202649-14-rick.p.edgecombe@intel.com/

Using VM_SHADOW_STACK only gets you part of the way there.
Mark Brown Aug. 15, 2024, 3:26 p.m. UTC | #2
On Thu, Aug 15, 2024 at 03:20:52PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2024-08-01 at 13:06 +0100, Mark Brown wrote:
> > Use VM_HIGH_ARCH_5 for guarded control stack pages.

> FYI - If you want to have more complete guard gaps, you need to do this for arm
> too:
> https://lore.kernel.org/linux-mm/20240326021656.202649-14-rick.p.edgecombe@intel.com/

> Using VM_SHADOW_STACK only gets you part of the way there.

Oh, thanks for the heads up - I'd missed that.
Mark Brown Aug. 15, 2024, 4:39 p.m. UTC | #3
On Thu, Aug 15, 2024 at 04:26:47PM +0100, Mark Brown wrote:
> On Thu, Aug 15, 2024 at 03:20:52PM +0000, Edgecombe, Rick P wrote:

> > FYI - If you want to have more complete guard gaps, you need to do this for arm
> > too:
> > https://lore.kernel.org/linux-mm/20240326021656.202649-14-rick.p.edgecombe@intel.com/

> > Using VM_SHADOW_STACK only gets you part of the way there.

> Oh, thanks for the heads up - I'd missed that.

Looking at this I think it makes sense to do as was done for x86 and
split this out into a separate series (part of why I'd missed it),
updating the generic implementation to do this by default.  That'll
touch a bunch of architectures and the series is already quite big,
it's not really an ABI impact.
Edgecombe, Rick P Aug. 15, 2024, 5:53 p.m. UTC | #4
On Thu, 2024-08-15 at 17:39 +0100, Mark Brown wrote:
> > Oh, thanks for the heads up - I'd missed that.
> 
> Looking at this I think it makes sense to do as was done for x86 and
> split this out into a separate series (part of why I'd missed it),
> updating the generic implementation to do this by default.  That'll
> touch a bunch of architectures and the series is already quite big,
> it's not really an ABI impact.

The series is already upstream. You just need to add an arm version of that
linked patch. But up to you.
Mark Brown Aug. 15, 2024, 6:19 p.m. UTC | #5
On Thu, Aug 15, 2024 at 05:53:19PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2024-08-15 at 17:39 +0100, Mark Brown wrote:

> > > Oh, thanks for the heads up - I'd missed that.

> > Looking at this I think it makes sense to do as was done for x86 and
> > split this out into a separate series (part of why I'd missed it),
> > updating the generic implementation to do this by default.  That'll
> > touch a bunch of architectures and the series is already quite big,
> > it's not really an ABI impact.

> The series is already upstream. You just need to add an arm version of that
> linked patch. But up to you.

Your series modified the existing x86 custom arch_get_unmapped_area*()
functions, arm64 uses the generic implementation of those so I'd have to
either add custom implementations (which I can't imagine would be met
with great enthusiasm) or update the generic ones.  A generic
implementation seems reasonable and it looks like RISC-V would also end
up using it so while it's a bit invasive it does seem more sensible to
do the change there.
Edgecombe, Rick P Aug. 16, 2024, 1:59 p.m. UTC | #6
On Thu, 2024-08-15 at 19:19 +0100, Mark Brown wrote:
> > The series is already upstream. You just need to add an arm version of that
> > linked patch. But up to you.
> 
> Your series modified the existing x86 custom arch_get_unmapped_area*()
> functions, arm64 uses the generic implementation of those so I'd have to
> either add custom implementations (which I can't imagine would be met
> with great enthusiasm) or update the generic ones.  A generic
> implementation seems reasonable and it looks like RISC-V would also end
> up using it so while it's a bit invasive it does seem more sensible to
> do the change there.

Ah, I misunderstood. Makes sense.
Catalin Marinas Aug. 19, 2024, 9:07 a.m. UTC | #7
On Thu, Aug 01, 2024 at 01:06:39PM +0100, Mark Brown wrote:
> Use VM_HIGH_ARCH_5 for guarded control stack pages.
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
diff mbox series

Patch

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index e834779d9611..6a882c57a7e7 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -579,7 +579,7 @@  encoded manner. The codes are the following:
     mt    arm64 MTE allocation tags are enabled
     um    userfaultfd missing tracking
     uw    userfaultfd wr-protect tracking
-    ss    shadow stack page
+    ss    shadow/guarded control stack page
     sl    sealed
     ==    =======================================
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 96faf26b6083..c6c7454ce4e0 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -353,7 +353,17 @@  extern unsigned int kobjsize(const void *objp);
  * for more details on the guard size.
  */
 # define VM_SHADOW_STACK	VM_HIGH_ARCH_5
-#else
+#endif
+
+#if defined(CONFIG_ARM64_GCS)
+/*
+ * arm64's Guarded Control Stack implements similar functionality and
+ * has similar constraints to shadow stacks.
+ */
+# define VM_SHADOW_STACK	VM_HIGH_ARCH_5
+#endif
+
+#ifndef VM_SHADOW_STACK
 # define VM_SHADOW_STACK	VM_NONE
 #endif