On Mon, Sep 14, 2020 at 08:26:47PM +0200, Christian König wrote: > Am 14.09.20 um 16:06 schrieb Jason Gunthorpe: > > On Mon, Sep 14, 2020 at 03:30:47PM +0200, Christian König wrote: > > > Am 14.09.20 um 15:29 schrieb Christian König: > > > > Hi Andrew, > > > > > > > > I'm the new DMA-buf maintainer and Daniel and others came up with > > > > patches extending the use of the dma_buf_mmap() function. > > > > > > > > Now this function is doing something a bit odd by changing the > > > > vma->vm_file while installing a VMA in the mmap() system call > > It doesn't look obviously safe as mmap_region() has an interesting mix > > of file and vma->file > > > > Eg it calls mapping_unmap_writable() using both routes > > Thanks for the hint, going to take a look at that code tomorrow. > > > What about security? Is it OK that some other random file, maybe in > > another process, is being linked to this mmap? > > Good question, I have no idea. That's why I send out this mail. > > > > > The background here is that DMA-buf allows device drivers to > > > > export buffer which are then imported into another device > > > > driver. The mmap() handler of the importing device driver then > > > > find that the pgoff belongs to the exporting device and so > > > > redirects the mmap() call there. > > So the pgoff is some virtualized thing? > > Yes, absolutely. Maybe notch more context. Conceptually the buffer objects we use to manage gpu memory are all stand-alone objects, internally refcounted and everything. And if you export them as a dma-buf, then they are indeed stand-alone file descriptors like any other. But within the driver, we generally need thousands of these, and that tends to bring fd exhaustion problems with it. That's why all the private buffer objects which aren't shared with other process or other drivers are handles only valid for a specific fd instance of the drm chardev (each open gets their own namespace), and only for ioctls done on that chardev. And for mmap we assign fake (but unique across all open fd on it) offsets within the overall chardev. Hence all the pgoff mangling and re-mangling. Now for unmap_mapping_range we'd like it to find all such fake offset aliases pointing at the one underlying buffer object: - mmap on the dma-buf fd, at offset 0 - mmap on the drm chardev where the buffer was originally allocated, at some unique offset - mmap on the drm chardev where the buffer was imported, again at some (likely) different unique (for that chardev) offset. So to make unmap_mapping_range work across the entire delegation change we'd actually need to change the vma->vma_file and pgoff twice: - once when forwarding from the importing drm chardev to the dma-buf - once when forwarding from the dma-buf to the exported drm chardev fake offset, which (mostly for historical reasons) is considered the canonical fake offset We can't really do the delegation in userspace because: - the importer might not have access to the exporters drm chardev, it only gets the dma-buf. If we'd give it the underlying drm chardev it could do stuff like issue rendering commands, breaking the access model. - the dma-buf fd is only used to establish the sharing, once it's imported everywhere it generally gets closed. Userspace could re-export it and then call mmap on that, but feels a bit contrived. - especially on SoC platforms this has already become uapi. It's not a big problem because the drivers that really need unmap_mapping_range to work are the big gpu drivers with discrete vram, where mappings need to be invalidate when moving buffer objects in/out of vram. Hence why we'd like to be able to forward aliasing mappings and adjust the file and pgoff, while hopefully everything keeps working. I thought this would work, but Christian noticed it doesn't really. Cheers, Daniel > > Christian. > > > > > Jason > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
