Re: [PATCH] /dev/zero: make private mapping full anonymous mapping

[Jann Horn]

On Tue, Jan 14, 2025 at 7:15 PM Lorenzo Stoakes
<lorenzo.stoakes@xxxxxxxxxx> wrote:
> On Tue, Jan 14, 2025 at 08:53:01AM -0800, Yang Shi wrote:
> > On 1/14/25 4:05 AM, Lorenzo Stoakes wrote:
> > > On Mon, Jan 13, 2025 at 02:30:33PM -0800, Yang Shi wrote:
> > > > + fput(vma->vm_file);
> > > > + vma->vm_file = NULL;
> > > > + vma->vm_pgoff = vma->vm_start >> PAGE_SHIFT;
>
> This is just not permitted. We maintain mmap state which contains the file
> and pgoff state which gets threaded through the mapping operation, and
> simply do not expect you to change these fields.
>
> In future we will assert on this or preferably, restrict users to only
> changing VMA flags, the private field and vm_ops.
>
> > > Hmm, this might have been mremap()'d _potentially_ though? And then now
> > > this will be wrong? But then we'd have no way of tracking it correctly...
> >
> > I'm not quite familiar with the subtle details and corner cases of
> > meremap(). But mmap_zero() should be called by mmap(), so the VMA has not
> > been visible to user yet at this point IIUC. How come mremap() could move
> > it?
>
> Ah OK, in that case fine on that front.
>
> But you are not permitted to touch this field (we need to enforce this...)

Sidenote: I think the GPU DRM subsystem relies on changing pgoff in
some of their mmap handlers; maybe talk to them about this if you
haven't already. See for example drm_gem_prime_mmap() and
dma_buf_mmap().