[PATCH 0/4] mm: Avoid sharing high VMA flag bits

[PATCH 0/4] mm: Avoid sharing high VMA flag bits

[Florent Revest]

While staring at include/linux/mm.h, I was wondering why VM_UFFD_MINOR and
VM_SHADOW_STACK share the same bit on arm64. I think I gained enough confidence
now to call it a bug.

The first patch of this series is a straightforward attempt at fixing this
specific bug by changing the bit used by VM_UFFD_MINOR. I cc-ed stable on that
one and I expect it to not be all too controversial.

The rest of the series however is a more zealous refactoring and likely to be
more contentious... :) Since this bug looks like a near miss which could have
been quite severe in terms of security, I think it's worth trying to simplify
the high VMA flag bits code. I tried to consolidate around the current usage of
VM_HIGH_ARCH_* macros but I'm not sure if this is the preferred approach here. I
really don't feel strongly about those refactorings so this is more of a
platform for discussion for people with more mm background, I'll be more than
happy to respin a v2!

This series applies on v6.15-rc5.

Florent Revest (4):
  mm: fix VM_UFFD_MINOR == VM_SHADOW_STACK on USERFAULTFD=y &&
    ARM64_GCS=y
  mm: remove CONFIG_ARCH_USES_HIGH_VMA_FLAGS
  mm: use VM_HIGH_ARCH_* macros consistently
  mm: consolidate VM_HIGH_ARCH_* macros into parametric macros

 arch/arm64/Kconfig   |  3 ---
 arch/powerpc/Kconfig |  1 -
 arch/x86/Kconfig     |  2 --
 include/linux/mm.h   | 49 +++++++++++++++-----------------------------
 mm/Kconfig           |  2 --
 5 files changed, 17 insertions(+), 40 deletions(-)

-- 
2.49.0.967.g6a0df3ecc3-goog

[PATCH 1/4] mm: fix VM_UFFD_MINOR == VM_SHADOW_STACK on USERFAULTFD=y && ARM64_GCS=y

[Florent Revest]

On configs with CONFIG_ARM64_GCS=y, VM_SHADOW_STACK is bit 38.
On configs with CONFIG_HAVE_ARCH_USERFAULTFD_MINOR=y (selected by
CONFIG_ARM64 when CONFIG_USERFAULTFD=y), VM_UFFD_MINOR is _also_ bit 38.

