Messages in this thread | | | Subject | Re: [PATCH 1/4] mm: introduce vma_set_file function v2 | From | Christian König <> | Date | Fri, 9 Oct 2020 14:15:16 +0200 |
| |
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
| |