linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification
@ 2022-11-10 20:13 Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 1/6] dma-buf: " Dmitry Osipenko
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

Hello,

Recently, dma-buf got a common locking convention for importers and
exporters. All the dma-buf functions were moved to the new locking
convention, apart from the dma_buf_mmap_internal() that was missed out
by accident. This series moves dma_buf_mmap_internal() to the dynamic
locking specification and updates drivers that support mmaping of
dma-bufs to use the debug-assert of the lock.

Thanks to Daniel Vetter for spotting the missed function!

Dmitry Osipenko (6):
  dma-buf: Move dma_buf_mmap_internal() to dynamic locking specification
  drm: Assert held reservation lock for dma-buf mmapping
  udmabuf: Assert held reservation lock for dma-buf mmapping
  dma-buf/heaps: Assert held reservation lock for dma-buf mmapping
  media: videobuf2: Assert held reservation lock for dma-buf mmapping
  fastrpc: Assert held reservation lock for dma-buf mmapping

 drivers/dma-buf/dma-buf.c                             | 7 ++++++-
 drivers/dma-buf/heaps/cma_heap.c                      | 3 +++
 drivers/dma-buf/heaps/system_heap.c                   | 3 +++
 drivers/dma-buf/udmabuf.c                             | 3 +++
 drivers/gpu/drm/drm_prime.c                           | 2 ++
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c            | 2 ++
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c             | 2 ++
 drivers/gpu/drm/tegra/gem.c                           | 2 ++
 drivers/media/common/videobuf2/videobuf2-dma-contig.c | 3 +++
 drivers/media/common/videobuf2/videobuf2-dma-sg.c     | 3 +++
 drivers/media/common/videobuf2/videobuf2-vmalloc.c    | 3 +++
 drivers/misc/fastrpc.c                                | 3 +++
 12 files changed, 35 insertions(+), 1 deletion(-)

-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/6] dma-buf: Move dma_buf_mmap_internal() to dynamic locking specification
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
@ 2022-11-10 20:13 ` Dmitry Osipenko
  2022-11-11 12:47   ` Christian König
  2022-11-10 20:13 ` [PATCH v1 2/6] drm: Assert held reservation lock for dma-buf mmapping Dmitry Osipenko
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

All dma-buf functions has been moved to dynamic locking specification
The dma_buf_mmap_internal() was missed out by accident. Take reservation
lock around file mapping operation to adhere the common locking convention.

Reported-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/dma-buf/dma-buf.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 13bfd2d09c56..b809513b03fe 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -129,6 +129,7 @@ static struct file_system_type dma_buf_fs_type = {
 static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
 {
 	struct dma_buf *dmabuf;
+	int ret;
 
 	if (!is_dma_buf_file(file))
 		return -EINVAL;
@@ -144,7 +145,11 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
 	    dmabuf->size >> PAGE_SHIFT)
 		return -EINVAL;
 
-	return dmabuf->ops->mmap(dmabuf, vma);
+	dma_resv_lock(dmabuf->resv, NULL);
+	ret = dmabuf->ops->mmap(dmabuf, vma);
+	dma_resv_unlock(dmabuf->resv);
+
+	return ret;
 }
 
 static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/6] drm: Assert held reservation lock for dma-buf mmapping
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 1/6] dma-buf: " Dmitry Osipenko
@ 2022-11-10 20:13 ` Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 3/6] udmabuf: " Dmitry Osipenko
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

When userspace mmaps dma-buf's fd, the dma-buf reservation lock must be
held. Add locking sanity checks to the dma-buf mmaping callbacks of DRM
drivers to ensure that the locking assumptions won't regress in future.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/gpu/drm/drm_prime.c                | 2 ++
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 2 ++
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c  | 2 ++
 drivers/gpu/drm/tegra/gem.c                | 2 ++
 4 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 20e109a802ae..f924b8b4ab6b 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -781,6 +781,8 @@ int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
 	struct drm_gem_object *obj = dma_buf->priv;
 	struct drm_device *dev = obj->dev;
 