This bit being shared by two different VMA flags could lead to all sorts
of unintended behaviors. Presumably, a process could maybe call into
userfaultfd in a way that disables the shadow stack vma flag. I can't
think of any attack where this would help (presumably, if an attacker
tries to disable shadow stacks, they are trying to hijack control flow
so can't arbitrarily call into userfaultfd yet anyway) but this still
feels somewhat scary.

Fixes: ae80e1629aea ("mm: Define VM_SHADOW_STACK for arm64 when we support GCS")
Cc: <stable@vger.kernel.org>
Signed-off-by: Florent Revest <revest@chromium.org>
---
 include/linux/mm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bf55206935c46..fdda6b16263b3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -385,7 +385,7 @@ extern unsigned int kobjsize(const void *objp);
 #endif
 
 #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
-# define VM_UFFD_MINOR_BIT	38
+# define VM_UFFD_MINOR_BIT	41
 # define VM_UFFD_MINOR		BIT(VM_UFFD_MINOR_BIT)	/* UFFD minor faults */
 #else /* !CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
 # define VM_UFFD_MINOR		VM_NONE
-- 
2.49.0.967.g6a0df3ecc3-goog

[PATCH 2/4] mm: remove CONFIG_ARCH_USES_HIGH_VMA_FLAGS

[Florent Revest]

Over the years, include/linux/mm.h has grown to use high VMA flags bits
in various ways. Some usages, like mseal(), are guarded by CONFIG_64BIT
while others like CONFIG_ARM64_MTE select
CONFIG_ARCH_USES_HIGH_VMA_FLAGS and depend on the VM_HIGH_ARCH* macros that
are guarded by that config.

Since CONFIG_ARCH_USES_HIGH_VMA_FLAGS only guards bit mask definitions,
it is safe to use on any CONFIG_64BIT config. Additionally, since all
configs that currently select CONFIG_ARCH_USES_HIGH_VMA_FLAGS depend on
CONFIG_64BIT, there should be no regressions.

Signed-off-by: Florent Revest <revest@chromium.org>
---
 arch/arm64/Kconfig   | 3 ---
 arch/powerpc/Kconfig | 1 -
 arch/x86/Kconfig     | 2 --
 include/linux/mm.h   | 4 ++--
 mm/Kconfig           | 2 --
 5 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a182295e6f08b..2a0d45b16b7fd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2149,7 +2149,6 @@ config ARM64_MTE
 	# Required for tag checking in the uaccess routines
 	select ARM64_PAN
 	select ARCH_HAS_SUBPAGE_FAULTS
-	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_USES_PG_ARCH_2
 	select ARCH_USES_PG_ARCH_3
 	help
@@ -2196,7 +2195,6 @@ menu "ARMv8.9 architectural features"
 config ARM64_POE
 	prompt "Permission Overlay Extension"
 	def_bool y
-	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_HAS_PKEYS
 	help
 	  The Permission Overlay Extension is used to implement Memory
@@ -2235,7 +2233,6 @@ config ARM64_GCS
 	bool "Enable support for Guarded Control Stack (GCS)"
 	default y
 	select ARCH_HAS_USER_SHADOW_STACK
-	select ARCH_USES_HIGH_VMA_FLAGS
 	depends on !UPROBES
 	help
 	  Guarded Control Stack (GCS) provides support for a separate
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6722625a406a0..e444191206b32 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1043,7 +1043,6 @@ config PPC_MEM_KEYS
 	def_bool y
 	depends on PPC_BOOK3S_64
 	depends on PPC_64S_HASH_MMU
-	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_HAS_PKEYS
 	help
 	  Memory Protection Keys provides a mechanism for enforcing
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b9f378e05f6b..e74ba77a066e6 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1818,7 +1818,6 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS
 	def_bool y
 	# Note: only available in 64-bit mode
 	depends on X86_64 && (CPU_SUP_INTEL || CPU_SUP_AMD)
-	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_HAS_PKEYS
 	help
 	  Memory Protection Keys provides a mechanism for enforcing
@@ -1900,7 +1899,6 @@ config X86_USER_SHADOW_STACK
 	bool "X86 userspace 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
diff --git a/include/linux/mm.h b/include/linux/mm.h
index fdda6b16263b3..da8f99a026deb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -297,7 +297,7 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_NOHUGEPAGE	0x40000000	/* MADV_NOHUGEPAGE marked this vma */
 #define VM_MERGEABLE	0x80000000	/* KSM may merge identical pages */
 
-#ifdef CONFIG_ARCH_USES_HIGH_VMA_FLAGS
+#ifdef CONFIG_64BIT
 #define VM_HIGH_ARCH_BIT_0	32	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_BIT_1	33	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_BIT_2	34	/* bit only usable on 64-bit architectures */
@@ -312,7 +312,7 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
 #define VM_HIGH_ARCH_5	BIT(VM_HIGH_ARCH_BIT_5)
 #define VM_HIGH_ARCH_6	BIT(VM_HIGH_ARCH_BIT_6)
-#endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
+#endif /* CONFIG_64BIT */
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
 # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
diff --git a/mm/Kconfig b/mm/Kconfig
index e113f713b4938..1fff7f8bfa96f 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1127,8 +1127,6 @@ config DEVICE_PRIVATE
 config VMAP_PFN
 	bool
 
-config ARCH_USES_HIGH_VMA_FLAGS
-	bool
 config ARCH_HAS_PKEYS
 	bool
 
-- 
2.49.0.967.g6a0df3ecc3-goog

[PATCH 3/4] mm: use VM_HIGH_ARCH_* macros consistently

[Florent Revest]

Currently, some high vma flag bits are defined using VM_HIGH_ARCH_*
helper macros but others are not. Use the macros consistently so it's
easier to spot which ones are used/free.

To keep the VMA flags space easier to read and think about, VM_SEALED is
also shifted down from a very high bit (63rd) to a lower one (42nd).

Signed-off-by: Florent Revest <revest@chromium.org>
---
 include/linux/mm.h | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index da8f99a026deb..b12549f0a6dce 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -305,6 +305,10 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_HIGH_ARCH_BIT_4	36	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_BIT_5	37	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_BIT_6	38	/* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_BIT_7	39	/* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_BIT_8	40	/* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_BIT_9	41	/* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_BIT_10	42	/* bit only usable on 64-bit architectures */
 #define VM_HIGH_ARCH_0	BIT(VM_HIGH_ARCH_BIT_0)
 #define VM_HIGH_ARCH_1	BIT(VM_HIGH_ARCH_BIT_1)
 #define VM_HIGH_ARCH_2	BIT(VM_HIGH_ARCH_BIT_2)
@@ -312,6 +316,10 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
 #define VM_HIGH_ARCH_5	BIT(VM_HIGH_ARCH_BIT_5)
 #define VM_HIGH_ARCH_6	BIT(VM_HIGH_ARCH_BIT_6)
+#define VM_HIGH_ARCH_7	BIT(VM_HIGH_ARCH_BIT_7)
+#define VM_HIGH_ARCH_8	BIT(VM_HIGH_ARCH_BIT_8)
+#define VM_HIGH_ARCH_9	BIT(VM_HIGH_ARCH_BIT_9)
+#define VM_HIGH_ARCH_10	BIT(VM_HIGH_ARCH_BIT_10)
 #endif /* CONFIG_64BIT */
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
@@ -385,8 +393,7 @@ extern unsigned int kobjsize(const void *objp);
 #endif
 
 #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
-# define VM_UFFD_MINOR_BIT	41
-# define VM_UFFD_MINOR		BIT(VM_UFFD_MINOR_BIT)	/* UFFD minor faults */
+# define VM_UFFD_MINOR		VM_HIGH_ARCH_9	/* UFFD minor faults */
 #else /* !CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
 # define VM_UFFD_MINOR		VM_NONE
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
@@ -399,15 +406,13 @@ extern unsigned int kobjsize(const void *objp);
  * if KVM does not lock down the memory type.
  */
 #ifdef CONFIG_64BIT
-#define VM_ALLOW_ANY_UNCACHED_BIT	39
-#define VM_ALLOW_ANY_UNCACHED		BIT(VM_ALLOW_ANY_UNCACHED_BIT)
+#define VM_ALLOW_ANY_UNCACHED		VM_HIGH_ARCH_7
 #else
 #define VM_ALLOW_ANY_UNCACHED		VM_NONE
 #endif
 
 #ifdef CONFIG_64BIT
-#define VM_DROPPABLE_BIT	40
-#define VM_DROPPABLE		BIT(VM_DROPPABLE_BIT)
+#define VM_DROPPABLE		VM_HIGH_ARCH_8
 #elif defined(CONFIG_PPC32)
 #define VM_DROPPABLE		VM_ARCH_1
 #else
@@ -416,7 +421,7 @@ extern unsigned int kobjsize(const void *objp);
 
 #ifdef CONFIG_64BIT
 /* VM is sealed, in vm_flags */
-#define VM_SEALED	_BITUL(63)
+#define VM_SEALED	VM_HIGH_ARCH_10
 #endif
 
 /* Bits set in the VMA until the stack is in its final location */
-- 
2.49.0.967.g6a0df3ecc3-goog

[PATCH 4/4] mm: consolidate VM_HIGH_ARCH_* macros into parametric macros

[Florent Revest]

This reduces code duplication and chances of mistakes.

Signed-off-by: Florent Revest <revest@chromium.org>
---
 include/linux/mm.h | 50 ++++++++++++++--------------------------------
 1 file changed, 15 insertions(+), 35 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index b12549f0a6dce..6750020d5ea37 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -298,42 +298,22 @@ extern unsigned int kobjsize(const void *objp);
 #define VM_MERGEABLE	0x80000000	/* KSM may merge identical pages */
 
 #ifdef CONFIG_64BIT
-#define VM_HIGH_ARCH_BIT_0	32	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_1	33	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_2	34	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_3	35	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_4	36	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_5	37	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_6	38	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_7	39	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_8	40	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_9	41	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_BIT_10	42	/* bit only usable on 64-bit architectures */
-#define VM_HIGH_ARCH_0	BIT(VM_HIGH_ARCH_BIT_0)
-#define VM_HIGH_ARCH_1	BIT(VM_HIGH_ARCH_BIT_1)
-#define VM_HIGH_ARCH_2	BIT(VM_HIGH_ARCH_BIT_2)
-#define VM_HIGH_ARCH_3	BIT(VM_HIGH_ARCH_BIT_3)
-#define VM_HIGH_ARCH_4	BIT(VM_HIGH_ARCH_BIT_4)
-#define VM_HIGH_ARCH_5	BIT(VM_HIGH_ARCH_BIT_5)
-#define VM_HIGH_ARCH_6	BIT(VM_HIGH_ARCH_BIT_6)
-#define VM_HIGH_ARCH_7	BIT(VM_HIGH_ARCH_BIT_7)
-#define VM_HIGH_ARCH_8	BIT(VM_HIGH_ARCH_BIT_8)
-#define VM_HIGH_ARCH_9	BIT(VM_HIGH_ARCH_BIT_9)
-#define VM_HIGH_ARCH_10	BIT(VM_HIGH_ARCH_BIT_10)
+#define VM_HIGH_ARCH_BIT(i)	(32+i)	/* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_(i)	BIT(VM_HIGH_ARCH_BIT(i))
 #endif /* CONFIG_64BIT */
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
 # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0
-# define VM_PKEY_BIT0  VM_HIGH_ARCH_0
-# define VM_PKEY_BIT1  VM_HIGH_ARCH_1
-# define VM_PKEY_BIT2  VM_HIGH_ARCH_2
+# define VM_PKEY_BIT0  VM_HIGH_ARCH(0)
+# define VM_PKEY_BIT1  VM_HIGH_ARCH(1)
+# define VM_PKEY_BIT2  VM_HIGH_ARCH(2)
 #if CONFIG_ARCH_PKEY_BITS > 3
-# define VM_PKEY_BIT3  VM_HIGH_ARCH_3
+# define VM_PKEY_BIT3  VM_HIGH_ARCH(3)
 #else
 # define VM_PKEY_BIT3  0
 #endif
 #if CONFIG_ARCH_PKEY_BITS > 4
-# define VM_PKEY_BIT4  VM_HIGH_ARCH_4
+# define VM_PKEY_BIT4  VM_HIGH_ARCH(4)
 #else
 # define VM_PKEY_BIT4  0
 #endif
@@ -349,7 +329,7 @@ extern unsigned int kobjsize(const void *objp);
  * (x86). See the comments near alloc_shstk() in arch/x86/kernel/shstk.c
  * for more details on the guard size.
  */
-# define VM_SHADOW_STACK	VM_HIGH_ARCH_5
+# define VM_SHADOW_STACK	VM_HIGH_ARCH(5)
 #endif
 
 #if defined(CONFIG_ARM64_GCS)
@@ -357,7 +337,7 @@ extern unsigned int kobjsize(const void *objp);
  * arm64's Guarded Control Stack implements similar functionality and
  * has similar constraints to shadow stacks.
  */
-# define VM_SHADOW_STACK	VM_HIGH_ARCH_6
+# define VM_SHADOW_STACK	VM_HIGH_ARCH(6)
 #endif
 
 #ifndef VM_SHADOW_STACK
@@ -381,8 +361,8 @@ extern unsigned int kobjsize(const void *objp);
 #endif
 
 #if defined(CONFIG_ARM64_MTE)
-# define VM_MTE		VM_HIGH_ARCH_4	/* Use Tagged memory for access control */
-# define VM_MTE_ALLOWED	VM_HIGH_ARCH_5	/* Tagged memory permitted */
+# define VM_MTE		VM_HIGH_ARCH(4)	/* Use Tagged memory for access control */
+# define VM_MTE_ALLOWED	VM_HIGH_ARCH(5)	/* Tagged memory permitted */
 #else
 # define VM_MTE		VM_NONE
 # define VM_MTE_ALLOWED	VM_NONE
@@ -393,7 +373,7 @@ extern unsigned int kobjsize(const void *objp);
 #endif
 
 #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
-# define VM_UFFD_MINOR		VM_HIGH_ARCH_9	/* UFFD minor faults */
+# define VM_UFFD_MINOR		VM_HIGH_ARCH(9)	/* UFFD minor faults */
 #else /* !CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
 # define VM_UFFD_MINOR		VM_NONE
 #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
@@ -406,13 +386,13 @@ extern unsigned int kobjsize(const void *objp);
  * if KVM does not lock down the memory type.
  */
 #ifdef CONFIG_64BIT
-#define VM_ALLOW_ANY_UNCACHED		VM_HIGH_ARCH_7
+#define VM_ALLOW_ANY_UNCACHED		VM_HIGH_ARCH(7)
 #else
 #define VM_ALLOW_ANY_UNCACHED		VM_NONE
 #endif
 
 #ifdef CONFIG_64BIT
-#define VM_DROPPABLE		VM_HIGH_ARCH_8
+#define VM_DROPPABLE		VM_HIGH_ARCH(8)
 #elif defined(CONFIG_PPC32)
 #define VM_DROPPABLE		VM_ARCH_1
 #else
@@ -421,7 +401,7 @@ extern unsigned int kobjsize(const void *objp);
 
 #ifdef CONFIG_64BIT
 /* VM is sealed, in vm_flags */
-#define VM_SEALED	VM_HIGH_ARCH_10
+#define VM_SEALED	VM_HIGH_ARCH(10)
 #endif
 
 /* Bits set in the VMA until the stack is in its final location */
-- 
2.49.0.967.g6a0df3ecc3-goog

Re: [PATCH 4/4] mm: consolidate VM_HIGH_ARCH_* macros into parametric macros

[Florent Revest]

On Tue, May 6, 2025 at 11:53 AM Florent Revest <revest@chromium.org> wrote:
>
> This reduces code duplication and chances of mistakes.
>
> Signed-off-by: Florent Revest <revest@chromium.org>
> ---
>  include/linux/mm.h | 50 ++++++++++++++--------------------------------
>  1 file changed, 15 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b12549f0a6dce..6750020d5ea37 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -298,42 +298,22 @@ extern unsigned int kobjsize(const void *objp);
>  #define VM_MERGEABLE   0x80000000      /* KSM may merge identical pages */
>
>  #ifdef CONFIG_64BIT
> -#define VM_HIGH_ARCH_BIT_0     32      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_1     33      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_2     34      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_3     35      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_4     36      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_5     37      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_6     38      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_7     39      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_8     40      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_9     41      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_BIT_10    42      /* bit only usable on 64-bit architectures */
> -#define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0)
> -#define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1)
> -#define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2)
> -#define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3)
> -#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
> -#define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5)
> -#define VM_HIGH_ARCH_6 BIT(VM_HIGH_ARCH_BIT_6)
> -#define VM_HIGH_ARCH_7 BIT(VM_HIGH_ARCH_BIT_7)
> -#define VM_HIGH_ARCH_8 BIT(VM_HIGH_ARCH_BIT_8)
> -#define VM_HIGH_ARCH_9 BIT(VM_HIGH_ARCH_BIT_9)
> -#define VM_HIGH_ARCH_10        BIT(VM_HIGH_ARCH_BIT_10)
> +#define VM_HIGH_ARCH_BIT(i)    (32+i)  /* bit only usable on 64-bit architectures */
> +#define VM_HIGH_ARCH_(i)       BIT(VM_HIGH_ARCH_BIT(i))

Argh, and of course I forgot to squash two local fixes before sending
the series out...

This should have been VM_HIGH_ARCH() here (the _ at the end is a typo)

>  #endif /* CONFIG_64BIT */
>
>  #ifdef CONFIG_ARCH_HAS_PKEYS
>  # define VM_PKEY_SHIFT VM_HIGH_ARCH_BIT_0

And this should have been a VM_HIGH_ARCH_BIT(0)

... Anyway, I think it still gets the point across that it could make
some sense to change those VM_HIGH_ARCH macros.

> -# define VM_PKEY_BIT0  VM_HIGH_ARCH_0
> -# define VM_PKEY_BIT1  VM_HIGH_ARCH_1
> -# define VM_PKEY_BIT2  VM_HIGH_ARCH_2
> +# define VM_PKEY_BIT0  VM_HIGH_ARCH(0)
> +# define VM_PKEY_BIT1  VM_HIGH_ARCH(1)
> +# define VM_PKEY_BIT2  VM_HIGH_ARCH(2)
>  #if CONFIG_ARCH_PKEY_BITS > 3
> -# define VM_PKEY_BIT3  VM_HIGH_ARCH_3
> +# define VM_PKEY_BIT3  VM_HIGH_ARCH(3)
>  #else
>  # define VM_PKEY_BIT3  0
>  #endif
>  #if CONFIG_ARCH_PKEY_BITS > 4
> -# define VM_PKEY_BIT4  VM_HIGH_ARCH_4
> +# define VM_PKEY_BIT4  VM_HIGH_ARCH(4)
>  #else
>  # define VM_PKEY_BIT4  0
>  #endif
> @@ -349,7 +329,7 @@ extern unsigned int kobjsize(const void *objp);
>   * (x86). See the comments near alloc_shstk() in arch/x86/kernel/shstk.c
>   * for more details on the guard size.
>   */
> -# define VM_SHADOW_STACK       VM_HIGH_ARCH_5
> +# define VM_SHADOW_STACK       VM_HIGH_ARCH(5)
>  #endif
>
>  #if defined(CONFIG_ARM64_GCS)
> @@ -357,7 +337,7 @@ extern unsigned int kobjsize(const void *objp);
>   * arm64's Guarded Control Stack implements similar functionality and
>   * has similar constraints to shadow stacks.
>   */
> -# define VM_SHADOW_STACK       VM_HIGH_ARCH_6
> +# define VM_SHADOW_STACK       VM_HIGH_ARCH(6)
>  #endif
>
>  #ifndef VM_SHADOW_STACK
> @@ -381,8 +361,8 @@ extern unsigned int kobjsize(const void *objp);
>  #endif
>
>  #if defined(CONFIG_ARM64_MTE)
> -# define VM_MTE                VM_HIGH_ARCH_4  /* Use Tagged memory for access control */
> -# define VM_MTE_ALLOWED        VM_HIGH_ARCH_5  /* Tagged memory permitted */
> +# define VM_MTE                VM_HIGH_ARCH(4) /* Use Tagged memory for access control */
> +# define VM_MTE_ALLOWED        VM_HIGH_ARCH(5) /* Tagged memory permitted */
>  #else
>  # define VM_MTE                VM_NONE
>  # define VM_MTE_ALLOWED        VM_NONE
> @@ -393,7 +373,7 @@ extern unsigned int kobjsize(const void *objp);
>  #endif
>
>  #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
> -# define VM_UFFD_MINOR         VM_HIGH_ARCH_9  /* UFFD minor faults */
> +# define VM_UFFD_MINOR         VM_HIGH_ARCH(9) /* UFFD minor faults */
>  #else /* !CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
>  # define VM_UFFD_MINOR         VM_NONE
>  #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
> @@ -406,13 +386,13 @@ extern unsigned int kobjsize(const void *objp);
>   * if KVM does not lock down the memory type.
>   */
>  #ifdef CONFIG_64BIT
> -#define VM_ALLOW_ANY_UNCACHED          VM_HIGH_ARCH_7
> +#define VM_ALLOW_ANY_UNCACHED          VM_HIGH_ARCH(7)
>  #else
>  #define VM_ALLOW_ANY_UNCACHED          VM_NONE
>  #endif
>
>  #ifdef CONFIG_64BIT
> -#define VM_DROPPABLE           VM_HIGH_ARCH_8
> +#define VM_DROPPABLE           VM_HIGH_ARCH(8)
>  #elif defined(CONFIG_PPC32)
>  #define VM_DROPPABLE           VM_ARCH_1
>  #else
> @@ -421,7 +401,7 @@ extern unsigned int kobjsize(const void *objp);
>
>  #ifdef CONFIG_64BIT
>  /* VM is sealed, in vm_flags */
> -#define VM_SEALED      VM_HIGH_ARCH_10
> +#define VM_SEALED      VM_HIGH_ARCH(10)
>  #endif
>
>  /* Bits set in the VMA until the stack is in its final location */
> --
> 2.49.0.967.g6a0df3ecc3-goog
>

Re: [PATCH 0/4] mm: Avoid sharing high VMA flag bits

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 338 bytes --]

