Re: [PATCH v4 17/20] mm/vma: only permit MAP_PRIVATE /dev/zero to be mapped anonymous

On Tue, Aug 11, 2026 at 07:07:27PM +0200, David Hildenbrand (Arm) wrote:

On 8/6/26 22:21, Lorenzo Stoakes (ARM) wrote:

quoted

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.

The sentence is a bit hard to get as you are mixing "with" with another "without".

quoted

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

Make it possible to explicitly identify /dev/zero by setting a global
DEVZERO_MINOR device minor number then explicitly check for this in mmap
code for a MAP_PRIVATE mapping and only set the VMA anonymous if we have
positively identified it.

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 mark the VMA anonymous.

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

What exactly do you mean by "nested MAP_PRIVATE mappings"? You mean, reusing
parts in other drives?

I should have said stacked I think.

Do you mean things like ...

[...]

quoted

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.

... this?

Yup.

quoted

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.

Also update the VMA userland tests to reflect the change.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---

[...]

quoted

+static bool map_is_dev_zero(const struct mmap_state *map)
+{
+	const struct file *file = map->file;
+	struct inode *inode;
+
+	if (!file)
+		return false;
+	inode = file_inode(file);
+	if (!S_ISCHR(inode->i_mode))
+		return false;
+	return imajor(inode) == MEM_MAJOR && iminor(inode) == DEVZERO_MINOR;
+}

My brain is a bit slow after digging through this series.

We identify shmem, for example, through shmem_vm_ops/shmem_anon_vm_ops.

So naturally I am wondering: couldn't we do something similar to identify that?
Like, checking for zero_fops?

We don't assign vm_ops for a MAP_PRIVATE-/dev/zero mapping. So that won't work.

We could expose zero->f_ops but then it's literally in drivers/char/ and that's
just weird to expose in mm.h or whatever.

I'm giving a really minimal possible thing to export, which is the DEVZERO_MINOR
number which avoids all kinds of weirdness like that. No driver stuff exported,
just a number :) MEM_MAJOR is already available.

So I think it's the least bad choice in this one, very very specific scenario.

quoted

+
+static bool map_is_private(const struct mmap_state *map)
+{
+	return !vma_flags_test(&map->vma_flags, VMA_SHARED_BIT);

Can't we use the is_cow_mapping() helper instead somehow?

Lol... yup. Let's see how the rest of the review goes and we'll see whether I
can ask Andrew to change it or I'll change it on a respin.

--
Cheers,

David

--
Cheers, Lorenzo