[PATCH 0/6] mm: make MAP_PRIVATE-/dev/zero mappings truly anonymous

[PATCH 0/6] mm: make MAP_PRIVATE-/dev/zero mappings truly anonymous

[Lorenzo Stoakes (ARM)]

Historically anonymous memory was obtained in linux by MAP_PRIVATE-mapping
/dev/zero.

The canonical way of doing these now is mmap() specifying MAP_PRIVATE |
MAP_ANON, but we must continue to support the legacy means of obtaining these
mappings.

As-is these mappings are an unusual edge-case - they satisfy
vma_is_anonymous() but have non-NULL vma->vm_file, and their page offset is
the offset into the /dev/zero file.

Commit 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE
file-backed anon folios") causes all other anonymous folios to be tracked
by their anon index (vma->vm_start >> PAGE_SHIFT at the point of first
fault), leaving MAP_PRIVATE-/dev/zero as the outlier.

This series remedies the situation by making MAP_PRIVATE-/dev/zero mappings
truly anonymous with !vma->vm_file and correct anonymous page offset.

It starts by bringing the memory character driver into mm/ - this file
implements /dev/zero, /dev/mem among other things and is already (as
clearly indicated by its name) within the remit of memory management.

By doing this, the file_is_dev_zero() function can be provided, internal to
mm, which allows for positive identification of these mappings.

Using this, first prevent any other mappings from mapping memory
anonymously, then make these mappings truly anonymous and eliminate all
code in the kernel that previously had to account for these strange beasts.

Finally, it adds userland VMA tests to assert the behaviour and selftests
to assert expected merge behaviour.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
Lorenzo Stoakes (ARM) (6):
      mm: move drivers/char/mem.c to mm/char-mem.c
      mm: implement file_is_dev_zero() to uniquely identify /dev/zero
      mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous
      mm/vma: make MAP_PRIVATE-mapped /dev/zero mappings truly anonymous
      tools/testing/vma: add test to assert MAP_PRIVATE-/dev/zero is anon
      tools/testing/selftests/mm: add MAP_PRIVATE-/dev/zero merge tests

 MAINTAINERS                                        |   4 +-
 drivers/char/Makefile                              |   2 +-
 include/linux/mm.h                                 |  10 +-
 include/linux/pagemap.h                            |   3 +-
 mm/Makefile                                        |   3 +-
 drivers/char/mem.c => mm/char-mem.c                |  21 +++--
 mm/internal.h                                      |  20 ++--
 mm/shmem.c                                         |   2 +-
 mm/vma.c                                           |  39 ++++++--
 mm/vma.h                                           |   3 -
 tools/testing/selftests/mm/merge.c                 | 104 +++++++++++++++++++++
 .../selftests/proc/proc-self-map-files-001.c       |   2 +-
 .../selftests/proc/proc-self-map-files-002.c       |   2 +-
 tools/testing/vma/include/dup.h                    |  10 +-
 tools/testing/vma/shared.c                         |   9 ++
 tools/testing/vma/tests/mmap.c                     |  37 ++++++++
 16 files changed, 229 insertions(+), 42 deletions(-)
---
base-commit: e3b5239afe1b8f0194db7436b17c33e94c1988c4
change-id: 20260902-map-private-dev-zero-ba36d76a2fc8

Best regards,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>

[PATCH 1/6] mm: move drivers/char/mem.c to mm/char-mem.c

[Lorenzo Stoakes (ARM)]

The memory character driver implements several mm-specific features and is
always compiled into the kernel, so move it to mm/ where it belongs.

Among other things the driver implements /dev/mem which provides raw access
to physical memory, and /dev/zero which either allows mapping of a shmem
region (if mapped with MAP_SHARED) or, uniquely, anonymous memory (if
mapped MAP_PRIVATE).

This change lays the foundations to allow MAP_PRIVATE-/dev/zero to be
mapped precisely the same as anonymous memory is mapped as currently it is
an edge case within mm.

Also update a couple of comments that reference 'drivers/char/mem.c' to
reference 'mm/char-mem.c'.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 MAINTAINERS                         | 4 ++--
 drivers/char/Makefile               | 2 +-
 mm/Makefile                         | 3 ++-
 drivers/char/mem.c => mm/char-mem.c | 2 +-
 mm/shmem.c                          | 2 +-
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2133aec4a200..ebadf4cef076 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17272,7 +17272,7 @@ F:	Documentation/ABI/testing/sysfs-kernel-mm-memory-tiers
 F:	Documentation/ABI/testing/sysfs-kernel-mm-numa
 F:	Documentation/admin-guide/mm/
 F:	Documentation/mm/
-F:	drivers/char/mem.c
+F:	mm/char-mem.c
 F:	include/linux/cma.h
 F:	include/linux/dmapool.h
 F:	include/linux/ioremap.h
@@ -17477,7 +17477,7 @@ L:	linux-mm@kvack.org
 S:	Maintained
 W:	http://www.linux-mm.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
-F:	drivers/char/mem.c
+F:	mm/char-mem.c
 F:	include/trace/events/mmap.h
 F:	fs/proc/task_mmu.c
 F:	fs/proc/task_nommu.c
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index a46d7bf7c4c8..bb3bf66937b3 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -3,7 +3,7 @@
 # Makefile for the kernel character device drivers.
 #
 
-obj-y				+= mem.o random.o
+obj-y				+= random.o
 obj-$(CONFIG_TTY_PRINTK)	+= ttyprintk.o
 obj-y				+= misc.o
 obj-$(CONFIG_TEST_MISC_MINOR)	+= misc_minor_kunit.o
diff --git a/mm/Makefile b/mm/Makefile
index e7245cb88c66..2a3ec53d62ee 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -55,7 +55,8 @@ obj-y			:= filemap.o mempool.o oom_kill.o fadvise.o \
 			   mm_init.o percpu.o slab_common.o \
 			   compaction.o show_mem.o \
 			   interval_tree.o list_lru.o workingset.o \
-			   debug.o gup.o mmap_lock.o vma_init.o $(mmu-y)
+			   debug.o gup.o mmap_lock.o vma_init.o char-mem.o \
+			   $(mmu-y)
 
 # Give 'page_alloc' its own module-parameter namespace
 page-alloc-y := page_alloc.o
diff --git a/drivers/char/mem.c b/mm/char-mem.c
similarity index 99%
rename from drivers/char/mem.c
rename to mm/char-mem.c
index 63253d1de5d7..d2e575545044 100644
--- a/drivers/char/mem.c
+++ b/mm/char-mem.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- *  linux/drivers/char/mem.c
+ *  mm/char-mem.c
  *
  *  Copyright (C) 1991, 1992  Linus Torvalds
  *