+	dma_resv_assert_held(dma_buf->resv);
+
 	if (!dev->driver->gem_prime_mmap)
 		return -ENOSYS;
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index ec6f7ae47783..9322ac29008b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -97,6 +97,8 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	int ret;
 
+	dma_resv_assert_held(dma_buf->resv);
+
 	if (obj->base.size < vma->vm_end - vma->vm_start)
 		return -EINVAL;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 8e194dbc9506..3abc47521b2c 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -66,6 +66,8 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
 	struct drm_gem_object *obj = buffer->priv;
 	int ret = 0;
 
+	dma_resv_assert_held(buffer->resv);
+
 	ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index b09b8ab40ae4..979e7bc902f6 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -694,6 +694,8 @@ static int tegra_gem_prime_mmap(struct dma_buf *buf, struct vm_area_struct *vma)
 	struct drm_gem_object *gem = buf->priv;
 	int err;
 
+	dma_resv_assert_held(buf->resv);
+
 	err = drm_gem_mmap_obj(gem, gem->size, vma);
 	if (err < 0)
 		return err;
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 3/6] udmabuf: Assert held reservation lock for dma-buf mmapping
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 1/6] dma-buf: " Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 2/6] drm: Assert held reservation lock for dma-buf mmapping Dmitry Osipenko
@ 2022-11-10 20:13 ` Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 4/6] dma-buf/heaps: " Dmitry Osipenko
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

When userspace mmaps dma-buf's fd, the dma-buf reservation lock must be
held. Add locking sanity check to the dma-buf mmaping callback to ensure
that the locking assumption won't regress in the future.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/dma-buf/udmabuf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index 2bcdb935a3ac..283816fbd72f 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -2,6 +2,7 @@
 #include <linux/cred.h>
 #include <linux/device.h>
 #include <linux/dma-buf.h>
+#include <linux/dma-resv.h>
 #include <linux/highmem.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -49,6 +50,8 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma)
 {
 	struct udmabuf *ubuf = buf->priv;
 
+	dma_resv_assert_held(buf->resv);
+
 	if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0)
 		return -EINVAL;
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 4/6] dma-buf/heaps: Assert held reservation lock for dma-buf mmapping
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2022-11-10 20:13 ` [PATCH v1 3/6] udmabuf: " Dmitry Osipenko
@ 2022-11-10 20:13 ` Dmitry Osipenko
  2022-11-10 20:13 ` [PATCH v1 5/6] media: videobuf2: " Dmitry Osipenko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

When userspace mmaps dma-buf's fd, the dma-buf reservation lock must be
held. Add locking sanity checks to the dma-buf mmaping callbacks to ensure
that the locking assumptions won't regress in the future.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/dma-buf/heaps/cma_heap.c    | 3 +++
 drivers/dma-buf/heaps/system_heap.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 28fb04eccdd0..1131fb943992 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -13,6 +13,7 @@
 #include <linux/dma-buf.h>
 #include <linux/dma-heap.h>
 #include <linux/dma-map-ops.h>
+#include <linux/dma-resv.h>
 #include <linux/err.h>
 #include <linux/highmem.h>
 #include <linux/io.h>
@@ -182,6 +183,8 @@ static int cma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
 {
 	struct cma_heap_buffer *buffer = dmabuf->priv;
 
+	dma_resv_assert_held(dmabuf->resv);
+
 	if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) == 0)
 		return -EINVAL;
 
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index fcf836ba9c1f..e8bd10e60998 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -13,6 +13,7 @@
 #include <linux/dma-buf.h>
 #include <linux/dma-mapping.h>
 #include <linux/dma-heap.h>
+#include <linux/dma-resv.h>
 #include <linux/err.h>
 #include <linux/highmem.h>
 #include <linux/mm.h>
@@ -201,6 +202,8 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
 	struct sg_page_iter piter;
 	int ret;
 
