Re: [PATCH v7 2/2] drm/gem: Don't map imported GEMs

[Thomas Hellström (Intel)]

On 6/30/22 22:22, Dmitry Osipenko wrote:
> Hello Thomas,
>
> On 6/30/22 23:15, Thomas Hellström (Intel) wrote:
> > Hi, Dmitry,
> >
> > On 6/30/22 22:04, Dmitry Osipenko wrote:
> > > Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't
> > > handle imported dma-bufs properly, which results in mapping of something
> > > else than the imported dma-buf. On NVIDIA Tegra we get a hard lockup when
> > > userspace writes to the memory mapping of a dma-buf that was imported
> > > into
> > > Tegra's DRM GEM.
> > >
> > > Majority of DRM drivers prohibit mapping of the imported GEM objects.
> > > Mapping of imported GEMs require special care from userspace since it
> > > should sync dma-buf because mapping coherency of the exporter device may
> > > not match the DRM device. Let's prohibit the mapping for all DRM drivers
> > > for consistency.
> > >
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Suggested-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
> > This might break drivers whose obj->funcs->mmap() callback already
> > handles this case, and has userspace that does the right thing.
> The drm-shmem helper should be the only that maps imported GEMs
> properly, but drivers that use drm-shmem already prohibit to map
> imported GEMs. Okay, I'll try to re-check once again to be sure.

OK. If you aren't 100.1% sure, then please drop the Cc: stable tag and 
let the patch ride out at least an -rc series, because breaking a stable 
kernel is something we woudln't want to do.

Thanks,

Thomas



> > I think the disabling must be checked on a per-driver basis to avoid
> > that; in particular drivers that already call dma_buf_mmap() should be
> > able to continue doing this.
> >
> > Also the Fixes: review comment remains,
> This should've been broken forever, don't think that we have a fixes tag
> here. I looked thought all my previous patches and added the Fixes
> wherever was possible. If I really missed something, then please let me
> know.