diff --git a/mm/shmem.c b/mm/shmem.c
index 255d69ebceba..c92ed17dbc4a 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2981,7 +2981,7 @@ unsigned long shmem_get_unmapped_area(struct file *file,
 			sb = file_inode(file)->i_sb;
 		} else {
 			/*
-			 * Called directly from mm/mmap.c, or drivers/char/mem.c
+			 * Called directly from mm/mmap.c, or mm/char-mem.c
 			 * for "/dev/zero", to create a shared anonymous object.
 			 */
 			if (IS_ERR(shm_mnt))

-- 
2.55.0

[PATCH 2/6] mm: implement file_is_dev_zero() to uniquely identify /dev/zero

[Lorenzo Stoakes (ARM)]

To lay the foundation for a future change that converts
MAP_PRIVATE-/dev/zero mappings to be truly anonymous, add the ability to
uniquely identify these mappings.

With the memory character device now part of mm/ this is trivially
achievable through a file_is_dev_zero() predicate that simply tests that
the file operation hooks are zero_fops.

Also update userland VMA tests to expose file_is_dev_zero() and provide
a stub zero_fops for testing.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/char-mem.c                   | 13 +++++++++++++
 mm/internal.h                   |  3 +++
 tools/testing/vma/include/dup.h |  7 +++++++
 tools/testing/vma/shared.c      |  9 +++++++++
 4 files changed, 32 insertions(+)

diff --git a/mm/char-mem.c b/mm/char-mem.c
index d2e575545044..e53e89e6ddd8 100644
--- a/mm/char-mem.c
+++ b/mm/char-mem.c
@@ -31,6 +31,8 @@
 #include <linux/uaccess.h>
 #include <linux/security.h>
 
+#include "internal.h"
+
 #define DEVMEM_MINOR	1
 #define DEVPORT_MINOR	4
 
@@ -707,6 +709,17 @@ static const struct memdev {
 #endif
 };
 
+/**
+ * file_is_dev_zero() - is the specified @file associated with the /dev/zero
+ * driver?
+ * @file: File to test.
+ * Returns: true if it is, false otherwise.
+ */
+bool file_is_dev_zero(const struct file *file)
+{
+	return file && file->f_op == &zero_fops;
+}
+
 static int memory_open(struct inode *inode, struct file *filp)
 {
 	int minor;
diff --git a/mm/internal.h b/mm/internal.h
index e16f1250b25c..5d474e5f7709 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1638,4 +1638,7 @@ static inline bool can_spin_trylock(void)
 	return true;
 }
 
+/* char-mem.c */
+bool file_is_dev_zero(const struct file *file);
+
 #endif	/* __MM_INTERNAL_H */
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 57046d8ac81d..0d1a2ac88922 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1641,3 +1641,10 @@ static inline pgoff_t linear_anon_page_index(const struct vm_area_struct *vma,
 
 	return pgoff;
 }
+
+extern const struct file_operations zero_fops;
+
+static inline bool file_is_dev_zero(const struct file *file)
+{
+	return file && file->f_op == &zero_fops;
+}
diff --git a/tools/testing/vma/shared.c b/tools/testing/vma/shared.c
index 4a39c9d50489..8c4826499f40 100644
--- a/tools/testing/vma/shared.c
+++ b/tools/testing/vma/shared.c
@@ -12,6 +12,15 @@ const struct vm_operations_struct vma_dummy_vm_ops;
 struct anon_vma dummy_anon_vma;
 struct task_struct __current;
 
+static int mmap_zero_prepare(struct vm_area_desc *desc)
+{
+	return 0;
+}
+
+const struct file_operations zero_fops = {
+	.mmap_prepare = mmap_zero_prepare,
+};
+
 struct vm_area_struct *alloc_vma(struct mm_struct *mm,
 		unsigned long start, unsigned long end,
 		pgoff_t pgoff, vma_flags_t vma_flags)

-- 
2.55.0

[PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[Lorenzo Stoakes (ARM)]

In order to use mmap_prepare() with MAP_PRIVATE mappings of /dev/zero
without the success_hook hack we explicitly permitted mmap_prepare handlers
to set NULL vm_ops.

However this is dangerous and we really only want to allow this for
MAP_PRIVATE-mapped /dev/zero.

Therefore use the newly introduced file_is_dev_zero() to uniquely identify
MAP_PRIVATE-/dev/zero mappings and only permit this behaviour for them.

Then, remove all ability for mmap_prepare or mmap hooks to set a VMA
anonymous and update mmap_zero_prepare() to leave it to the core mmap code
to do so.

Note that this disallows nested MAP_PRIVATE-mappings of /dev/zero
regions. Doing this would be broken in any case.

We therefore do not need to update the mmap_prepare() compatibility layer
to reflect these changes, as the mmap hook check suffices to disallow this
behaviour.

Now we're setting vma->vm_ops to NULL for an mmap_prepare-initialised
MAP_PRIVATE-/dev/zero mapping, we have to avoid a subtle issue when
updating user-defined fields via set_vma_user_defined_fields().

The default for vma->vm_ops for all mmap_prepare-initialised mappings is
vma_dummy_vm_ops, so map->vm_ops will be set to this and setting
vma->vm_ops to this will render the VMA mistakenly non-anon.

In general, we should never be setting user-defined fields for an anonymous
VMA, so explicitly check for this to avoid doing so for the one case where
a mapping can be both mmap_prepare and anonymous.

In the case of legacy ->mmap hooks some drivers may set vma->vm_ops NULL
believing this is the equivalent of setting no VMA operations. Therefore
update mmap_file() to correct this by setting dummy VMA operations if this
occurs.

An example of this is drm_gem_shmem_mmap() which deliberately clears
vma->vm_ops before handing the VMA to dma-buf. Cases such as this will be
updated when they are converted to mmap_prepare.

Also, in order to avoid a single commit bisection hazard, add a temporary
workaround to set the VMA anonymous only after vma->vm_file is assigned in
__mmap_new_file_vma().

This is because vma_set_range() calls vma_set_pgoff() and
assert_sane_pgoff() in turn, prior to the vma->vm_file being assigned. If
we set the VMA anonymous early then this assert will fail.

This is removed in the subsequent commit.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/char-mem.c |  6 +-----
 mm/internal.h | 17 ++++++++++-------
 mm/vma.c      | 33 +++++++++++++++++++++++++--------
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/mm/char-mem.c b/mm/char-mem.c
index e53e89e6ddd8..c0b5fb019223 100644
--- a/mm/char-mem.c
+++ b/mm/char-mem.c
@@ -508,11 +508,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)
 	if (vma_desc_test(desc, VMA_SHARED_BIT))
 		return shmem_zero_setup_desc(desc);
 
-	/*
-	 * This is a highly unique situation where we mark a MAP_PRIVATE mapping
-	 * of /dev/zero anonymous, despite it not being.
-	 */
-	vma_desc_set_anonymous(desc);
+	/* MAP_PRIVATE semantics are taken care of for us by core mm. */
 	return 0;
 }
 
diff --git a/mm/internal.h b/mm/internal.h
index 5d474e5f7709..da14c56fb24e 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -226,15 +226,18 @@ static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
 {
 	int err = vfs_mmap(file, vma);
 
-	if (likely(!err))
-		return 0;
-
 	/*
-	 * OK, we tried to call the file hook for mmap(), but an error
-	 * arose. The mapping is in an inconsistent state and we must not invoke
-	 * any further hooks on it.
+	 * Either we tried to call the file hook for mmap() and an error arose
+	 * or a driver set vma->vm_ops = NULL intending there to be no VMA
+	 * operations.
+	 *
+	 * In the former case the VMA is in an inconsistent state and we mustn't
+	 * invoke any further hooks on it, in the latter case the hook actually
+	 * wanted no further hooks to be invoked, so fix both by setting dummy
+	 * VMA ops.
 	 */
-	vma->vm_ops = &vma_dummy_vm_ops;
+	if (unlikely(err || !vma->vm_ops))
+		vma->vm_ops = &vma_dummy_vm_ops;
 
 	return err;
 }
diff --git a/mm/vma.c b/mm/vma.c
index 35e7a64855fa..4b8d430d9619 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2621,6 +2621,19 @@ static int __mmap_new_file_vma(struct mmap_state *map,
 	return 0;
 }
 
+static bool map_is_private(const struct mmap_state *map)
+{
+	return !vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
+}
+
+static bool map_is_anon(const struct mmap_state *map)
+{
+	if (!map_is_private(map))
+		return false;
+
+	return !map->file || file_is_dev_zero(map->file);
+}
+
 /*
  * __mmap_new_vma() - Allocate a new VMA for the region, as merging was not
  * possible.
@@ -2634,8 +2647,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
 static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
 	struct mmap_action *action)
 {
-	const bool is_anon = !map->file &&
-		!vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
+	const bool is_anon = map_is_anon(map);
 	struct vma_iterator *vmi = map->vmi;
 	int error = 0;
 	struct vm_area_struct *vma;
@@ -2651,7 +2663,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
 
 	vma_iter_config(vmi, map->addr, map->end);
 
-	if (is_anon)
+	if (is_anon && !map->file)
 		vma_set_anonymous(vma);
 
 	vma_set_range(vma, map->addr, map->end, map->pgoff, map->anon_pgoff);
@@ -2669,6 +2681,10 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
 	else if (!is_anon)
 		error = shmem_zero_setup(vma);
 
+	/* Temporary MAP_PRIVATE-/dev/zero workaround. */
+	if (is_anon && map->file)
+		vma_set_anonymous(vma);
+
 	if (error)
 		goto free_iter_vma;
 