+	dma_resv_assert_held(dmabuf->resv);
+
 	for_each_sgtable_page(table, &piter, vma->vm_pgoff) {
 		struct page *page = sg_page_iter_page(&piter);
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 5/6] media: videobuf2: Assert held reservation lock for dma-buf mmapping
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2022-11-10 20:13 ` [PATCH v1 4/6] dma-buf/heaps: " Dmitry Osipenko
@ 2022-11-10 20:13 ` Dmitry Osipenko
  2022-11-11  3:39   ` Tomasz Figa
  2022-11-10 20:13 ` [PATCH v1 6/6] fastrpc: " Dmitry Osipenko
  2022-11-11 20:59 ` [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
  6 siblings, 1 reply; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

When userspace mmaps dma-buf's fd, the dma-buf reservation lock must be
held. Add locking sanity checks to the dma-buf mmaping callbacks to ensure
that the locking assumptions won't regress in the future.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/media/common/videobuf2/videobuf2-dma-contig.c | 3 +++
 drivers/media/common/videobuf2/videobuf2-dma-sg.c     | 3 +++
 drivers/media/common/videobuf2/videobuf2-vmalloc.c    | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 555bd40fa472..7f45a62969f2 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/dma-buf.h>
+#include <linux/dma-resv.h>
 #include <linux/module.h>
 #include <linux/refcount.h>
 #include <linux/scatterlist.h>
@@ -455,6 +456,8 @@ static int vb2_dc_dmabuf_ops_vmap(struct dma_buf *dbuf, struct iosys_map *map)
 static int vb2_dc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 	struct vm_area_struct *vma)
 {
+	dma_resv_assert_held(dbuf->resv);
+
 	return vb2_dc_mmap(dbuf->priv, vma);
 }
 
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 36981a5b5c53..b7f39ee49ed8 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -10,6 +10,7 @@
  * the Free Software Foundation.
  */
 
+#include <linux/dma-resv.h>
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/refcount.h>
@@ -495,6 +496,8 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
 static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
 	struct vm_area_struct *vma)
 {
+	dma_resv_assert_held(dbuf->resv);
+
 	return vb2_dma_sg_mmap(dbuf->priv, vma);
 }
 
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
index 41db707e43a4..f9b665366365 100644
--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
@@ -10,6 +10,7 @@
  * the Free Software Foundation.
  */
 
+#include <linux/dma-resv.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/mm.h>
@@ -316,6 +317,8 @@ static int vb2_vmalloc_dmabuf_ops_vmap(struct dma_buf *dbuf,
 static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
 	struct vm_area_struct *vma)
 {
+	dma_resv_assert_held(dbuf->resv);
+
 	return vb2_vmalloc_mmap(dbuf->priv, vma);
 }
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 6/6] fastrpc: Assert held reservation lock for dma-buf mmapping
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2022-11-10 20:13 ` [PATCH v1 5/6] media: videobuf2: " Dmitry Osipenko
@ 2022-11-10 20:13 ` Dmitry Osipenko
  2022-11-11 20:59 ` [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
  6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-10 20:13 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

When userspace mmaps dma-buf's fd, the dma-buf reservation lock must be
held. Add locking sanity check to the dma-buf mmaping callback to ensure
that the locking assumption won't regress in the future.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/misc/fastrpc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 1ad580865525..0f467a71b069 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -6,6 +6,7 @@
 #include <linux/device.h>
 #include <linux/dma-buf.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-resv.h>
 #include <linux/idr.h>
 #include <linux/list.h>
 #include <linux/miscdevice.h>
@@ -682,6 +683,8 @@ static int fastrpc_mmap(struct dma_buf *dmabuf,
 	struct fastrpc_buf *buf = dmabuf->priv;
 	size_t size = vma->vm_end - vma->vm_start;
 
+	dma_resv_assert_held(dmabuf->resv);
+
 	return dma_mmap_coherent(buf->dev, vma, buf->virt,
 				 FASTRPC_PHYS(buf->phys), size);
 }
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 5/6] media: videobuf2: Assert held reservation lock for dma-buf mmapping
  2022-11-10 20:13 ` [PATCH v1 5/6] media: videobuf2: " Dmitry Osipenko
@ 2022-11-11  3:39   ` Tomasz Figa
  0 siblings, 0 replies; 10+ messages in thread
From: Tomasz Figa @ 2022-11-11  3:39 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari, linux-media, dri-devel, linux-kernel, intel-gfx,
	linux-tegra, linux-arm-msm, kernel

On Fri, Nov 11, 2022 at 5:15 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> When userspace mmaps dma-buf's fd, the dma-buf reservation lock must be
> held. Add locking sanity checks to the dma-buf mmaping callbacks to ensure
> that the locking assumptions won't regress in the future.
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>  drivers/media/common/videobuf2/videobuf2-dma-contig.c | 3 +++
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c     | 3 +++
>  drivers/media/common/videobuf2/videobuf2-vmalloc.c    | 3 +++
>  3 files changed, 9 insertions(+)
>

Acked-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index 555bd40fa472..7f45a62969f2 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -11,6 +11,7 @@
>   */
>
>  #include <linux/dma-buf.h>
> +#include <linux/dma-resv.h>
>  #include <linux/module.h>
>  #include <linux/refcount.h>
>  #include <linux/scatterlist.h>
> @@ -455,6 +456,8 @@ static int vb2_dc_dmabuf_ops_vmap(struct dma_buf *dbuf, struct iosys_map *map)
>  static int vb2_dc_dmabuf_ops_mmap(struct dma_buf *dbuf,
>         struct vm_area_struct *vma)
>  {
> +       dma_resv_assert_held(dbuf->resv);
> +
>         return vb2_dc_mmap(dbuf->priv, vma);
>  }
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index 36981a5b5c53..b7f39ee49ed8 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -10,6 +10,7 @@
>   * the Free Software Foundation.
>   */
>
> +#include <linux/dma-resv.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
>  #include <linux/refcount.h>
> @@ -495,6 +496,8 @@ static int vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf,
>  static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
>         struct vm_area_struct *vma)
>  {
> +       dma_resv_assert_held(dbuf->resv);
> +
>         return vb2_dma_sg_mmap(dbuf->priv, vma);
>  }
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> index 41db707e43a4..f9b665366365 100644
> --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> @@ -10,6 +10,7 @@
>   * the Free Software Foundation.
>   */
>
> +#include <linux/dma-resv.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
> @@ -316,6 +317,8 @@ static int vb2_vmalloc_dmabuf_ops_vmap(struct dma_buf *dbuf,
>  static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
>         struct vm_area_struct *vma)
>  {
> +       dma_resv_assert_held(dbuf->resv);
> +
>         return vb2_vmalloc_mmap(dbuf->priv, vma);
>  }
>
> --
> 2.37.3
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/6] dma-buf: Move dma_buf_mmap_internal() to dynamic locking specification
  2022-11-10 20:13 ` [PATCH v1 1/6] dma-buf: " Dmitry Osipenko
@ 2022-11-11 12:47   ` Christian König
  0 siblings, 0 replies; 10+ messages in thread
From: Christian König @ 2022-11-11 12:47 UTC (permalink / raw)
  To: Dmitry Osipenko, Sumit Semwal, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

Am 10.11.22 um 21:13 schrieb Dmitry Osipenko:
> All dma-buf functions has been moved to dynamic locking specification
> The dma_buf_mmap_internal() was missed out by accident. Take reservation
> lock around file mapping operation to adhere the common locking convention.
>
> Reported-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for this patch here.

Acked-by: Christian König <christian.koenig@amd.com> for the rest of the 
series.

Regards,
Christian.