On Tue, May 06, 2025 at 11:52:20AM +0200, Florent Revest wrote:

> While staring at include/linux/mm.h, I was wondering why VM_UFFD_MINOR and
> VM_SHADOW_STACK share the same bit on arm64. I think I gained enough confidence
> now to call it a bug.

Yes, it's a bug - it'll be an add/add conflict with those coming in via
different trees.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] mm: fix VM_UFFD_MINOR == VM_SHADOW_STACK on USERFAULTFD=y && ARM64_GCS=y

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Tue, May 06, 2025 at 11:52:21AM +0200, Florent Revest wrote:
> On configs with CONFIG_ARM64_GCS=y, VM_SHADOW_STACK is bit 38.
> On configs with CONFIG_HAVE_ARCH_USERFAULTFD_MINOR=y (selected by
> CONFIG_ARM64 when CONFIG_USERFAULTFD=y), VM_UFFD_MINOR is _also_ bit 38.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/4] mm: Avoid sharing high VMA flag bits

[Florent Revest]

On Tue, May 6, 2025 at 3:34 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, May 06, 2025 at 11:52:20AM +0200, Florent Revest wrote:
>
> > While staring at include/linux/mm.h, I was wondering why VM_UFFD_MINOR and
> > VM_SHADOW_STACK share the same bit on arm64. I think I gained enough confidence
> > now to call it a bug.
>
> Yes, it's a bug - it'll be an add/add conflict with those coming in via
> different trees.