@@ -2777,6 +2793,10 @@ static int call_mmap_prepare(struct mmap_state *map,
 	if (err)
 		return err;
 
+	/* Hooks cannot mark themselves anonymous. */
+	if (!desc->vm_ops)
+		return -EINVAL;
+
 	err = call_action_prepare(map, desc);
 	if (err)
 		return err;
@@ -2799,10 +2819,7 @@ static int call_mmap_prepare(struct mmap_state *map,
 static void set_vma_user_defined_fields(struct vm_area_struct *vma,
 		struct mmap_state *map)
 {
-	if (map->vm_ops)
-		vma->vm_ops = map->vm_ops;
-	else	/* Only /dev/zero should do this. */
-		vma_set_anonymous(vma);
+	vma->vm_ops = map->vm_ops;
 	vma->vm_private_data = map->vm_private_data;
 }
 
@@ -2882,7 +2899,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
 		allocated_new = true;
 	}
 
-	if (have_mmap_prepare)
+	if (have_mmap_prepare && !map_is_anon(&map))
 		set_vma_user_defined_fields(vma, &map);
 
 	__mmap_complete(&map, vma);

-- 
2.55.0

[PATCH 4/6] mm/vma: make MAP_PRIVATE-mapped /dev/zero mappings truly anonymous

[Lorenzo Stoakes (ARM)]

When mapping /dev/zero with MAP_PRIVATE, one ends up with strange VMAs
originating from Linux's distant past.

These have vma->vm_file set but NULL vma->vm_ops, meaning they satisfy
vma_is_anonymous() but otherwise resemble a file-backed VMA.

The introduction of anonymous page offsets and their subsequent use as
indexes for MAP_PRIVATE-file-backed mappings mean the rmap does the right
thing with these but we are left with inconsistencies.

The vma_start_pgoff(vma) == vma_start_anon_pgoff(vma) invariant is true for
all other anonymous VMAs, but not these.

These VMAs are also observable as files in /proc/<pid>/[maps, smaps,
map_files] but otherwise behave like anonymous mappings.

Therefore let's make these VMAs actually anonymous at mapping time which
will activate the anonymous code path for mappings.

This means we no longer have to account for this discrepancy anywhere and
no longer have to think about these at all.

This is user-observable, as MAP_PRIVATE-/dev/zero will no longer appear in
procfs as a file-backed mapping, but the impact of this change should be
low as likely nobody is relying upon this.

However in any case, in using MAP_PRIVATE-/dev/zero they are explicitly
asking anonymous memory, so no longer seeing these as file mappings is in
fact correct.

A previous commit gave us file_is_dev_zero() to positively identify these
mappings, so we expressly only do so for these alone.

Update assert_sane_pgoff(), the comment for vma_start_pgoff() and
linear_anon_page_index() to reflect the change.

We make this change in call_mmap_prepare() alone as /dev/zero has been
converted to an mmap_prepare hook and we do not permit nested MAP_PRIVATE
mapping of /dev/zero.

We also remove the now defunct vma_desc_set_anonymous() and eliminate the
temporary bisection hazard fix from the previous commit.

Also update the VMA userland tests to reflect the change.

Finally, update the procfs self tests proc-self-map-files-001 and
proc-self-map-files-002 which both intend to map an arbitrary file
MAP_PRIVATE then assert procfs state, but happen to choose /dev/zero.

Fix them by updating these to /proc/self/exe which is guaranteed to be
present if procfs is mounted.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 include/linux/mm.h                                 | 10 ++-------
 include/linux/pagemap.h                            |  3 +--
 mm/vma.c                                           | 26 ++++++++++++++--------
 mm/vma.h                                           |  3 ---
 .../selftests/proc/proc-self-map-files-001.c       |  2 +-
 .../selftests/proc/proc-self-map-files-002.c       |  2 +-
 tools/testing/vma/include/dup.h                    |  3 +--
 7 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1b28e6fc8d5d..9238b7d52198 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1539,11 +1539,6 @@ static inline void vma_set_anonymous(struct vm_area_struct *vma)
 	vma->vm_ops = NULL;
 }
 
