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:
> > 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".
>
> >
> > 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 ...
>
> [...]
>
> >
> > 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.

>
> >
> > 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) <[email protected]>
> > ---
>
> [...]
>
> >
> > +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.

>
> > +
> > +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