Re: [PATCH v3 00/40] mm: make VMA flag semantics explicit, eliminate VM_SPECIAL

On Thu, 17 Sep 2026 17:22:09 +0100 "Lorenzo Stoakes (ARM)" <[email protected]> wrote:

> The VM_SPECIAL / VMA_SPECIAL_FLAGS mask conflates several unrelated
> properties:
> 
> * Is this kernel-owned, whether MMIO, kernel-allocated pages, or ordinary
>   pages a driver maps itself?
> * Can it be expanded or merged?
> * Is this a 'weird' case like mlock where migration might race and we
>   'have' to set invalid flags to notify?
> * Is it another 'weird' case where we just want to stop GUP from touching
>   it?
> 
> ...
> 
> This series brings some order to things by both limiting what drivers can
> do with VMA flags and switching to using predicates that describe
> behaviour, not arbitrary flags.

Thanks, I've updated mm.git's mm-unstable branch to this version.

> v3:
> * Fixed up bug in patch 1 as reported by Mike - have to delay setting
>   map->vma_flags until after action prepare, though map->vm_file needs to
>   be set before for correct reference count management.
> * Updated 4/40 to add a symmetric vm_end check as well as vm_start in case
>   of a dangerously insane driver, as per Sashiko.
> * Updated 8/40 to check if a driver did something REALLY stupid like having
>   a NULL discontig_kernel_page_ops ptr, as per Sashiko.
> * Updated 15/40 to trivially synchronise userland test comments.
> * Updated 17/40 to correctly duplicate code to the userland VMA tests as
>   per Sashiko.

Here's how v3 altered mm.git:


 mm/internal.h                   |    4 ++-
 mm/memory.c                     |    2 -
 mm/vma.c                        |   34 +++++++++++++++++-------------
 mm/vma.h                        |    3 +-
 tools/testing/vma/include/dup.h |   24 ++++++++++++++++-----
 5 files changed, 45 insertions(+), 22 deletions(-)