-static inline void vma_desc_set_anonymous(struct vm_area_desc *desc)
-{
-	desc->vm_ops = NULL;
-}
-
 static inline bool vma_is_anonymous(const struct vm_area_struct *vma)
 {
 	return !vma->vm_ops;
@@ -4405,9 +4400,8 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
  * If @vma is a MAP_PRIVATE file-backed mapping, then this returns the
  * page offset within the file.
  *
- * Edge cases: nommu does not abide by these, MAP_PRIVATE-/dev/zero satisfies
- * vma_is_anonymous() but has file-backed page offset, and MAP_PRIVATE-pfnmap
- * regions have their page offset set to the first PFN in the range.
+ * Edge cases: nommu does not abide by these and CoW MAP_PRIVATE-pfnmap regions
+ * have their page offset set to the first PFN in the range.
  *
  * Returns: The page offset of the start of @vma.
  */
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 0adfa6605653..939f3a5e973f 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -1128,8 +1128,7 @@ static inline pgoff_t linear_anon_page_index(const struct vm_area_struct *vma,
 	const pgoff_t pgoff = __linear_anon_page_index(vma, address);
 
 	VM_WARN_ON_ONCE(!vma_is_cow_mapping(vma));
-	/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */
-	if (vma_is_anonymous(vma) && !vma->vm_file)
+	if (vma_is_anonymous(vma))
 		VM_WARN_ON_ONCE(pgoff != linear_page_index(vma, address));
 
 	return pgoff;
diff --git a/mm/vma.c b/mm/vma.c
index 4b8d430d9619..fa5ee026e03a 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2621,6 +2621,13 @@ static int __mmap_new_file_vma(struct mmap_state *map,
 	return 0;
 }
 
+static void map_set_anon(struct mmap_state *map)
+{
+	map->file = NULL;
+	map->vm_ops = NULL;
+	map->pgoff = map->addr >> PAGE_SHIFT;
+}
+
 static bool map_is_private(const struct mmap_state *map)
 {
 	return !vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
@@ -2628,10 +2635,7 @@ static bool map_is_private(const struct mmap_state *map)
 
 static bool map_is_anon(const struct mmap_state *map)
 {
-	if (!map_is_private(map))
-		return false;
-
-	return !map->file || file_is_dev_zero(map->file);
+	return map_is_private(map) && !map->file;
 }
 
 /*
@@ -2663,7 +2667,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
 
 	vma_iter_config(vmi, map->addr, map->end);
 
-	if (is_anon && !map->file)
+	if (is_anon)
 		vma_set_anonymous(vma);
 
 	vma_set_range(vma, map->addr, map->end, map->pgoff, map->anon_pgoff);
@@ -2681,10 +2685,6 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
 	else if (!is_anon)
 		error = shmem_zero_setup(vma);
 
-	/* Temporary MAP_PRIVATE-/dev/zero workaround. */
-	if (is_anon && map->file)
-		vma_set_anonymous(vma);
-
 	if (error)
 		goto free_iter_vma;
 
@@ -2813,6 +2813,14 @@ static int call_mmap_prepare(struct mmap_state *map,
 	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
+	 * outliers, simply make them truly anonymous.
+	 */
+	if (map_is_private(map) && file_is_dev_zero(map->file))
+		map_set_anon(map);
+
 	return 0;
 }
 
diff --git a/mm/vma.h b/mm/vma.h
index 024fabe63560..e97bd2dfa786 100644
--- a/mm/vma.h
+++ b/mm/vma.h
@@ -267,9 +267,6 @@ static inline void assert_sane_pgoff(struct vm_area_struct *vma, pgoff_t pgoff)
 	 */
 	if (!vma_is_anonymous(vma))
 		return;
-	/* MAP_PRIVATE-/dev/zero is anon, non-NULL vm_file, but has file pgoff. */
-	if (vma->vm_file)
-		return;
 	/* If faulted in, could have been remapped. */
 	if (vma->anon_vma)
 		return;
diff --git a/tools/testing/selftests/proc/proc-self-map-files-001.c b/tools/testing/selftests/proc/proc-self-map-files-001.c
index 4209c64283d6..bbca9f9e2743 100644
--- a/tools/testing/selftests/proc/proc-self-map-files-001.c
+++ b/tools/testing/selftests/proc/proc-self-map-files-001.c
@@ -51,7 +51,7 @@ int main(void)
 	int fd;
 	unsigned long a, b;
 
-	fd = open("/dev/zero", O_RDONLY);
+	fd = open("/proc/self/exe", O_RDONLY);
 	if (fd == -1)
 		return 1;
 
diff --git a/tools/testing/selftests/proc/proc-self-map-files-002.c b/tools/testing/selftests/proc/proc-self-map-files-002.c
index e6aa00a183bc..5786cdffbbf6 100644
--- a/tools/testing/selftests/proc/proc-self-map-files-002.c
+++ b/tools/testing/selftests/proc/proc-self-map-files-002.c
@@ -57,7 +57,7 @@ int main(void)
 	int fd;
 	unsigned long a, b;
 
-	fd = open("/dev/zero", O_RDONLY);
+	fd = open("/proc/self/exe", O_RDONLY);
 	if (fd == -1)
 		return 1;
 
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index 0d1a2ac88922..16c09dac59d9 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -1635,8 +1635,7 @@ static inline pgoff_t linear_anon_page_index(const struct vm_area_struct *vma,
 	const pgoff_t pgoff = __linear_anon_page_index(vma, address);
 
 	VM_WARN_ON_ONCE(!vma_is_cow_mapping(vma));
-	/* Account for MAP_PRIVATE-/dev/zero which is only semi-anonymous. */
-	if (vma_is_anonymous(vma) && !vma->vm_file)
+	if (vma_is_anonymous(vma))
 		VM_WARN_ON_ONCE(pgoff != linear_page_index(vma, address));
 
 	return pgoff;

-- 
2.55.0

[PATCH 5/6] tools/testing/vma: add test to assert MAP_PRIVATE-/dev/zero is anon

[Lorenzo Stoakes (ARM)]

Now we've made MAP_PRIVATE-mapped /dev/zero mappings truly anonymous, add a
VMA userland test to assert that this is the case and everything is as we
would expect for an anonymous mapping.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 tools/testing/vma/tests/mmap.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/vma/tests/mmap.c b/tools/testing/vma/tests/mmap.c
index c85bc000d1cb..fa73faff2262 100644
--- a/tools/testing/vma/tests/mmap.c
+++ b/tools/testing/vma/tests/mmap.c
@@ -45,7 +45,44 @@ static bool test_mmap_region_basic(void)
 	return true;
 }
 
+static bool test_pure_anon_dev_zero(void)
+{
+	const vma_flags_t vma_flags = mk_vma_flags(VMA_READ_BIT, VMA_WRITE_BIT,
+			VMA_MAYREAD_BIT, VMA_MAYWRITE_BIT);
+	struct file file = {
+		.f_op = &zero_fops,
+	};
+	struct mm_struct mm = {};
+	struct vm_area_struct *vma;
+	unsigned long addr;
+	VMA_ITERATOR(vmi, &mm, 0);
+
+	current->mm = &mm;
+
+	/*
+	 * Map a MAP_PRIVATE-/dev/zero mapping at address 0x300000 with a page
+	 * offset of 0x10, which we expect to be reset to the anonymous page
+	 * offset.
+	 */
+	addr = __mmap_region(&file, 0x300000, 0x3000, vma_flags, 0x10, NULL);
+	ASSERT_EQ(addr, 0x300000);
+
+	/* Assert that it truly is an anonymous mapping. */
+	vma = vma_lookup(&mm, addr);
+	ASSERT_NE(vma, NULL);
+	ASSERT_TRUE(vma_is_anonymous(vma));
+	ASSERT_EQ(vma->vm_file, NULL);
+	ASSERT_EQ(vma->vm_private_data, NULL);
+	/* Expect anonymous page offsets. */
+	ASSERT_EQ(vma->vm_pgoff, 0x300);
+	ASSERT_EQ(vma_start_anon_pgoff(vma), 0x300);
+
+	cleanup_mm(&mm, &vmi);
+	return true;
+}
+
 static void run_mmap_tests(int *num_tests, int *num_fail)
 {
 	TEST(mmap_region_basic);
+	TEST(pure_anon_dev_zero);
 }

-- 
2.55.0

[PATCH 6/6] tools/testing/selftests/mm: add MAP_PRIVATE-/dev/zero merge tests

[Lorenzo Stoakes (ARM)]

Assert that MAP_PRIVATE-mapped /dev/zero mappings behave like they are
anonymous.

We test both unfaulted and faulted/unfaulted merges - each with the regions
having page offset of 0, which would not merge if the mappings were treated
as if they were file-backed.

With the recent change that makes them behave as pure anonymous mappings,
the merges should succeed as their page offsets are equal to their
anonymous page offsets.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 tools/testing/selftests/mm/merge.c | 104 +++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/tools/testing/selftests/mm/merge.c b/tools/testing/selftests/mm/merge.c
index 52b8727b6628..7c528d470404 100644
--- a/tools/testing/selftests/mm/merge.c
+++ b/tools/testing/selftests/mm/merge.c
@@ -1362,6 +1362,110 @@ TEST_F(merge, anon_and_page_offset_mismatch_memfd)
 	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 5 * page_size);
 }
 
+TEST_F(merge, merge_map_private_dev_zero_unfaulted)
+{
+	struct procmap_fd *procmap = &self->procmap;
+	unsigned int page_size = self->page_size;
+	char *carveout = self->carveout;
+	char *ptr, *ptr2;
+	int fd_zero;
+
+	if (access("/dev/zero", F_OK))
+		SKIP(return, "No /dev/zero.");
+	fd_zero = open("/dev/zero", O_RDWR);
+	ASSERT_NE(fd_zero, -1);
+
+	/*
+	 * Map two MAP_PRIVATE-/dev/zero VMAs next to one another with offset 0
+	 * each.
+	 *
+	 * With these being made truly anonymous upon mapping, they will
+	 * merge. If they were file-backed VMAs the page offsets would prevent
+	 * merge:
+	 *
+	 * |-----||------|    |-------------|
+	 * | ptr || ptr2 | -> |     ptr     |
+	 * |-----||------|    |-------------|
+	 */
+	ptr = mmap(carveout, 5 * page_size, PROT_READ | PROT_WRITE,
+		   MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
+	if (ptr == MAP_FAILED) {
+		close(fd_zero);
+		ASSERT_TRUE(false);
+	}
+	ptr2 = mmap(&carveout[5 * page_size], 5 * page_size,
+		   PROT_READ | PROT_WRITE, MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
+	if (ptr2 == MAP_FAILED) {
+		close(fd_zero);
+		ASSERT_TRUE(false);
+	}
+	close(fd_zero);
+
+	/* Assert that they merged. */
+	ASSERT_TRUE(find_vma_procmap(procmap, ptr));
+	ASSERT_EQ(procmap->query.vma_start, (unsigned long)ptr);
+	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 10 * page_size);
+}
+
+TEST_F(merge, merge_map_private_dev_zero_faulted_unfaulted)
+{
+	struct procmap_fd *procmap = &self->procmap;
+	unsigned int page_size = self->page_size;
+	char *carveout = self->carveout;
+	char *ptr, *ptr2;
+	int fd_zero;
+
+	if (access("/dev/zero", F_OK))
+		SKIP(return, "No /dev/zero.");
+	fd_zero = open("/dev/zero", O_RDWR);
+	ASSERT_NE(fd_zero, -1);
+
+	/*
+	 * Map a MAP_PRIVATE mapping of /dev/zero with page offset 0, then fault
+	 * it in:
+	 *
+	 * |-------------------------------|
+	 * |           faulted             |
+	 * |-------------------------------|
+	 */
+	ptr = mmap(carveout, 15 * page_size, PROT_READ | PROT_WRITE,
+		   MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
+	if (ptr == MAP_FAILED) {
+		close(fd_zero);
+		ASSERT_TRUE(false);
+	}
+	memset(ptr, 'x', 15 * page_size);
+
+	/*
+	 * Unmap the middle:
+	 *
+	 * |---------|           |---------|
+	 * | faulted |           | faulted |
+	 * |---------|           |---------|
+	 */
+	ASSERT_EQ(munmap(&ptr[5 * page_size], 5 * page_size), 0);
+
+	/*
+	 * Map in a new unfaulted mapping in the middle with page offset 0 -
+	 * this should merge and would not if it were treated as a file rather
+	 * than pure anon:
+	 *
+	 * |---------|-----------|---------|
+	 * | faulted | unfaulted | faulted |
+	 * |---------|-----------|---------|
+	 */
+	ptr2 = mmap(&carveout[5 * page_size], 5 * page_size,
+		    PROT_READ | PROT_WRITE, MAP_FIXED | MAP_PRIVATE,
+		    fd_zero, 0);
+	close(fd_zero);
+	ASSERT_NE(ptr2, MAP_FAILED);
+
+	/* Assert that they merged. */
+	ASSERT_TRUE(find_vma_procmap(procmap, ptr));
+	ASSERT_EQ(procmap->query.vma_start, (unsigned long)ptr);
+	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 15 * page_size);
+}
+
 TEST_F(merge_with_fork, mremap_faulted_to_unfaulted_prev)
 {
 	struct procmap_fd *procmap = &self->procmap;

-- 
2.55.0

Re: [PATCH 1/6] mm: move drivers/char/mem.c to mm/char-mem.c

[Mike Rapoport]

On Wed, Sep 02, 2026 at 07:00:18PM +0100, Lorenzo Stoakes (ARM) wrote:
> The memory character driver implements several mm-specific features and is
> always compiled into the kernel, so move it to mm/ where it belongs.
> 
> Among other things the driver implements /dev/mem which provides raw access
> to physical memory, and /dev/zero which either allows mapping of a shmem
> region (if mapped with MAP_SHARED) or, uniquely, anonymous memory (if
> mapped MAP_PRIVATE).
> 
> This change lays the foundations to allow MAP_PRIVATE-/dev/zero to be
> mapped precisely the same as anonymous memory is mapped as currently it is
> an edge case within mm.
> 
> Also update a couple of comments that reference 'drivers/char/mem.c' to
> reference 'mm/char-mem.c'.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  MAINTAINERS                         | 4 ++--
>  drivers/char/Makefile               | 2 +-
>  mm/Makefile                         | 3 ++-
>  drivers/char/mem.c => mm/char-mem.c | 2 +-
>  mm/shmem.c                          | 2 +-
>  5 files changed, 7 insertions(+), 6 deletions(-)

-- 
Sincerely yours,
Mike.

Re: [PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[Gregory Price]

On Wed, Sep 02, 2026 at 07:00:20PM +0100, Lorenzo Stoakes (ARM) wrote:
> In order to use mmap_prepare() with MAP_PRIVATE mappings of /dev/zero
> without the success_hook hack we explicitly permitted mmap_prepare handlers
> to set NULL vm_ops.
> 
> However this is dangerous and we really only want to allow this for
> MAP_PRIVATE-mapped /dev/zero.
> 

"this is dangerous" -> can you expand on this?

I had been experimenting with mmap'ing kmem dax devices as a way to test
generating a driver-defined efault mempolicy on an anonymous region, and
this exact pattern came up for me during mmap_prepare trying to get rid
of the "fileness" of the VMA.

Basically looked exactly like the /dev/zero vma.

I understand this is a hack, i'm just trying to better understand why
"this is dangerous" and it shouldn't be a supported pattern.

for clarity:

   fd = open("/dev/dax0.0",...);
   buf = mmap(fd, ...);
   /* 
    * mmap(_prepare) callback marks the vma anonymous so it takes anon
    * fault routes and sets an mbind mempolicy installed on the vma to
    * prefer the node the dax device is registered to.
    */
   buf[0] = 0xDEADBEEF; /* faults an anon page from the node */

Re: [PATCH 1/6] mm: move drivers/char/mem.c to mm/char-mem.c

[David Hildenbrand (Arm)]

On 9/2/26 20:00, Lorenzo Stoakes (ARM) wrote:
> The memory character driver implements several mm-specific features and is
> always compiled into the kernel, so move it to mm/ where it belongs.
> 
> Among other things the driver implements /dev/mem which provides raw access
> to physical memory, and /dev/zero which either allows mapping of a shmem
> region (if mapped with MAP_SHARED) or, uniquely, anonymous memory (if
> mapped MAP_PRIVATE).
> 
> This change lays the foundations to allow MAP_PRIVATE-/dev/zero to be
> mapped precisely the same as anonymous memory is mapped as currently it is
> an edge case within mm.
> 
> Also update a couple of comments that reference 'drivers/char/mem.c' to
> reference 'mm/char-mem.c'.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

Yes, that's the way to go

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH 2/6] mm: implement file_is_dev_zero() to uniquely identify /dev/zero

[David Hildenbrand (Arm)]

On 9/2/26 20:00, Lorenzo Stoakes (ARM) wrote:
> To lay the foundation for a future change that converts
> MAP_PRIVATE-/dev/zero mappings to be truly anonymous, add the ability to
> uniquely identify these mappings.
> 
> With the memory character device now part of mm/ this is trivially
> achievable through a file_is_dev_zero() predicate that simply tests that
> the file operation hooks are zero_fops.
> 
> Also update userland VMA tests to expose file_is_dev_zero() and provide
> a stub zero_fops for testing.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---

yes!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[Lorenzo Stoakes (ARM)]

On Mon, Sep 07, 2026 at 12:04:50PM -0400, Gregory Price wrote:
> On Wed, Sep 02, 2026 at 07:00:20PM +0100, Lorenzo Stoakes (ARM) wrote:
> > In order to use mmap_prepare() with MAP_PRIVATE mappings of /dev/zero
> > without the success_hook hack we explicitly permitted mmap_prepare handlers
> > to set NULL vm_ops.
> >
> > However this is dangerous and we really only want to allow this for
> > MAP_PRIVATE-mapped /dev/zero.
> >
>
> "this is dangerous" -> can you expand on this?
>
> I had been experimenting with mmap'ing kmem dax devices as a way to test
> generating a driver-defined efault mempolicy on an anonymous region, and
> this exact pattern came up for me during mmap_prepare trying to get rid
> of the "fileness" of the VMA.
>
> Basically looked exactly like the /dev/zero vma.
>
> I understand this is a hack, i'm just trying to better understand why
> "this is dangerous" and it shouldn't be a supported pattern.
>
> for clarity:
>
>    fd = open("/dev/dax0.0",...);
>    buf = mmap(fd, ...);
>    /*
>     * mmap(_prepare) callback marks the vma anonymous so it takes anon
>     * fault routes and sets an mbind mempolicy installed on the vma to
>     * prefer the node the dax device is registered to.
>     */
>    buf[0] = 0xDEADBEEF; /* faults an anon page from the node */

Well firstly it's a contradiction in terms as it has to be file-backed for a
driver to get it :)

This exception is just a historic artifact.

This kind of edge case VMA is a real footgun too, there's been bugs around it,
weird behaviour-by mistake and in general it's safer, more maintainable and
easier on the old noggin' to eliminate weirdo edge cases.

It's also probably a good idea from a security point of view, especially now
that can be LLM'd endlessly :)

But also the driver can't properly ensure that everything is set up correctly
re: rmap, mapcount, etc. and also drivers cannot be and should not be trusted to
do this.

The right away round here is for userland to allocate the memory and have a
driver use GUP to fiddle with it.

Finally CONFIG_DEBUG_VM warnings might go off because you set pgoff to something
random. We kinda tolerate pgoff abuse for file-backed memory, but now for anon
it's established as an invariant that it's what we expect (vm_start >>
PAGE_SHIFT if unfaulted, or if faulted from first fault time).

--
Cheers, Lorenzo

Re: [PATCH 2/6] mm: implement file_is_dev_zero() to uniquely identify /dev/zero

[Lorenzo Stoakes (ARM)]

On Mon, Sep 07, 2026 at 06:21:27PM +0200, David Hildenbrand (Arm) wrote:
> On 9/2/26 20:00, Lorenzo Stoakes (ARM) wrote:
> > To lay the foundation for a future change that converts
> > MAP_PRIVATE-/dev/zero mappings to be truly anonymous, add the ability to
> > uniquely identify these mappings.
> >
> > With the memory character device now part of mm/ this is trivially
> > achievable through a file_is_dev_zero() predicate that simply tests that
> > the file operation hooks are zero_fops.
> >
> > Also update userland VMA tests to expose file_is_dev_zero() and provide
> > a stub zero_fops for testing.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
>
> yes!
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

I thought you might like these ;)

Thanks for pushing back last cycle, this is MUCH better. Struck me that
actually that it's trivial + requires no fugliness the second we have it
mm/.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[David Hildenbrand (Arm)]

On 9/2/26 20:00, Lorenzo Stoakes (ARM) wrote:
> In order to use mmap_prepare() with MAP_PRIVATE mappings of /dev/zero
> without the success_hook hack we explicitly permitted mmap_prepare handlers
> to set NULL vm_ops.
> 
> However this is dangerous and we really only want to allow this for
> MAP_PRIVATE-mapped /dev/zero.
> 
> Therefore use the newly introduced file_is_dev_zero() to uniquely identify
> MAP_PRIVATE-/dev/zero mappings and only permit this behaviour for them.
> 
> Then, remove all ability for mmap_prepare or mmap hooks to set a VMA
> anonymous and update mmap_zero_prepare() to leave it to the core mmap code
> to do so.
> 
> Note that this disallows nested MAP_PRIVATE-mappings of /dev/zero
> regions. Doing this would be broken in any case.
> 
> We therefore do not need to update the mmap_prepare() compatibility layer
> to reflect these changes, as the mmap hook check suffices to disallow this
> behaviour.
> 
> Now we're setting vma->vm_ops to NULL for an mmap_prepare-initialised
> MAP_PRIVATE-/dev/zero mapping, we have to avoid a subtle issue when
> updating user-defined fields via set_vma_user_defined_fields().
> 
> The default for vma->vm_ops for all mmap_prepare-initialised mappings is
> vma_dummy_vm_ops, so map->vm_ops will be set to this and setting
> vma->vm_ops to this will render the VMA mistakenly non-anon.
> 
> In general, we should never be setting user-defined fields for an anonymous
> VMA, so explicitly check for this to avoid doing so for the one case where
> a mapping can be both mmap_prepare and anonymous.
> 
> In the case of legacy ->mmap hooks some drivers may set vma->vm_ops NULL
> believing this is the equivalent of setting no VMA operations. Therefore
> update mmap_file() to correct this by setting dummy VMA operations if this
> occurs.
> 
> An example of this is drm_gem_shmem_mmap() which deliberately clears
> vma->vm_ops before handing the VMA to dma-buf. Cases such as this will be
> updated when they are converted to mmap_prepare.
> 
> Also, in order to avoid a single commit bisection hazard, add a temporary
> workaround to set the VMA anonymous only after vma->vm_file is assigned in
> __mmap_new_file_vma().
> 
> This is because vma_set_range() calls vma_set_pgoff() and
> assert_sane_pgoff() in turn, prior to the vma->vm_file being assigned. If
> we set the VMA anonymous early then this assert will fail.
> 
> This is removed in the subsequent commit.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  mm/char-mem.c |  6 +-----
>  mm/internal.h | 17 ++++++++++-------
>  mm/vma.c      | 33 +++++++++++++++++++++++++--------
>  3 files changed, 36 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/char-mem.c b/mm/char-mem.c
> index e53e89e6ddd8..c0b5fb019223 100644
> --- a/mm/char-mem.c
> +++ b/mm/char-mem.c
> @@ -508,11 +508,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)
>  	if (vma_desc_test(desc, VMA_SHARED_BIT))
>  		return shmem_zero_setup_desc(desc);
>  
> -	/*
> -	 * This is a highly unique situation where we mark a MAP_PRIVATE mapping
> -	 * of /dev/zero anonymous, despite it not being.
> -	 */
> -	vma_desc_set_anonymous(desc);
> +	/* MAP_PRIVATE semantics are taken care of for us by core mm. */
>  	return 0;
>  }
>  
> diff --git a/mm/internal.h b/mm/internal.h
> index 5d474e5f7709..da14c56fb24e 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -226,15 +226,18 @@ static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
>  {
>  	int err = vfs_mmap(file, vma);
>  
> -	if (likely(!err))
> -		return 0;
> -
>  	/*
> -	 * OK, we tried to call the file hook for mmap(), but an error
> -	 * arose. The mapping is in an inconsistent state and we must not invoke
> -	 * any further hooks on it.
> +	 * Either we tried to call the file hook for mmap() and an error arose
> +	 * or a driver set vma->vm_ops = NULL intending there to be no VMA
> +	 * operations.
> +	 *
> +	 * In the former case the VMA is in an inconsistent state and we mustn't
> +	 * invoke any further hooks on it, in the latter case the hook actually
> +	 * wanted no further hooks to be invoked, so fix both by setting dummy
> +	 * VMA ops.
>  	 */
> -	vma->vm_ops = &vma_dummy_vm_ops;
> +	if (unlikely(err || !vma->vm_ops))
> +		vma->vm_ops = &vma_dummy_vm_ops;
>  
>  	return err;
>  }
> diff --git a/mm/vma.c b/mm/vma.c
> index 35e7a64855fa..4b8d430d9619 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2621,6 +2621,19 @@ static int __mmap_new_file_vma(struct mmap_state *map,
>  	return 0;
>  }
>  
> +static bool map_is_private(const struct mmap_state *map)
> +{
> +	return !vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
> +}
> +
> +static bool map_is_anon(const struct mmap_state *map)