Thanks for the review Mark! :)

I just want to make sure I fully understand the point you're making
here, it seems that you are suggesting that 7677f7fd8be7
("userfaultfd: add minor fault registration mode") and ae80e1629aea
("mm: Define VM_SHADOW_STACK for arm64 when we support GCS") came in
from two different trees and independently chose to use the same bit
around the ~same time, is that right ? And that a conflict would have
appeared when they'd eventually get merged into a shared tree ?

I don't think that's what happened in this specific case though. As
far as I can tell, the former was merged in 2021 and the latter was
merged in late 2024. Also, since the hunks got added in very different
parts of include/linux/mm.h, I don't think they caused a noticeable
merge conflict. But I agree it would probably be preferable if these
cases would cause some sort of noticeable merge conflict in the future
...

I'll quickly respin this series to address my typos on patch 4 (sigh)
and add your Reviewed-by tag but just to be clear, my refactorings in
patches 2/3/4 currently focus on using VM_HIGH_ARCH macros
consistently, to make it a bit more obvious to a reader if two
features choose the same bit. But maybe what we would really need
instead is a more obvious way for these bits to be mutually exclusive
and to cause merge conflicts if they get added through independent
trees ? For example, my colleague Brendan Jackman suggested using an
enum for VMA flags bit offsets but I'm not sure what the sentiment is
around that.

