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