I was wondering whether we should call this "map_is_private_anon", due to
MAP_ANON|MAP_SHARED. But looking at __mmap_new_vma(), the existing "is_anon" is
also limited to MAP_ANON|MAP_PRIVATE.

> +{
> +	if (!map_is_private(map))
> +		return false;
> +
> +	return !map->file || file_is_dev_zero(map->file);
> +}
> +
>  /*
>   * __mmap_new_vma() - Allocate a new VMA for the region, as merging was not
>   * possible.
> @@ -2634,8 +2647,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
>  static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
>  	struct mmap_action *action)
>  {
> -	const bool is_anon = !map->file &&
> -		!vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
> +	const bool is_anon = map_is_anon(map);
>  	struct vma_iterator *vmi = map->vmi;
>  	int error = 0;
>  	struct vm_area_struct *vma;
> @@ -2651,7 +2663,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
>  
>  	vma_iter_config(vmi, map->addr, map->end);
>  
> -	if (is_anon)
> +	if (is_anon && !map->file)
>  		vma_set_anonymous(vma);
>  
>  	vma_set_range(vma, map->addr, map->end, map->pgoff, map->anon_pgoff);
> @@ -2669,6 +2681,10 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
>  	else if (!is_anon)
>  		error = shmem_zero_setup(vma);
>  
> +	/* Temporary MAP_PRIVATE-/dev/zero workaround. */
> +	if (is_anon && map->file)
> +		vma_set_anonymous(vma);
> +
>  	if (error)
>  		goto free_iter_vma;
>  
> @@ -2777,6 +2793,10 @@ static int call_mmap_prepare(struct mmap_state *map,
>  	if (err)
>  		return err;
>  
> +	/* Hooks cannot mark themselves anonymous. */

I guess this comment will be stale soon (after #4 where you drop the
set_anonymous part).

Should it be

"vm_ops are strictly required with mmap_prepare"

or sth like that?

> +	if (!desc->vm_ops)
> +		return -EINVAL;
> +
>  	err = call_action_prepare(map, desc);
>  	if (err)
>  		return err;
> @@ -2799,10 +2819,7 @@ static int call_mmap_prepare(struct mmap_state *map,
>  static void set_vma_user_defined_fields(struct vm_area_struct *vma,
>  		struct mmap_state *map)
>  {
> -	if (map->vm_ops)
> -		vma->vm_ops = map->vm_ops;
> -	else	/* Only /dev/zero should do this. */
> -		vma_set_anonymous(vma);
> +	vma->vm_ops = map->vm_ops;
>  	vma->vm_private_data = map->vm_private_data;
>  }
>  
> @@ -2882,7 +2899,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  		allocated_new = true;
>  	}
>  
> -	if (have_mmap_prepare)
> +	if (have_mmap_prepare && !map_is_anon(&map))
>  		set_vma_user_defined_fields(vma, &map);

Ah, we have mmap_zero_prepare() for handling the shmem_zero_setup_desc(). I was
just about to ask whether we can just get rid of this here.


But, hold on, do we now even need that? Could core-mm now take care of that as
well, and we could just remove mmap_zero_prepare() entirely?

That is, we'd make shmem_zero_setup() in __mmap_new_vma() take care of this?
Then we might not even need shmem_zero_setup_desc() anymore.

Maybe harder than it sounds at first.

-- 
Cheers,

David

Re: [PATCH 6/6] tools/testing/selftests/mm: add MAP_PRIVATE-/dev/zero merge tests

[David Hildenbrand (Arm)]

On 9/2/26 20:00, Lorenzo Stoakes (ARM) wrote:
> Assert that MAP_PRIVATE-mapped /dev/zero mappings behave like they are
> anonymous.
> 
> We test both unfaulted and faulted/unfaulted merges - each with the regions
> having page offset of 0, which would not merge if the mappings were treated
> as if they were file-backed.
> 
> With the recent change that makes them behave as pure anonymous mappings,
> the merges should succeed as their page offsets are equal to their
> anonymous page offsets.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  tools/testing/selftests/mm/merge.c | 104 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/merge.c b/tools/testing/selftests/mm/merge.c
> index 52b8727b6628..7c528d470404 100644
> --- a/tools/testing/selftests/mm/merge.c
> +++ b/tools/testing/selftests/mm/merge.c
> @@ -1362,6 +1362,110 @@ TEST_F(merge, anon_and_page_offset_mismatch_memfd)
>  	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 5 * page_size);
>  }
>  
> +TEST_F(merge, merge_map_private_dev_zero_unfaulted)
> +{
> +	struct procmap_fd *procmap = &self->procmap;
> +	unsigned int page_size = self->page_size;
> +	char *carveout = self->carveout;
> +	char *ptr, *ptr2;
> +	int fd_zero;
> +
> +	if (access("/dev/zero", F_OK))
> +		SKIP(return, "No /dev/zero.");
> +	fd_zero = open("/dev/zero", O_RDWR);
> +	ASSERT_NE(fd_zero, -1);
> +
> +	/*
> +	 * Map two MAP_PRIVATE-/dev/zero VMAs next to one another with offset 0
> +	 * each.
> +	 *
> +	 * With these being made truly anonymous upon mapping, they will
> +	 * merge. If they were file-backed VMAs the page offsets would prevent
> +	 * merge:

Nit: "the" merge? You're the native speaker, so I don't know if what you have is
just correct :)

> +	 *
> +	 * |-----||------|    |-------------|
> +	 * | ptr || ptr2 | -> |     ptr     |
> +	 * |-----||------|    |-------------|
> +	 */
> +	ptr = mmap(carveout, 5 * page_size, PROT_READ | PROT_WRITE,
> +		   MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
> +	if (ptr == MAP_FAILED) {
> +		close(fd_zero);
> +		ASSERT_TRUE(false);
> +	}
> +	ptr2 = mmap(&carveout[5 * page_size], 5 * page_size,
> +		   PROT_READ | PROT_WRITE, MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
> +	if (ptr2 == MAP_FAILED) {
> +		close(fd_zero);

Is the close() really required before the ASSERT?  After all, you're also not
munmap'ing, so I wonder to which degree we have to clean up.

So maybe this could just become a

	ASSERT_NE(ptr2, MAP_FAILED);

Same for ptr above.

You could likely also do

ptr = mmap()
ptr2 = mmap()
close(fd_zero);

ASSERT_NE(ptr, MAP_FAILED);
ASSERT_NE(ptr2, MAP_FAILED);

> +		ASSERT_TRUE(false);
> +	}
> +	close(fd_zero);
> +
> +	/* Assert that they merged. */
> +	ASSERT_TRUE(find_vma_procmap(procmap, ptr));
> +	ASSERT_EQ(procmap->query.vma_start, (unsigned long)ptr);
> +	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 10 * page_size);
> +}
> +
> +TEST_F(merge, merge_map_private_dev_zero_faulted_unfaulted)
> +{
> +	struct procmap_fd *procmap = &self->procmap;
> +	unsigned int page_size = self->page_size;
> +	char *carveout = self->carveout;
> +	char *ptr, *ptr2;
> +	int fd_zero;
> +
> +	if (access("/dev/zero", F_OK))
> +		SKIP(return, "No /dev/zero.");
> +	fd_zero = open("/dev/zero", O_RDWR);
> +	ASSERT_NE(fd_zero, -1);
> +
> +	/*
> +	 * Map a MAP_PRIVATE mapping of /dev/zero with page offset 0, then fault
> +	 * it in:
> +	 *
> +	 * |-------------------------------|
> +	 * |           faulted             |
> +	 * |-------------------------------|
> +	 */
> +	ptr = mmap(carveout, 15 * page_size, PROT_READ | PROT_WRITE,
> +		   MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
> +	if (ptr == MAP_FAILED) {
> +		close(fd_zero);
> +		ASSERT_TRUE(false);

Same question regarding cleanup requirements. The ASSERT_TRUE(false) looks a bit
odd.

-- 
Cheers,

David

Re: [PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[Lorenzo Stoakes (ARM)]

On Mon, Sep 07, 2026 at 06:56:51PM +0200, David Hildenbrand (Arm) wrote:
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 35e7a64855fa..4b8d430d9619 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -2621,6 +2621,19 @@ static int __mmap_new_file_vma(struct mmap_state *map,
> >  	return 0;
> >  }
> >
> > +static bool map_is_private(const struct mmap_state *map)
> > +{
> > +	return !vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
> > +}
> > +
> > +static bool map_is_anon(const struct mmap_state *map)
>
> I was wondering whether we should call this "map_is_private_anon", due to
> MAP_ANON|MAP_SHARED. But looking at __mmap_new_vma(), the existing "is_anon" is
> also limited to MAP_ANON|MAP_PRIVATE.

A 'shared anon' mapping is not anon at all, and that's handled early in
do_mmap().

I wish that we didn't confuse people by allowing MAP_SHARED | MAP_ANON as a
shorthand but there we are.

So I don't like to make that distinction on the basis that a 'shared
anon' mapping isn't something that exists :)

And then imagine vma_is_private_anonymous() vs. vma_is_anonymous() etc. It would
get silly, quick...

>
> > +{
> > +	if (!map_is_private(map))
> > +		return false;
> > +
> > +	return !map->file || file_is_dev_zero(map->file);
> > +}
> > +
> >  /*
> >   * __mmap_new_vma() - Allocate a new VMA for the region, as merging was not
> >   * possible.
> > @@ -2634,8 +2647,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
> >  static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
> >  	struct mmap_action *action)
> >  {
> > -	const bool is_anon = !map->file &&
> > -		!vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
> > +	const bool is_anon = map_is_anon(map);
> >  	struct vma_iterator *vmi = map->vmi;
> >  	int error = 0;
> >  	struct vm_area_struct *vma;
> > @@ -2651,7 +2663,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
> >
> >  	vma_iter_config(vmi, map->addr, map->end);
> >
> > -	if (is_anon)
> > +	if (is_anon && !map->file)
> >  		vma_set_anonymous(vma);
> >
> >  	vma_set_range(vma, map->addr, map->end, map->pgoff, map->anon_pgoff);
> > @@ -2669,6 +2681,10 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
> >  	else if (!is_anon)
> >  		error = shmem_zero_setup(vma);
> >
> > +	/* Temporary MAP_PRIVATE-/dev/zero workaround. */
> > +	if (is_anon && map->file)
> > +		vma_set_anonymous(vma);
> > +
> >  	if (error)
> >  		goto free_iter_vma;
> >
> > @@ -2777,6 +2793,10 @@ static int call_mmap_prepare(struct mmap_state *map,
> >  	if (err)
> >  		return err;
> >
> > +	/* Hooks cannot mark themselves anonymous. */
>
> I guess this comment will be stale soon (after #4 where you drop the
> set_anonymous part).

Not really, it's there to catch drivers doing something silly/broken (likely by
mistake).

I want to catch that early. I have a 36 patch series that extends this kind of
idea... a lot :)

>
> Should it be
>
> "vm_ops are strictly required with mmap_prepare"
>
> or sth like that?

Well that's confusing though, because desc->vm_ops defaults to &dummy_vma_ops,
and we absolutely do not require drivers to set vm_ops at all.

And as far as the driver is concerned maybe it's NULL? They maybe don't realise
:)

So the idea is to say don't allow them to try to do something they can't do.

>
> > +	if (!desc->vm_ops)
> > +		return -EINVAL;
> > +
> >  	err = call_action_prepare(map, desc);
> >  	if (err)
> >  		return err;
> > @@ -2799,10 +2819,7 @@ static int call_mmap_prepare(struct mmap_state *map,
> >  static void set_vma_user_defined_fields(struct vm_area_struct *vma,
> >  		struct mmap_state *map)
> >  {
> > -	if (map->vm_ops)
> > -		vma->vm_ops = map->vm_ops;
> > -	else	/* Only /dev/zero should do this. */
> > -		vma_set_anonymous(vma);
> > +	vma->vm_ops = map->vm_ops;
> >  	vma->vm_private_data = map->vm_private_data;
> >  }
> >
> > @@ -2882,7 +2899,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
> >  		allocated_new = true;
> >  	}
> >
> > -	if (have_mmap_prepare)
> > +	if (have_mmap_prepare && !map_is_anon(&map))
> >  		set_vma_user_defined_fields(vma, &map);
>
> Ah, we have mmap_zero_prepare() for handling the shmem_zero_setup_desc(). I was
> just about to ask whether we can just get rid of this here.
>
>
> But, hold on, do we now even need that? Could core-mm now take care of that as
> well, and we could just remove mmap_zero_prepare() entirely?
>
> That is, we'd make shmem_zero_setup() in __mmap_new_vma() take care of this?
> Then we might not even need shmem_zero_setup_desc() anymore.
>
> Maybe harder than it sounds at first.

I think I'd rather that be a follow up :) this series is about eliminiating the
one last (I hope?) corner case for anon VMAs.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[David Hildenbrand (Arm)]

>> I was wondering whether we should call this "map_is_private_anon", due to
>> MAP_ANON|MAP_SHARED. But looking at __mmap_new_vma(), the existing "is_anon" is
>> also limited to MAP_ANON|MAP_PRIVATE.
> 
> A 'shared anon' mapping is not anon at all, and that's handled early in
> do_mmap().
> 
> I wish that we didn't confuse people by allowing MAP_SHARED | MAP_ANON as a
> shorthand but there we are.
> 
> So I don't like to make that distinction on the basis that a 'shared
> anon' mapping isn't something that exists :)
> 
> And then imagine vma_is_private_anonymous() vs. vma_is_anonymous() etc. It would
> get silly, quick...
> 

Yeah, agreed.

>>
>>> +{
>>> +	if (!map_is_private(map))
>>> +		return false;
>>> +
>>> +	return !map->file || file_is_dev_zero(map->file);
>>> +}
>>> +
>>>  /*
>>>   * __mmap_new_vma() - Allocate a new VMA for the region, as merging was not
>>>   * possible.
>>> @@ -2634,8 +2647,7 @@ static int __mmap_new_file_vma(struct mmap_state *map,
>>>  static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
>>>  	struct mmap_action *action)
>>>  {
>>> -	const bool is_anon = !map->file &&
>>> -		!vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);
>>> +	const bool is_anon = map_is_anon(map);
>>>  	struct vma_iterator *vmi = map->vmi;
>>>  	int error = 0;
>>>  	struct vm_area_struct *vma;
>>> @@ -2651,7 +2663,7 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
>>>
>>>  	vma_iter_config(vmi, map->addr, map->end);
>>>
>>> -	if (is_anon)
>>> +	if (is_anon && !map->file)
>>>  		vma_set_anonymous(vma);
>>>
>>>  	vma_set_range(vma, map->addr, map->end, map->pgoff, map->anon_pgoff);
>>> @@ -2669,6 +2681,10 @@ static int __mmap_new_vma(struct mmap_state *map, struct vm_area_struct **vmap,
>>>  	else if (!is_anon)
>>>  		error = shmem_zero_setup(vma);
>>>
>>> +	/* Temporary MAP_PRIVATE-/dev/zero workaround. */
>>> +	if (is_anon && map->file)
>>> +		vma_set_anonymous(vma);
>>> +
>>>  	if (error)
>>>  		goto free_iter_vma;
>>>
>>> @@ -2777,6 +2793,10 @@ static int call_mmap_prepare(struct mmap_state *map,
>>>  	if (err)
>>>  		return err;
>>>
>>> +	/* Hooks cannot mark themselves anonymous. */
>>
>> I guess this comment will be stale soon (after #4 where you drop the
>> set_anonymous part).
> 
> Not really, it's there to catch drivers doing something silly/broken (likely by
> mistake).
> 
> I want to catch that early. I have a 36 patch series that extends this kind of
> idea... a lot :)
> 
>>
>> Should it be
>>
>> "vm_ops are strictly required with mmap_prepare"
>>
>> or sth like that?
> 
> Well that's confusing though, because desc->vm_ops defaults to &dummy_vma_ops,
> and we absolutely do not require drivers to set vm_ops at all.
> 
> And as far as the driver is concerned maybe it's NULL? They maybe don't realise
> :)
> 
> So the idea is to say don't allow them to try to do something they can't do.

