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? 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? > > 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? > + > +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? -- Cheers, David
