Re: [PATCH 1/4] mm: introduce vma_set_file function v2

[Christian König]

Am 09.10.20 um 14:12 schrieb Jason Gunthorpe:
> On Fri, Oct 09, 2020 at 09:39:00AM +0200, Daniel Vetter wrote:
> > I just noticed this here in the patch because everyone else does not do
> > this. But looking at the mmap_region() code in mmap.c we seem to indeed
> > have this problem for the error path:
> >
> > unmap_and_free_vma:
> > 	vma->vm_file = NULL;
> > 	fput(file);
> >
> > Note that the success path does things correctly (a bit above):
> >
> > 	file = vma->vm_file;
> > out:
> >
> > So it indeed looks like dma-buf is the only one that does this fully
> > correctly. So maybe we should do a follow-up patch to change the
> > mmap_region exit code to pick up whatever vma->vm_file was set instead,
> > and fput that?
> Given that this new vma_set_file() should be the only way to
> manipulate vm_file from the mmap op, I think this reflects a bug in
> mm/mmap.c.. Should be:
>
> unmap_and_free_vma:
>          fput(vma->vm_file);
>          vma->vm_file = NULL;
>
> Then everything works the way you'd expect without tricky error
> handling

That's what Daniel suggested as well, yes.

Going to craft a separate patch for this.

Thanks,
Christian.

> Jason