linux-kernel - Re: [PATCH v1 0/7] Move dma-buf mmap() reservation locking down to exporters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <80cdf100-ef3c-23ae-5adf-bcf97fa85e72@collabora.com>
Date:   Mon, 3 Apr 2023 13:19:01 +0300
From:   Dmitry Osipenko <dmitry.osipenko@...labora.com>
To:     Sumit Semwal <sumit.semwal@...aro.org>,
        Christian König <christian.koenig@....com>,
        Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        Liam Mark <lmark@...eaurora.org>,
        Brian Starkey <Brian.Starkey@....com>,
        John Stultz <jstultz@...gle.com>,
        Gerd Hoffmann <kraxel@...hat.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Tomi Valkeinen <tomba@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Tomasz Figa <tfiga@...omium.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Amol Maheshwari <amahesh@....qualcomm.com>
Cc:     linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
        linux-tegra@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        kernel@...labora.com
Subject: Re: [PATCH v1 0/7] Move dma-buf mmap() reservation locking down to
 exporters

On 4/2/23 19:48, Dmitry Osipenko wrote:
> This patchset makes dma-buf exporters responisble for taking care of
> the reservation lock. I also included patch that moves drm-shmem to use
> reservation lock, to let CI test the whole set. I'm going to take all
> the patches via the drm-misc tree, please give an ack.
> 
> Previous policy stated that dma-buf core takes the lock around mmap()
> callback. Which meant that both importers and exporters shouldn't touch
> the reservation lock in the mmap() code path. This worked well until
> Intel-CI found a deadlock problem in a case of self-imported dma-buf [1].
> 
> The problem happens when userpace mmaps a self-imported dma-buf, i.e.
> mmaps the dma-buf FD. DRM core treats self-imported dma-bufs as own GEMs
> [2]. There is no way to differentiate a prime GEM from a normal GEM for
> drm-shmem in drm_gem_shmem_mmap(), which resulted in a deadlock problem
> for drm-shmem mmap() code path once it's switched to use reservation lock.
> 
> It was difficult to fix the drm-shmem problem without adjusting dma-buf
> locking policy. In parctice not much changed from importers perspective
> because previosly dma-buf was taking the lock in between of importers
> and exporters. Now this lock is shifted down to exporters.
> 
> [1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114671v2/shard-snb5/igt@prime_vgem@sync@rcs0.html
> [2] https://elixir.bootlin.com/linux/v6.3-rc4/source/drivers/gpu/drm/drm_prime.c#L924
> 
> Dmitry Osipenko (7):
>   Revert "media: videobuf2: Assert held reservation lock for dma-buf
>     mmapping"
>   Revert "dma-buf/heaps: Assert held reservation lock for dma-buf
>     mmapping"
>   Revert "udmabuf: Assert held reservation lock for dma-buf mmapping"
>   Revert "fastrpc: Assert held reservation lock for dma-buf mmapping"
>   Revert "drm: Assert held reservation lock for dma-buf mmapping"
>   dma-buf: Change locking policy for mmap()
>   drm/shmem-helper: Switch to reservation lock
> 
>  drivers/dma-buf/dma-buf.c                     |  17 +-
>  drivers/dma-buf/heaps/cma_heap.c              |   3 -
>  drivers/dma-buf/heaps/system_heap.c           |   3 -
>  drivers/dma-buf/udmabuf.c                     |   2 -
>  drivers/gpu/drm/drm_gem_shmem_helper.c        | 217 ++++++++----------
>  drivers/gpu/drm/drm_prime.c                   |   2 -
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c    |   2 -
>  drivers/gpu/drm/lima/lima_gem.c               |   8 +-
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c     |   2 -
>  drivers/gpu/drm/panfrost/panfrost_drv.c       |   7 +-
>  .../gpu/drm/panfrost/panfrost_gem_shrinker.c  |   6 +-
>  drivers/gpu/drm/panfrost/panfrost_mmu.c       |  19 +-
>  drivers/gpu/drm/tegra/gem.c                   |   2 -
>  .../common/videobuf2/videobuf2-dma-contig.c   |   3 -
>  .../media/common/videobuf2/videobuf2-dma-sg.c |   3 -
>  .../common/videobuf2/videobuf2-vmalloc.c      |   3 -
>  drivers/misc/fastrpc.c                        |   3 -
>  include/drm/drm_gem_shmem_helper.h            |  14 +-
>  18 files changed, 123 insertions(+), 193 deletions(-)
> 

Intel's IGT passed[1] (see Tests section) with the new locking policy,
which failed previously for the "drm/shmem-helper: Switch to reservation
lock" patch.

[1] https://patchwork.freedesktop.org/series/116000/

-- 
Best regards,
Dmitry