Yes, but my point is that the comment

"cannot mark themselves anonymous"

will not really be correct after the next patch, no?

> 
>>
>>> +	if (!desc->vm_ops)
>>> +		return -EINVAL;
>>> +
>>>  	err = call_action_prepare(map, desc);
>>>  	if (err)
>>>  		return err;
>>> @@ -2799,10 +2819,7 @@ static int call_mmap_prepare(struct mmap_state *map,
>>>  static void set_vma_user_defined_fields(struct vm_area_struct *vma,
>>>  		struct mmap_state *map)
>>>  {
>>> -	if (map->vm_ops)
>>> -		vma->vm_ops = map->vm_ops;
>>> -	else	/* Only /dev/zero should do this. */
>>> -		vma_set_anonymous(vma);
>>> +	vma->vm_ops = map->vm_ops;
>>>  	vma->vm_private_data = map->vm_private_data;
>>>  }
>>>
>>> @@ -2882,7 +2899,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>>>  		allocated_new = true;
>>>  	}
>>>
>>> -	if (have_mmap_prepare)
>>> +	if (have_mmap_prepare && !map_is_anon(&map))
>>>  		set_vma_user_defined_fields(vma, &map);
>>
>> Ah, we have mmap_zero_prepare() for handling the shmem_zero_setup_desc(). I was
>> just about to ask whether we can just get rid of this here.
>>
>>
>> But, hold on, do we now even need that? Could core-mm now take care of that as
>> well, and we could just remove mmap_zero_prepare() entirely?
>>
>> That is, we'd make shmem_zero_setup() in __mmap_new_vma() take care of this?
>> Then we might not even need shmem_zero_setup_desc() anymore.
>>
>> Maybe harder than it sounds at first.
> 
> I think I'd rather that be a follow up :) this series is about eliminiating the
> one last (I hope?) corner case for anon VMAs.
Right; having to deal with anonymous mappings that have mmap_prepare is rather
suboptimal. Ideally we'd just handle the odd dev-zero special-casing early in
the mmap path also for MAP_SHARED, and avoid messing with mmap_prepare entirely.