> ---
>   drivers/dma-buf/dma-buf.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 13bfd2d09c56..b809513b03fe 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -129,6 +129,7 @@ static struct file_system_type dma_buf_fs_type = {
>   static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
>   {
>   	struct dma_buf *dmabuf;
> +	int ret;
>   
>   	if (!is_dma_buf_file(file))
>   		return -EINVAL;
> @@ -144,7 +145,11 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
>   	    dmabuf->size >> PAGE_SHIFT)
>   		return -EINVAL;
>   
> -	return dmabuf->ops->mmap(dmabuf, vma);
> +	dma_resv_lock(dmabuf->resv, NULL);
> +	ret = dmabuf->ops->mmap(dmabuf, vma);
> +	dma_resv_unlock(dmabuf->resv);
> +
> +	return ret;
>   }
>   
>   static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification
  2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2022-11-10 20:13 ` [PATCH v1 6/6] fastrpc: " Dmitry Osipenko
@ 2022-11-11 20:59 ` Dmitry Osipenko
  6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Osipenko @ 2022-11-11 20:59 UTC (permalink / raw)
  To: Sumit Semwal, Christian König, Benjamin Gaignard, Liam Mark,
	Brian Starkey, John Stultz, Gerd Hoffmann, Daniel Vetter,
	Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
	Thomas Zimmermann, Tomi Valkeinen, Thierry Reding, Tomasz Figa,
	Marek Szyprowski, Mauro Carvalho Chehab, Srinivas Kandagatla,
	Amol Maheshwari
  Cc: linux-media, dri-devel, linux-kernel, intel-gfx, linux-tegra,
	linux-arm-msm, kernel

On 11/10/22 23:13, Dmitry Osipenko wrote:
> Hello,
> 
> Recently, dma-buf got a common locking convention for importers and
> exporters. All the dma-buf functions were moved to the new locking
> convention, apart from the dma_buf_mmap_internal() that was missed out
> by accident. This series moves dma_buf_mmap_internal() to the dynamic
> locking specification and updates drivers that support mmaping of
> dma-bufs to use the debug-assert of the lock.
> 
> Thanks to Daniel Vetter for spotting the missed function!
> 
> Dmitry Osipenko (6):
>   dma-buf: Move dma_buf_mmap_internal() to dynamic locking specification
>   drm: Assert held reservation lock for dma-buf mmapping
>   udmabuf: Assert held reservation lock for dma-buf mmapping
>   dma-buf/heaps: Assert held reservation lock for dma-buf mmapping
>   media: videobuf2: Assert held reservation lock for dma-buf mmapping
>   fastrpc: Assert held reservation lock for dma-buf mmapping
> 
>  drivers/dma-buf/dma-buf.c                             | 7 ++++++-
>  drivers/dma-buf/heaps/cma_heap.c                      | 3 +++
>  drivers/dma-buf/heaps/system_heap.c                   | 3 +++
>  drivers/dma-buf/udmabuf.c                             | 3 +++
>  drivers/gpu/drm/drm_prime.c                           | 2 ++
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c            | 2 ++
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c             | 2 ++
>  drivers/gpu/drm/tegra/gem.c                           | 2 ++
>  drivers/media/common/videobuf2/videobuf2-dma-contig.c | 3 +++
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c     | 3 +++
>  drivers/media/common/videobuf2/videobuf2-vmalloc.c    | 3 +++
>  drivers/misc/fastrpc.c                                | 3 +++
>  12 files changed, 35 insertions(+), 1 deletion(-)
> 

Applied to drm-misc-next

-- 
Best regards,
Dmitry


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-11-11 20:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-10 20:13 [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko
2022-11-10 20:13 ` [PATCH v1 1/6] dma-buf: " Dmitry Osipenko
2022-11-11 12:47   ` Christian König
2022-11-10 20:13 ` [PATCH v1 2/6] drm: Assert held reservation lock for dma-buf mmapping Dmitry Osipenko
2022-11-10 20:13 ` [PATCH v1 3/6] udmabuf: " Dmitry Osipenko
2022-11-10 20:13 ` [PATCH v1 4/6] dma-buf/heaps: " Dmitry Osipenko
2022-11-10 20:13 ` [PATCH v1 5/6] media: videobuf2: " Dmitry Osipenko
2022-11-11  3:39   ` Tomasz Figa
2022-11-10 20:13 ` [PATCH v1 6/6] fastrpc: " Dmitry Osipenko
2022-11-11 20:59 ` [PATCH v1 0/6] Move dma_buf_mmap_internal() to dynamic locking specification Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).