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

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Tue, Jan 14, 2025 at 07:32:51PM +0100, Jann Horn wrote:
> On Tue, Jan 14, 2025 at 7:15 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> 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().

Thanks Jann , I feel like I've opened up a can of worms with this :) I will
note these as things to prioritise in the audit.

It might be worth both auditing and then actually doing the change to
restrict what can be done here too.

The problem is it requires changing a trillion callers, but hey I'm
Mr. Churn after all... ;)

Sorry Yang - I realise this is a pain and not at all obvious. Something we
in mm need to sort out (by which I mean _me_ :) your contribution and ideas
here are very valued!