Re: [PATCH 0/4] mm: Avoid sharing high VMA flag bits

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2214 bytes --]

On Wed, May 07, 2025 at 03:09:50PM +0200, Florent Revest wrote:

> I just want to make sure I fully understand the point you're making
> here, it seems that you are suggesting that 7677f7fd8be7
> ("userfaultfd: add minor fault registration mode") and ae80e1629aea
> ("mm: Define VM_SHADOW_STACK for arm64 when we support GCS") came in
> from two different trees and independently chose to use the same bit
> around the ~same time, is that right ? And that a conflict would have
> appeared when they'd eventually get merged into a shared tree ?

> I don't think that's what happened in this specific case though. As
> far as I can tell, the former was merged in 2021 and the latter was
> merged in late 2024. Also, since the hunks got added in very different

Yes, indeed - I wrote that initial reply before I saw the actual change.
I wasn't expecting a conflict but rather that what'd happened was that
the bit was allocated independently in two different trees and then due
to the way the allocations are scattered everywhere no conflict had
flagged the issue.

> parts of include/linux/mm.h, I don't think they caused a noticeable
> merge conflict. But I agree it would probably be preferable if these
> cases would cause some sort of noticeable merge conflict in the future

Putting all the flags somewhere in the vicinity of each other would make
the whole thing much more robust, or having some build assert for
uniqueness.

> I'll quickly respin this series to address my typos on patch 4 (sigh)
> and add your Reviewed-by tag but just to be clear, my refactorings in
> patches 2/3/4 currently focus on using VM_HIGH_ARCH macros
> consistently, to make it a bit more obvious to a reader if two
> features choose the same bit. But maybe what we would really need
> instead is a more obvious way for these bits to be mutually exclusive
> and to cause merge conflicts if they get added through independent
> trees ? For example, my colleague Brendan Jackman suggested using an
> enum for VMA flags bit offsets but I'm not sure what the sentiment is
> around that.

I think we need something here, although it wasn't what actually
happened here the potential for the scenario I mentioned above to occur
remains.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]