So agreed that this can be done separately.

-- 
Cheers,

David

Re: [PATCH 3/6] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

[Lorenzo Stoakes (ARM)]

On Mon, Sep 07, 2026 at 09:54:04PM +0200, David Hildenbrand (Arm) wrote:
> Yes, but my point is that the comment
>
> "cannot mark themselves anonymous"
>
> will not really be correct after the next patch, no?

Ah ok ack I get your point. I will update this comment in the next patch.

> >>>  		set_vma_user_defined_fields(vma, &map);
> >>
> >> Ah, we have mmap_zero_prepare() for handling the shmem_zero_setup_desc(). I was
> >> just about to ask whether we can just get rid of this here.
> >>
> >>
> >> But, hold on, do we now even need that? Could core-mm now take care of that as
> >> well, and we could just remove mmap_zero_prepare() entirely?
> >>
> >> That is, we'd make shmem_zero_setup() in __mmap_new_vma() take care of this?
> >> Then we might not even need shmem_zero_setup_desc() anymore.
> >>
> >> Maybe harder than it sounds at first.
> >
> > I think I'd rather that be a follow up :) this series is about eliminiating the
> > one last (I hope?) corner case for anon VMAs.
> Right; having to deal with anonymous mappings that have mmap_prepare is rather
> suboptimal. Ideally we'd just handle the odd dev-zero special-casing early in
> the mmap path also for MAP_SHARED, and avoid messing with mmap_prepare entirely.
>
> So agreed that this can be done separately.