--- a/mm/internal.h~b
+++ a/mm/internal.h
@@ -244,6 +244,7 @@ static inline void vma_close(struct vm_a
 static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
 {
 	const unsigned long prev_start = vma->vm_start;
+	const unsigned long prev_end = vma->vm_end;
 	const vma_flags_t prev_flags = vma->flags;
 	int err;
 
@@ -263,9 +264,10 @@ static inline int mmap_file(struct file
 	if (unlikely(err))
 		return err;
 
-	err = mmap_hook_validate(prev_start, &prev_flags, vma);
+	err = mmap_hook_validate(prev_start, prev_end, &prev_flags, vma);
 	if (unlikely(err)) {
 		vma->vm_start = prev_start;
+		vma->vm_end = prev_end;
 		vma_close(vma);
 	}
 
--- a/mm/memory.c~b
+++ a/mm/memory.c
@@ -2653,7 +2653,7 @@ int map_discontig_kernel_pages_prepare(s
 		action->map_kernel_discontig.ops;
 
 	/* At minimum need to be able to get pages. */
-	if (WARN_ON_ONCE(!ops->get))
+	if (WARN_ON_ONCE(!ops || !ops->get))
 		return -EINVAL;
 
 	__map_kernel_pages_prepare(desc);
--- a/mm/vma.c~b
+++ a/mm/vma.c
@@ -2797,15 +2797,15 @@ static int mmap_validate_vma_flags(const
 }
 
 /* Check to ensure a driver hasn't done something crazy. */
-static int mmap_validate(unsigned long prev_start,
-			 unsigned long curr_start,
+static int mmap_validate(unsigned long prev_start, unsigned long prev_end,
+			 unsigned long curr_start, unsigned long curr_end,
 			 const vma_flags_t *prev_flags,
 			 const vma_flags_t *curr_flags)
 {
 	bool was_maywrite, is_maywrite;
 
-	/* Drivers cannot alter the address of the VMA. */
-	if (WARN_ON_ONCE(prev_start != curr_start))
+	/* Drivers cannot alter the range of the VMA. */
+	if (WARN_ON_ONCE(prev_start != curr_start || prev_end != curr_end))
 		return -EINVAL;
 
 	was_maywrite = vma_flags_test(prev_flags, VMA_MAYWRITE_BIT);
@@ -2843,7 +2843,8 @@ int mmap_prepare_validate(const struct v
 	    WARN_ON_ONCE(desc->action.type != MMAP_NOTHING))
 		return -EINVAL;
 
-	return mmap_validate(prev_desc->start, desc->start,
+	return mmap_validate(prev_desc->start, prev_desc->end,
+			     desc->start, desc->end,
 			     &prev_desc->vma_flags, &desc->vma_flags);
 }
 
@@ -2851,19 +2852,22 @@ int mmap_prepare_validate(const struct v
  * mmap_hook_validate() - Ensure the driver hasn't violated invariants in
  * its f_op->mmap hook.
  * @prev_start: The start of the mapping prior to the mmap hook.
+ * @prev_end: The end of the mapping prior to the mmap hook.
  * @prev_flags: The VMA flags set for the VMA prior to the mmap hook.
  * @vma: The VMA after the hook has been applied.
  *
  * Returns: 0 on success, otherwise an error.
  */
-int mmap_hook_validate(unsigned long prev_start,
+int mmap_hook_validate(unsigned long prev_start, unsigned long prev_end,
 		       const vma_flags_t *prev_flags,
 		       const struct vm_area_struct *vma)
 {
 	const unsigned long start = vma->vm_start;
+	const unsigned long end = vma->vm_end;
 	const vma_flags_t *flags = &vma->flags;
 
-	return mmap_validate(prev_start, start, prev_flags, flags);
+	return mmap_validate(prev_start, prev_end, start, end, prev_flags,
+			     flags);
 }
 
 static int call_action_prepare(struct mmap_state *map,
@@ -2900,15 +2904,9 @@ static int call_mmap_prepare(struct mmap
 	if (err)
 		return err;
 
-	/* Update fields permitted to be changed. */
-	map->pgoff = desc->pgoff;
+	/* Update first so file refcount tracked correctly. */
 	if (desc->vm_file != map->vm_file)
 		map->vm_file = desc->vm_file;
-	map->vma_flags = desc->vma_flags;
-	map->page_prot = desc->page_prot;
-	/* User-defined fields. */
-	map->vm_ops = desc->vm_ops;
-	map->vm_private_data = desc->private_data;
 
 	/* It's invalid for mmap_prepare hooks to clear vm_ops. */
 	if (!desc->vm_ops)
@@ -2923,6 +2921,14 @@ static int call_mmap_prepare(struct mmap
 	if (err)
 		return err;
 
+	/* Update fields permitted to be changed. */
+	map->pgoff = desc->pgoff;
+	map->vma_flags = desc->vma_flags;
+	map->page_prot = desc->page_prot;
+	/* User-defined fields. */
+	map->vm_ops = desc->vm_ops;
+	map->vm_private_data = desc->private_data;
+
 	/*
 	 * MAP_PRIVATE-/dev/zero mappings are an ancient way of getting
 	 * anonymous mappings. Rather than allowing these mappings to be odd
--- a/mm/vma.h~b
+++ a/mm/vma.h
@@ -786,7 +786,7 @@ void vm_area_free(struct vm_area_struct
 int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
 			  const struct vm_area_desc *desc);
 
-int mmap_hook_validate(unsigned long prev_start,
+int mmap_hook_validate(unsigned long prev_start, unsigned long prev_end,
 		       const vma_flags_t *prev_flags,
 		       const struct vm_area_struct *vma);
 
@@ -851,6 +851,7 @@ static inline int mmap_prepare_validate(
 }
 
 static inline int mmap_hook_validate(unsigned long prev_start,
+				     unsigned long prev_end,
 				     const vma_flags_t *prev_flags,
 				     const struct vm_area_struct *vma)
 {
--- a/tools/testing/vma/include/dup.h~b
+++ a/tools/testing/vma/include/dup.h
@@ -1667,22 +1667,36 @@ static inline bool vma_is_kernel_owned(c
 	return vma_flags_is_kernel_owned(&vma->flags);
 }
 
+static inline bool vma_flags_is_fixed_mapping(const vma_flags_t *flags)
+{
+	/*
+	 * VMA_PFNMAP_BIT should imply VMA_DONTEXPAND_BIT, but some callers set
+	 * only the former.
+	 */
+	return vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_DONTEXPAND_BIT);
+}
+
+static inline bool vma_is_fixed_mapping(const struct vm_area_struct *vma)
+{
+	return vma_flags_is_fixed_mapping(&vma->flags);
+}
+
 static inline bool vma_flags_can_merge(const vma_flags_t *flags)
 {
 	/*
-	 * VMA merging assumes that the properties of a VMA completely describe
-	 * the properties of that VMA.
+	 * VMA merging assumes that a VMA's flags and fields completely describe
+	 * its state.
 	 *
 	 * However, kernel-owned mappings may have established state upon mapping
 	 * not embodied in any attribute of the VMA.
 	 *
-	 * Additionally, PFN maps encode the source PFN of the range in
-	 * vma->vm_pgoff, which may otherwise cause spurious merges.
+	 * Additionally, private (CoW) PFN maps encode the source PFN of the
+	 * range in vma->vm_pgoff, which may otherwise cause spurious merges.
 	 */
 	if (vma_flags_is_kernel_owned(flags))
 		return false;
 	/* VMA explicitly marked as being unmergeable. */
-	if (vma_flags_test(flags, VMA_DONTEXPAND_BIT))
+	if (vma_flags_is_fixed_mapping(flags))
 		return false;
 
 	return true;
_