Re: Changing vma->vm_file in dma_buf_mmap()

From: Daniel Vetter
Date: Wed Sep 16 2020 - 15:03:35 EST

On Wed, Sep 16, 2020 at 1:45 PM Christian König
<christian.koenig@xxxxxxx> wrote:
> [SNIP]
> But Jason pointed me to the right piece of code. See this comment in in mmap_region():
> /* ->mmap() can change vma->vm_file, but must guarantee that
> * vma_link() below can deny write-access if VM_DENYWRITE is set
> * and map writably if VM_SHARED is set. This usually means the
> * new file must not have been exposed to user-space, yet.
> */
> vma->vm_file = get_file(file);
> error = call_mmap(file, vma);
> So changing vma->vm_file is allowed at least under certain circumstances.
> Only the "file must not have been exposed to user-space, yet" part still needs double checking. Currently working on that.
> Ok, I think we can guarantee for all DMA-bufs what is required here.
> While searching the code I've found that at least vgem_prime_mmap() and i915_gem_dmabuf_mmap() are doing the same thing of modifying vma->vm_file.
> So I'm leaning towards that this works as expected and we should just document this properly.
> Daniel and Jason what do you think?

Well I can claim I started this, so I started out with naively
assuming that it Just Works :-)

I think we already have vgem/i915 prime testcases in igt which try to
excercise this mmap forwarding, including provoking pte shoot-downs.
So I think best would be if you could also add a variant for amdgpu,
to make sure this really works and keeps working.

Plus ofc the documentation patch so that core mm folks can officially
ack this as legit.
Daniel Vetter
Software Engineer, Intel Corporation