Yeah, really life will be easier once the mmap path is gone in general :)

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH 6/6] tools/testing/selftests/mm: add MAP_PRIVATE-/dev/zero merge tests

[Lorenzo Stoakes (ARM)]

On Mon, Sep 07, 2026 at 07:08:48PM +0200, David Hildenbrand (Arm) wrote:
> On 9/2/26 20:00, Lorenzo Stoakes (ARM) wrote:
> > Assert that MAP_PRIVATE-mapped /dev/zero mappings behave like they are
> > anonymous.
> >
> > We test both unfaulted and faulted/unfaulted merges - each with the regions
> > having page offset of 0, which would not merge if the mappings were treated
> > as if they were file-backed.
> >
> > With the recent change that makes them behave as pure anonymous mappings,
> > the merges should succeed as their page offsets are equal to their
> > anonymous page offsets.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  tools/testing/selftests/mm/merge.c | 104 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 104 insertions(+)
> >
> > diff --git a/tools/testing/selftests/mm/merge.c b/tools/testing/selftests/mm/merge.c
> > index 52b8727b6628..7c528d470404 100644
> > --- a/tools/testing/selftests/mm/merge.c
> > +++ b/tools/testing/selftests/mm/merge.c
> > @@ -1362,6 +1362,110 @@ TEST_F(merge, anon_and_page_offset_mismatch_memfd)
> >  	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 5 * page_size);
> >  }
> >
> > +TEST_F(merge, merge_map_private_dev_zero_unfaulted)
> > +{
> > +	struct procmap_fd *procmap = &self->procmap;
> > +	unsigned int page_size = self->page_size;
> > +	char *carveout = self->carveout;
> > +	char *ptr, *ptr2;
> > +	int fd_zero;
> > +
> > +	if (access("/dev/zero", F_OK))
> > +		SKIP(return, "No /dev/zero.");
> > +	fd_zero = open("/dev/zero", O_RDWR);
> > +	ASSERT_NE(fd_zero, -1);
> > +
> > +	/*
> > +	 * Map two MAP_PRIVATE-/dev/zero VMAs next to one another with offset 0
> > +	 * each.
> > +	 *
> > +	 * With these being made truly anonymous upon mapping, they will
> > +	 * merge. If they were file-backed VMAs the page offsets would prevent
> > +	 * merge:
>
> Nit: "the" merge? You're the native speaker, so I don't know if what you have is
> just correct :)

You're right ;) native speakers are not immune from messing up grammar, as my
copy editor will tell you :P

Will fix up on respin.

>
> > +	 *
> > +	 * |-----||------|    |-------------|
> > +	 * | ptr || ptr2 | -> |     ptr     |
> > +	 * |-----||------|    |-------------|
> > +	 */
> > +	ptr = mmap(carveout, 5 * page_size, PROT_READ | PROT_WRITE,
> > +		   MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
> > +	if (ptr == MAP_FAILED) {
> > +		close(fd_zero);
> > +		ASSERT_TRUE(false);
> > +	}
> > +	ptr2 = mmap(&carveout[5 * page_size], 5 * page_size,
> > +		   PROT_READ | PROT_WRITE, MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
> > +	if (ptr2 == MAP_FAILED) {
> > +		close(fd_zero);
>
> Is the close() really required before the ASSERT?  After all, you're also not
> munmap'ing, so I wonder to which degree we have to clean up.
>
> So maybe this could just become a
>
> 	ASSERT_NE(ptr2, MAP_FAILED);
>
> Same for ptr above.
>
> You could likely also do
>
> ptr = mmap()
> ptr2 = mmap()
> close(fd_zero);
>
> ASSERT_NE(ptr, MAP_FAILED);
> ASSERT_NE(ptr2, MAP_FAILED);

Yeah that's easiest I think! Will fixup on respin.

>
> > +		ASSERT_TRUE(false);
> > +	}
> > +	close(fd_zero);
> > +
> > +	/* Assert that they merged. */
> > +	ASSERT_TRUE(find_vma_procmap(procmap, ptr));
> > +	ASSERT_EQ(procmap->query.vma_start, (unsigned long)ptr);
> > +	ASSERT_EQ(procmap->query.vma_end, (unsigned long)ptr + 10 * page_size);
> > +}
> > +
> > +TEST_F(merge, merge_map_private_dev_zero_faulted_unfaulted)
> > +{
> > +	struct procmap_fd *procmap = &self->procmap;
> > +	unsigned int page_size = self->page_size;
> > +	char *carveout = self->carveout;
> > +	char *ptr, *ptr2;
> > +	int fd_zero;
> > +
> > +	if (access("/dev/zero", F_OK))
> > +		SKIP(return, "No /dev/zero.");
> > +	fd_zero = open("/dev/zero", O_RDWR);
> > +	ASSERT_NE(fd_zero, -1);
> > +
> > +	/*
> > +	 * Map a MAP_PRIVATE mapping of /dev/zero with page offset 0, then fault
> > +	 * it in:
> > +	 *
> > +	 * |-------------------------------|
> > +	 * |           faulted             |
> > +	 * |-------------------------------|
> > +	 */
> > +	ptr = mmap(carveout, 15 * page_size, PROT_READ | PROT_WRITE,
> > +		   MAP_FIXED | MAP_PRIVATE, fd_zero, 0);
> > +	if (ptr == MAP_FAILED) {
> > +		close(fd_zero);
> > +		ASSERT_TRUE(false);
>
> Same question regarding cleanup requirements. The ASSERT_TRUE(false) looks a bit
> odd.

Yeah it does. This case is trickier as you then do a memset(), so maybe just
live with the 'leaked' fd in this case (the tests fail so it aborts the run at
that point anyway and tears down).

Fixed and will send respin!

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo