mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] dma-buf: warn-only mode for DMABUF_DEBUG
@ 2026-10-05  6:41 Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 1/3] dma-buf: keep the DMA flags in the DMABUF_DEBUG copy Karl Mehltretter
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Karl Mehltretter @ 2026-10-05  6:41 UTC (permalink / raw)
  To: Sumit Semwal, Christian König
  Cc: Karl Mehltretter, Andrew Morton, Jason Gunthorpe, Rob Clark,
	Jianfeng Liu, Diederik de Haas, Andy Shevchenko, Vinod Koul,
	Bjorn Andersson, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel

With DMABUF_DEBUG, dma_buf_map_attachment() gives the importer a copy of
the exporter's sg_table without the struct pages and offsets, and
without the lengths where sg_dma_len() is a separate field. An importer
that uses those fields stops working. That is the point of the option,
but the failure usually shows up somewhere else and says nothing about
the cause.

Commit 143755bdabaa9 ("dma-buf: Make DMABUF_DEBUG default to y on
DEBUG_KERNEL kernels"), which I wrote, went into v7.3-rc4. It made the
default from commit 646013f513f3 ("dma-buf: enable DMABUF_DEBUG by
default on DEBUG kernels") take effect, and distribution configs
usually have DEBUG_KERNEL=y. Two reports followed:

- Jianfeng Liu: hardware video decode with drm/msm ends in GPU
  translation faults [1]. He then sent a revert [2].

- Diederik de Haas: rockchip fails to import buffers when playing
  video, on a config based on Debian's [3].

In [2] Rob Clark said the problem goes beyond msm and cannot be fixed
quickly, and Christian König offered to set the default to N for
another few months [4]. I listed more importers that look affected and
mentioned this RFC in [5]. As of v7.3-rc6 the default is unchanged.

This series adds DMABUF_DEBUG_WARN as a sub-option of DMABUF_DEBUG. The
importer still gets a copy, but one that keeps the CPU side of the
exporter's table. The entries are marked with a new bit in dma_flags.
sg_page(), sg_nents_for_len() and sg_split() print a rate limited
message with a stack trace when they see the bit. These CPU-side
accesses can continue after the report instead of failing because the
fields were cleared. The report is not a WARN(), so it does not taint
and does not trigger panic_on_warn. Strict mode remains the default and
continues to remove the CPU-side fields.

Known limits:

- Only access through sg_page(), sg_nents_for_len() and sg_split() is
  seen. sg_page() covers sg_phys(), sg_virt() and the page iterators.
  An importer that reads sg->length or sg->offset directly is not
  noticed.

- When the option is enabled, every sg_page() tests the flag.

- It selects NEED_SG_DMA_FLAGS, which adds dma_flags and may increase
  the size of struct scatterlist. It also relies on dma_flags being
  initialised, as the DMA mapping code already does.

- It finds importers. It does not fix them.

Question for the maintainers: could warn mode be what DEBUG_KERNEL
kernels get by default, with strict mode kept for CI? Or is an opt-in
sub-option all that is wanted, if anything? The series does not change
any default.

Testing, on the commits as posted:

- KUnit, the dma-buf suites, under UML and on x86_64 in QEMU, in strict
  and in warn mode: 56 passed, 1 skipped (it needs 2 CPUs) each time.

- Builds: x86_64, arm64 and ARM926 with DMABUF_DEBUG_WARN=y, ARM926
  without DMABUF_DEBUG. Without DMABUF_DEBUG the generated code of
  dma-buf.o and lib/scatterlist.o is the same as before, except for
  line numbers.

- IIO DMABUF capture on a Zynq in QEMU, with local device models. The
  IIO dmaengine buffer calls sg_nents_for_len() on the attachment's
  table. In strict mode with NEED_SG_DMA_LENGTH the capture fails with
  -EBUSY. In warn mode it works, the kernel is not tainted, and the log
  has:

  DMA-BUF: importer used the CPU side of an exporter's sg_table
  CPU: 0 UID: 0 PID: 48 Comm: iio-dmabuf Not tainted 7.3.0-rc4+ #2 VOLUNTARY
  Call trace:
   [...]
   dump_stack_lvl from sg_nents_for_len+0xd8/0xe4
   sg_nents_for_len from iio_dmaengine_buffer_submit_block+0x4c/0x33c
   iio_dmaengine_buffer_submit_block from iio_dma_buffer_submit_block.part.0+0x5c/0x104
   iio_dma_buffer_submit_block.part.0 from iio_dma_buffer_enqueue_dmabuf+0x68/0xa0
   iio_dma_buffer_enqueue_dmabuf from iio_buffer_chrdev_ioctl+0x520/0x9a4
   iio_buffer_chrdev_ioctl from sys_ioctl+0x460/0x914

- sur40 behind xHCI and intel-iommu on x86_64 in QEMU, with a one-line
  test-only change in sur40. Here sg_page() is called in
  iommu_dma_map_sg(), and the trace leads back to sur40_poll(). The
  capture itself did not finish in that setup.

Details are in the notes on the patches. Not tested on hardware.

Based on v7.3-rc4-70-gfe2ec83746e5.

An LLM agent helped with the code and the testing.

[1] https://lore.kernel.org/r/20260923074256.9357-1-liujianfeng1994@gmail.com
[2] https://lore.kernel.org/r/20260926022026.10539-1-liujianfeng1994@gmail.com
[3] https://lists.freedesktop.org/archives/dri-devel/2026-September/600904.html
[4] https://lore.kernel.org/r/50a9c1f1-6889-4bd5-b4f7-0500d30d3dd9@amd.com
[5] https://lore.kernel.org/r/arrAvk4aYQ4sDEzN@gmail.com

Karl Mehltretter (3):
  dma-buf: keep the DMA flags in the DMABUF_DEBUG copy
  dma-buf: add a warn-only mode to DMABUF_DEBUG
  dma-buf: test the debug scatterlist wrapper

 drivers/dma-buf/.kunitconfig |   1 +
 drivers/dma-buf/Kconfig      |  23 ++++
 drivers/dma-buf/Makefile     |   1 +
 drivers/dma-buf/dma-buf.c    |  48 ++++++-
 drivers/dma-buf/st-dma-buf.c | 253 +++++++++++++++++++++++++++++++++++
 include/linux/scatterlist.h  |  26 +++-
 lib/scatterlist.c            |  22 +++
 lib/sg_split.c               |   2 +
 8 files changed, 372 insertions(+), 4 deletions(-)
 create mode 100644 drivers/dma-buf/st-dma-buf.c


base-commit: fe2ec83746e501645709761605c2464a44fd2929
-- 
2.53.0


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

* [RFC PATCH 1/3] dma-buf: keep the DMA flags in the DMABUF_DEBUG copy
  2026-10-05  6:41 [RFC PATCH 0/3] dma-buf: warn-only mode for DMABUF_DEBUG Karl Mehltretter
@ 2026-10-05  6:41 ` Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 2/3] dma-buf: add a warn-only mode to DMABUF_DEBUG Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 3/3] dma-buf: test the debug scatterlist wrapper Karl Mehltretter
  2 siblings, 0 replies; 4+ messages in thread
From: Karl Mehltretter @ 2026-10-05  6:41 UTC (permalink / raw)
  To: Sumit Semwal, Christian König
  Cc: Karl Mehltretter, Andrew Morton, Jason Gunthorpe, Rob Clark,
	Jianfeng Liu, Diederik de Haas, Andy Shevchenko, Vinod Koul,
	Bjorn Andersson, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel

With DMABUF_DEBUG, dma_buf_map_attachment() hands the importer a copy
of the sg_table that keeps only sg_dma_address() and sg_dma_len(). The
dma_flags (SG_DMA_BUS_ADDRESS, SG_DMA_SWIOTLB) are dropped.

The flags describe the DMA side of an entry, which is the side
importers may use. Copy them as well.

This is not a bug fix. No importer reads the flags today. Their only
readers are dma-iommu, dma-direct and iommu_map_sg(), which are not
supposed to see an importer's copy at all. The warn mode added in the
next patch keeps its marker in dma_flags and lets importers that do
get there continue. They should then see the same flags as without
DMABUF_DEBUG.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Notes:
    Built alone on x86_64 with NEED_SG_DMA_FLAGS=y (dma-buf.o, W=1). The
    KUnit test in patch 3 checks the flags in the copy. It passes on x86_64
    under QEMU in strict and in warn mode.

 drivers/dma-buf/dma-buf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 4c9add51f9ef..b3d311acb883 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -904,6 +904,10 @@ static int dma_buf_wrap_sg_table(struct sg_table **sg_table)
 		sg_assign_page(to_sg, NULL);
 		sg_dma_address(to_sg) = sg_dma_address(from_sg);
 		sg_dma_len(to_sg) = sg_dma_len(from_sg);
+#ifdef CONFIG_NEED_SG_DMA_FLAGS
+		/* the flags describe the DMA side, e.g. SG_DMA_BUS_ADDRESS */
+		to_sg->dma_flags = from_sg->dma_flags;
+#endif
 		to_sg = sg_next(to_sg);
 	}
 
-- 
2.53.0


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

* [RFC PATCH 2/3] dma-buf: add a warn-only mode to DMABUF_DEBUG
  2026-10-05  6:41 [RFC PATCH 0/3] dma-buf: warn-only mode for DMABUF_DEBUG Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 1/3] dma-buf: keep the DMA flags in the DMABUF_DEBUG copy Karl Mehltretter
@ 2026-10-05  6:41 ` Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 3/3] dma-buf: test the debug scatterlist wrapper Karl Mehltretter
  2 siblings, 0 replies; 4+ messages in thread
From: Karl Mehltretter @ 2026-10-05  6:41 UTC (permalink / raw)
  To: Sumit Semwal, Christian König
  Cc: Karl Mehltretter, Andrew Morton, Jason Gunthorpe, Rob Clark,
	Jianfeng Liu, Diederik de Haas, Andy Shevchenko, Vinod Koul,
	Bjorn Andersson, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel

DMABUF_DEBUG hands importers a copy of the exporter's sg_table without
the struct pages. An importer that uses them then fails, often far from
the cause and without a message that points at it. Where sg_dma_len()
is sg->length, the copy cannot hide the length either.

Add DMABUF_DEBUG_WARN. With it the copy keeps the CPU side of the
exporter's table: page, offset and length of all orig_nents entries,
and the exporter's nents and orig_nents. A table without a CPU side
(orig_nents == 0) stays that way. Every entry of the copy is marked
with a new dma_flags bit, SG_DMA_DMABUF_DEBUG.

sg_page(), sg_nents_for_len() and sg_split() test the bit and print a
rate limited message with a stack trace. sg_page() is mostly reached
through DMA and scatterlist helpers, so the trace is what names the
importer. These CPU-side accesses can continue after the report instead
of failing because the fields were cleared.

The report is not a WARN(). It does not taint the kernel and does not
trigger panic_on_warn.

Reads of sg->offset and sg->length that do not go through these
helpers are not seen. The option adds a test to every sg_page() and
selects NEED_SG_DMA_FLAGS.

Strict mode remains the default and continues to remove the CPU-side
fields.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Notes:
    Tested on v7.3-rc4-70-gfe2ec83746e5 with GCC 15.2.0.
    
    Builds, without warnings:
    
    - x86_64 with DMABUF_DEBUG_WARN=y, full build
    - arm64 defconfig with DMABUF_DEBUG_WARN=y: Image, and the msm and
      rockchip DRM drivers
    - ARM926 (SAM9X75) with DMABUF_DEBUG_WARN=y and without
      NEED_SG_DMA_LENGTH, full build
    - ARM926 with DMABUF_DEBUG=n and without NEED_SG_DMA_FLAGS, full build.
      dma-buf.o and lib/scatterlist.o disassemble as on the base commit,
      except for __LINE__ constants.
    - x86_64 with SG_SPLIT, W=1, the objects touched here: with
      DMABUF_DEBUG_WARN=y, with DMABUF_DEBUG=y alone, and with
      DMABUF_DEBUG=n
    
    Runtime, both in QEMU with local device models, not on hardware.
    
    Zynq with an AXI ADC, IIO DMABUF capture into udmabuf buffers,
    NEED_SG_DMA_LENGTH=y. The IIO dmaengine buffer calls sg_nents_for_len()
    on the attachment's table. With DMABUF_DEBUG=y alone the capture fails:
    
      DMABUF_ENQUEUE failed: Device or resource busy
    
    With DMABUF_DEBUG_WARN=y all 16 blocks arrive with the right data, the
    kernel is not tainted, and the log has one report:
    
      DMA-BUF: importer used the CPU side of an exporter's sg_table
      CPU: 0 UID: 0 PID: 48 Comm: iio-dmabuf Not tainted 7.3.0-rc4+ #2 VOLUNTARY
      Hardware name: Xilinx Zynq Platform
      Call trace:
       unwind_backtrace from show_stack+0x10/0x14
       show_stack from dump_stack_lvl+0x54/0x68
       dump_stack_lvl from sg_nents_for_len+0xd8/0xe4
       sg_nents_for_len from iio_dmaengine_buffer_submit_block+0x4c/0x33c
       iio_dmaengine_buffer_submit_block from iio_dma_buffer_submit_block.part.0+0x5c/0x104
       iio_dma_buffer_submit_block.part.0 from iio_dma_buffer_enqueue_dmabuf+0x68/0xa0
       iio_dma_buffer_enqueue_dmabuf from iio_buffer_chrdev_ioctl+0x520/0x9a4
       iio_buffer_chrdev_ioctl from sys_ioctl+0x460/0x914
       sys_ioctl from ret_fast_syscall+0x0/0x4c
    
    x86_64 with intel-iommu, xHCI and a SUR40, capture into udmabuf
    buffers. This kernel had a one-line test-only change in sur40 that
    makes the vb2 queue use the USB controller's DMA device. There was no
    report during boot. The capture gives one, from sg_page() in
    iommu_dma_map_sg():
    
      DMA-BUF: importer used the CPU side of an exporter's sg_table
      CPU: 0 UID: 0 PID: 9 Comm: kworker/0:0 Not tainted 7.3.0-rc4+ #5 PREEMPT(lazy)
      Workqueue: events_freezable input_dev_poller_work
      Call Trace:
       <TASK>
       dump_stack_lvl+0x4d/0x70
       iommu_dma_map_sg+0x4c0/0x1010
       __dma_map_sg_attrs+0x254/0x3b0
       dma_map_sg_attrs+0xe/0x20
       usb_hcd_map_urb_for_dma+0x7e0/0x1620
       usb_hcd_submit_urb+0x162/0x1af0
       usb_sg_wait+0x17c/0x550
       sur40_poll+0xb3e/0xff0
       input_dev_poller_work+0x54/0x90
       process_one_work+0x692/0xf90
       [...]
    
    The capture did not finish in this setup. The guest stalled in
    xhci_queue_bulk_tx() after the report, so this run only shows the
    report.
    
    The SG_DMA_* defines move up in scatterlist.h because sg_page() needs
    the new one.

 drivers/dma-buf/Kconfig     | 23 +++++++++++++++++++
 drivers/dma-buf/dma-buf.c   | 44 ++++++++++++++++++++++++++++++++++++-
 include/linux/scatterlist.h | 26 +++++++++++++++++++---
 lib/scatterlist.c           | 22 +++++++++++++++++++
 lib/sg_split.c              |  2 ++
 5 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index e4f078a326a4..06177465091a 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -49,6 +49,29 @@ config DMABUF_DEBUG
 	  exporters. Specifically it validates that importers do not peek at the
 	  underlying struct page when they import a buffer.
 
+config DMABUF_DEBUG_WARN
+	bool "Warn instead of hiding the pages from DMA-BUF importers"
+	depends on DMABUF_DEBUG
+	select NEED_SG_DMA_FLAGS
+	help
+	  DMABUF_DEBUG normally hands importers a copy of the exporter's
+	  sg_table without the struct page pointers, so that an importer which
+	  uses them fails early.
+
+	  With this option the copy keeps the pages, offsets and lengths and is
+	  only marked. sg_page() and the helpers built on it, sg_nents_for_len()
+	  and sg_split() then print a rate limited message with a stack trace
+	  when they are used on such a table. The access itself continues
+	  instead of failing because the fields were cleared. The message is
+	  not a WARN(): it does not taint the kernel or trigger panic_on_warn.
+
+	  Importers that read sg->offset or sg->length directly are not
+	  noticed. The option adds a test to every sg_page() call. It selects
+	  NEED_SG_DMA_FLAGS, which adds dma_flags and may increase the size of
+	  struct scatterlist.
+
+	  If unsure, say N.
+
 config DMABUF_KUNIT_TEST
 	tristate "KUnit tests for DMA-BUF" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index b3d311acb883..bd82b6b7f5ff 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -57,6 +57,7 @@
 struct dma_buf_sg_table_wrapper {
 	struct sg_table *original;
 	struct sg_table wrapper;
+	unsigned int alloc_nents;
 };
 
 static inline int is_dma_buf_file(struct file *);
@@ -874,11 +875,42 @@ void dma_buf_put(struct dma_buf *dmabuf)
 }
 EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
 
+#ifdef CONFIG_DMABUF_DEBUG_WARN
+/*
+ * Warn mode: the importer also gets the CPU side of the exporter's table,
+ * marked so that sg_page() and friends can report who uses it.
+ */
+static void dma_buf_wrap_cpu_side(struct sg_table *to, struct sg_table *from)
+{
+	struct scatterlist *to_sg, *from_sg;
+	int i;
+
+	for_each_sgtable_sg(to, to_sg, i)
+		to_sg->dma_flags |= SG_DMA_DMABUF_DEBUG;
+
+	to_sg = to->sgl;
+	for_each_sgtable_sg(from, from_sg, i) {
+		sg_assign_page(to_sg, sg_page(from_sg));
+		to_sg->offset = from_sg->offset;
+		to_sg->length = from_sg->length;
+		to_sg = sg_next(to_sg);
+	}
+
+	to->nents = from->nents;
+	to->orig_nents = from->orig_nents;
+}
+#else
+static void dma_buf_wrap_cpu_side(struct sg_table *to, struct sg_table *from)
+{
+}
+#endif
+
 static int dma_buf_wrap_sg_table(struct sg_table **sg_table)
 {
 	struct scatterlist *to_sg, *from_sg;
 	struct sg_table *from = *sg_table;
 	struct dma_buf_sg_table_wrapper *to;
+	unsigned int nents = from->nents;
 	int i, ret;
 
 	if (!IS_ENABLED(CONFIG_DMABUF_DEBUG))
@@ -888,14 +920,21 @@ static int dma_buf_wrap_sg_table(struct sg_table **sg_table)
 	 * To catch abuse of the underlying struct page by importers copy the
 	 * sg_table without copying the page_link and give only the copy back to
 	 * the importer.
+	 *
+	 * With DMABUF_DEBUG_WARN the copy keeps the CPU side, which can have
+	 * more entries than the DMA side.
 	 */
 	to = kzalloc_obj(*to);
 	if (!to)
 		return -ENOMEM;
 
-	ret = sg_alloc_table(&to->wrapper, from->nents, GFP_KERNEL);
+	if (IS_ENABLED(CONFIG_DMABUF_DEBUG_WARN))
+		nents = max(nents, from->orig_nents);
+
+	ret = sg_alloc_table(&to->wrapper, nents, GFP_KERNEL);
 	if (ret)
 		goto free_to;
+	to->alloc_nents = nents;
 
 	to_sg = to->wrapper.sgl;
 	for_each_sgtable_dma_sg(from, from_sg, i) {
@@ -910,6 +949,7 @@ static int dma_buf_wrap_sg_table(struct sg_table **sg_table)
 #endif
 		to_sg = sg_next(to_sg);
 	}
+	dma_buf_wrap_cpu_side(&to->wrapper, from);
 
 	to->original = from;
 	*sg_table = &to->wrapper;
@@ -929,6 +969,8 @@ static void dma_buf_unwrap_sg_table(struct sg_table **sg_table)
 
 	copy = container_of(*sg_table, typeof(*copy), wrapper);
 	*sg_table = copy->original;
+	/* sg_free_table() needs the number of allocated entries */
+	copy->wrapper.orig_nents = copy->alloc_nents;
 	sg_free_table(&copy->wrapper);
 	kfree(copy);
 }
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 6de1a2434299..5dc8c134ca4c 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -21,6 +21,13 @@ struct scatterlist {
 #endif
 };
 
+/* Bits in dma_flags, see below. sg_page() needs SG_DMA_DMABUF_DEBUG. */
+#ifdef CONFIG_NEED_SG_DMA_FLAGS
+#define SG_DMA_BUS_ADDRESS	(1 << 0)
+#define SG_DMA_SWIOTLB		(1 << 1)
+#define SG_DMA_DMABUF_DEBUG	(1 << 2)
+#endif
+
 /*
  * These macros should be used after a dma_map_sg call has been done
  * to get bus addresses of each of the SG entries and their lengths.
@@ -188,11 +195,27 @@ static inline void sg_set_folio(struct scatterlist *sg, struct folio *folio,
 	sg->length = len;
 }
 
+#ifdef CONFIG_DMABUF_DEBUG_WARN
+void sg_dmabuf_cpu_access_warn(void);
+
+/* Report use of the CPU side of a table that a DMA-BUF importer was given */
+static inline void sg_dmabuf_cpu_access_check(struct scatterlist *sg)
+{
+	if (unlikely(sg->dma_flags & SG_DMA_DMABUF_DEBUG))
+		sg_dmabuf_cpu_access_warn();
+}
+#else
+static inline void sg_dmabuf_cpu_access_check(struct scatterlist *sg)
+{
+}
+#endif
+
 static inline struct page *sg_page(struct scatterlist *sg)
 {
 #ifdef CONFIG_DEBUG_SG
 	BUG_ON(sg_is_chain(sg));
 #endif
+	sg_dmabuf_cpu_access_check(sg);
 	return (struct page *)((sg)->page_link & ~SG_PAGE_LINK_MASK);
 }
 
@@ -303,9 +326,6 @@ static inline void sg_unmark_end(struct scatterlist *sg)
  */
 #ifdef CONFIG_NEED_SG_DMA_FLAGS
 
-#define SG_DMA_BUS_ADDRESS	(1 << 0)
-#define SG_DMA_SWIOTLB		(1 << 1)
-
 /**
  * sg_dma_is_bus_address - Return whether a given segment was marked
  *			   as a bus address
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 6ea40d2e6247..55a3697df881 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -12,6 +12,27 @@
 #include <linux/bvec.h>
 #include <linux/uio.h>
 #include <linux/folio_queue.h>
+#include <linux/printk.h>
+#include <linux/ratelimit.h>
+
+#ifdef CONFIG_DMABUF_DEBUG_WARN
+/*
+ * Not a WARN(): a wrong importer must not taint the kernel or trigger
+ * panic_on_warn. sg_page() is mostly reached through DMA or scatterlist
+ * helpers, so the stack trace is what identifies the importer.
+ */
+void sg_dmabuf_cpu_access_warn(void)
+{
+	static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, 1);
+
+	if (!__ratelimit(&rs))
+		return;
+
+	pr_warn("DMA-BUF: importer used the CPU side of an exporter's sg_table\n");
+	dump_stack_lvl(KERN_WARNING);
+}
+EXPORT_SYMBOL(sg_dmabuf_cpu_access_warn);
+#endif
 
 /**
  * sg_nents - return total count of entries in scatterlist
@@ -54,6 +75,7 @@ int sg_nents_for_len(struct scatterlist *sg, u64 len)
 		return 0;
 
 	for (nents = 0, total = 0; sg; sg = sg_next(sg)) {
+		sg_dmabuf_cpu_access_check(sg);
 		nents++;
 		total += sg->length;
 		if (total >= len)
diff --git a/lib/sg_split.c b/lib/sg_split.c
index 24e8f5e48e63..cbbc7f9a206d 100644
--- a/lib/sg_split.c
+++ b/lib/sg_split.c
@@ -33,6 +33,8 @@ static int sg_calculate_split(struct scatterlist *in, int nents, int nb_splits,
 	}
 
 	for_each_sg(in, sg, nents, i) {
+		if (!mapped)
+			sg_dmabuf_cpu_access_check(sg);
 		sglen = mapped ? sg_dma_len(sg) : sg->length;
 		if (skip > sglen) {
 			skip -= sglen;
-- 
2.53.0


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

* [RFC PATCH 3/3] dma-buf: test the debug scatterlist wrapper
  2026-10-05  6:41 [RFC PATCH 0/3] dma-buf: warn-only mode for DMABUF_DEBUG Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 1/3] dma-buf: keep the DMA flags in the DMABUF_DEBUG copy Karl Mehltretter
  2026-10-05  6:41 ` [RFC PATCH 2/3] dma-buf: add a warn-only mode to DMABUF_DEBUG Karl Mehltretter
@ 2026-10-05  6:41 ` Karl Mehltretter
  2 siblings, 0 replies; 4+ messages in thread
From: Karl Mehltretter @ 2026-10-05  6:41 UTC (permalink / raw)
  To: Sumit Semwal, Christian König
  Cc: Karl Mehltretter, Andrew Morton, Jason Gunthorpe, Rob Clark,
	Jianfeng Liu, Diederik de Haas, Andy Shevchenko, Vinod Koul,
	Bjorn Andersson, linux-media, dri-devel, linaro-mm-sig,
	linux-kernel

Map a dma-buf from a mock exporter and check the sg_table the importer
gets. The exporter's table has one DMA entry and one, two or no CPU
entries. That covers a mapping that merged entries and a table without
a CPU side.

In all modes the DMA address, length and existing DMA flags must be
preserved, and unmap must give the exporter its own table back. In
strict mode the CPU fields must be cleared. In warn mode they must be
there for all CPU entries and every entry must be marked.

The warn mode check calls sg_page() and sg_nents_for_len() on the copy
on purpose, so it prints one report.

The .kunitconfig now sets DMABUF_DEBUG. For the warn mode add
--kconfig_add CONFIG_DMABUF_DEBUG_WARN=y.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Notes:
    KUnit, the dma-buf suites, with
    tools/testing/kunit/kunit.py run --kunitconfig drivers/dma-buf/.kunitconfig:
    
    - UML, strict mode (the .kunitconfig as it is): 56 passed, 1 skipped
    - UML, --kconfig_add CONFIG_DMABUF_DEBUG_WARN=y: 56 passed, 1 skipped
    - UML, --kconfig_add CONFIG_DMABUF_DEBUG=n: 56 passed, 1 skipped
    - x86_64 in QEMU (--arch x86_64), strict mode: 56 passed, 1 skipped
    - x86_64 in QEMU, DMABUF_DEBUG_WARN=y: 56 passed, 1 skipped
    - x86_64 in QEMU with KASAN, both modes: 56 passed, 1 skipped, no
      KASAN report
    
    The skipped test is test_race_signal_callback, which needs 2 CPUs. UML
    has no NEED_SG_DMA_LENGTH, x86_64 has it.
    
    In warn mode the test prints one report. On x86_64:
    
      DMA-BUF: importer used the CPU side of an exporter's sg_table
      [...]
      Call Trace:
       <TASK>
       dump_stack_lvl+0x2f/0x50
       check_warn+0x113/0x4b0
       test_debug_sg_table+0x2c4/0x740
       kunit_try_run_case+0x8e/0x120
       kunit_generic_run_threadfn_adapter+0x1c/0x40
       kthread+0xc5/0x100
    
    The test also builds as a module on x86_64 (DMABUF_KUNIT_TEST=m).

 drivers/dma-buf/.kunitconfig |   1 +
 drivers/dma-buf/Makefile     |   1 +
 drivers/dma-buf/st-dma-buf.c | 253 +++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+)
 create mode 100644 drivers/dma-buf/st-dma-buf.c

diff --git a/drivers/dma-buf/.kunitconfig b/drivers/dma-buf/.kunitconfig
index 1ce5fb7e6cf9..7dcf41de464e 100644
--- a/drivers/dma-buf/.kunitconfig
+++ b/drivers/dma-buf/.kunitconfig
@@ -1,2 +1,3 @@
 CONFIG_KUNIT=y
 CONFIG_DMABUF_KUNIT_TEST=y
+CONFIG_DMABUF_DEBUG=y
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index b25d7550bacf..e34f18c4a6ed 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_SW_SYNC)		+= sw_sync.o sync_debug.o
 obj-$(CONFIG_UDMABUF)		+= udmabuf.o
 
 dmabuf_kunit-y := \
+	st-dma-buf.o \
 	st-dma-fence.o \
 	st-dma-fence-chain.o \
 	st-dma-fence-unwrap.o \
diff --git a/drivers/dma-buf/st-dma-buf.c b/drivers/dma-buf/st-dma-buf.c
new file mode 100644
index 000000000000..892c195b749c
--- /dev/null
+++ b/drivers/dma-buf/st-dma-buf.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Test the sg_table that dma_buf_map_attachment() hands to importers.
+ */
+
+#include <kunit/device.h>
+#include <kunit/test.h>
+
+#include <linux/dma-buf.h>
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+
+#define MOCK_DMA_ADDR	0x12340000
+#define MOCK_ORDER	2
+
+struct mock_param {
+	const char *desc;
+	/* Entries of the CPU side. The DMA side always has one. */
+	unsigned int orig_nents;
+};
+
+struct mock_buf {
+	struct kunit *test;
+	struct sg_table sgt;
+	unsigned int alloc_nents;
+	struct page *pages;
+	bool unmapped;
+};
+
+static struct sg_table *mock_map(struct dma_buf_attachment *attach,
+				 enum dma_data_direction dir)
+{
+	struct mock_buf *buf = attach->dmabuf->priv;
+
+	return &buf->sgt;
+}
+
+static void mock_unmap(struct dma_buf_attachment *attach, struct sg_table *sgt,
+		       enum dma_data_direction dir)
+{
+	struct mock_buf *buf = attach->dmabuf->priv;
+
+	/* The exporter gets its own table back, not the copy */
+	KUNIT_EXPECT_PTR_EQ(buf->test, sgt, &buf->sgt);
+	buf->unmapped = true;
+}
+
+static void mock_free(struct mock_buf *buf)
+{
+	buf->sgt.orig_nents = buf->alloc_nents;
+	sg_free_table(&buf->sgt);
+	__free_pages(buf->pages, MOCK_ORDER);
+	kfree(buf);
+}
+
+/* Runs from delayed fput, the test may be gone by then */
+static void mock_release(struct dma_buf *dmabuf)
+{
+	mock_free(dmabuf->priv);
+}
+
+static const struct dma_buf_ops mock_ops = {
+	.map_dma_buf = mock_map,
+	.unmap_dma_buf = mock_unmap,
+	.release = mock_release,
+};
+
+/* A table as an exporter would return it, with a made up DMA mapping */
+static struct mock_buf *mock_alloc(struct kunit *test, unsigned int orig_nents)
+{
+	struct scatterlist *sg;
+	struct mock_buf *buf;
+	unsigned int i;
+
+	buf = kzalloc_obj(*buf);
+	if (!buf)
+		return NULL;
+
+	buf->pages = alloc_pages(GFP_KERNEL, MOCK_ORDER);
+	if (!buf->pages)
+		goto err_buf;
+
+	buf->alloc_nents = max(orig_nents, 1U);
+	if (sg_alloc_table(&buf->sgt, buf->alloc_nents, GFP_KERNEL))
+		goto err_pages;
+
+	/* An offset of a page keeps the lengths page aligned */
+	for_each_sg(buf->sgt.sgl, sg, orig_nents, i)
+		sg_set_page(sg, buf->pages + 2 * i, PAGE_SIZE, PAGE_SIZE);
+
+	sg = buf->sgt.sgl;
+	sg_dma_address(sg) = MOCK_DMA_ADDR;
+	/* Without NEED_SG_DMA_LENGTH the DMA length is the CPU length */
+	if (IS_ENABLED(CONFIG_NEED_SG_DMA_LENGTH) || !orig_nents)
+		sg_dma_len(sg) = buf->alloc_nents * PAGE_SIZE;
+#ifdef CONFIG_NEED_SG_DMA_FLAGS
+	sg_dma_mark_bus_address(sg);
+	sg_dma_mark_swiotlb(sg);
+#endif
+	buf->sgt.nents = 1;
+	buf->sgt.orig_nents = orig_nents;
+	buf->test = test;
+
+	return buf;
+
+err_pages:
+	__free_pages(buf->pages, MOCK_ORDER);
+err_buf:
+	kfree(buf);
+	return NULL;
+}
+
+/* Strict mode: only the DMA side, the CPU fields are cleared */
+static void check_strict(struct kunit *test, struct sg_table *sgt)
+{
+	struct scatterlist *sg;
+	int i;
+
+	KUNIT_EXPECT_EQ(test, sgt->orig_nents, sgt->nents);
+
+	for_each_sgtable_sg(sgt, sg, i) {
+		KUNIT_EXPECT_NULL(test, sg_page(sg));
+		KUNIT_EXPECT_EQ(test, sg->offset, 0U);
+		if (IS_ENABLED(CONFIG_NEED_SG_DMA_LENGTH))
+			KUNIT_EXPECT_EQ(test, sg->length, 0U);
+	}
+}
+
+#ifdef CONFIG_DMABUF_DEBUG_WARN
+/* Warn mode: the CPU side is all there, and all entries are marked */
+static void check_warn(struct kunit *test, struct sg_table *sgt,
+		       struct sg_table *orig)
+{
+	struct scatterlist *orig_sg = orig->sgl;
+	struct scatterlist *sg;
+	u64 len = 0;
+	int i;
+
+	KUNIT_EXPECT_EQ(test, sgt->orig_nents, orig->orig_nents);
+	KUNIT_EXPECT_TRUE(test, sgt->sgl->dma_flags & SG_DMA_DMABUF_DEBUG);
+	KUNIT_EXPECT_FALSE(test, orig->sgl->dma_flags & SG_DMA_DMABUF_DEBUG);
+
+	/* sg_page() and sg_nents_for_len() report this, rate limited */
+	for_each_sgtable_sg(sgt, sg, i) {
+		KUNIT_EXPECT_TRUE(test, sg->dma_flags & SG_DMA_DMABUF_DEBUG);
+		KUNIT_EXPECT_PTR_EQ(test, sg_page(sg), sg_page(orig_sg));
+		KUNIT_EXPECT_EQ(test, sg->offset, orig_sg->offset);
+		KUNIT_EXPECT_EQ(test, sg->length, orig_sg->length);
+		len += sg->length;
+		orig_sg = sg_next(orig_sg);
+	}
+
+	if (len)
+		KUNIT_EXPECT_EQ(test, sg_nents_for_len(sgt->sgl, len),
+				(int)orig->orig_nents);
+	else
+		KUNIT_EXPECT_NULL(test, sg_page(sgt->sgl));
+}
+#else
+static void check_warn(struct kunit *test, struct sg_table *sgt,
+		       struct sg_table *orig)
+{
+}
+#endif
+
+static void test_debug_sg_table(struct kunit *test)
+{
+	const struct mock_param *param = test->param_value;
+	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+	struct dma_buf_attachment *attach;
+	struct dma_buf *dmabuf;
+	struct sg_table *sgt;
+	struct mock_buf *buf;
+	struct device *dev;
+
+	dev = kunit_device_register(test, "dma-buf-test");
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+
+	buf = mock_alloc(test, param->orig_nents);
+	KUNIT_ASSERT_NOT_NULL(test, buf);
+
+	exp_info.ops = &mock_ops;
+	exp_info.size = buf->alloc_nents * PAGE_SIZE;
+	exp_info.flags = O_RDWR;
+	exp_info.priv = buf;
+	dmabuf = dma_buf_export(&exp_info);
+	if (IS_ERR(dmabuf)) {
+		mock_free(buf);
+		KUNIT_FAIL(test, "dma_buf_export: %pe", dmabuf);
+		return;
+	}
+
+	attach = dma_buf_attach(dmabuf, dev);
+	if (IS_ERR(attach)) {
+		KUNIT_FAIL(test, "dma_buf_attach: %pe", attach);
+		goto out_put;
+	}
+
+	sgt = dma_buf_map_attachment_unlocked(attach, DMA_BIDIRECTIONAL);
+	if (IS_ERR(sgt)) {
+		KUNIT_FAIL(test, "dma_buf_map_attachment: %pe", sgt);
+		goto out_detach;
+	}
+
+	/* The DMA side is the same in all modes */
+	KUNIT_EXPECT_EQ(test, sgt->nents, 1U);
+	KUNIT_EXPECT_EQ(test, sg_dma_address(sgt->sgl), (dma_addr_t)MOCK_DMA_ADDR);
+	KUNIT_EXPECT_EQ(test, sg_dma_len(sgt->sgl), sg_dma_len(buf->sgt.sgl));
+#ifdef CONFIG_NEED_SG_DMA_FLAGS
+	KUNIT_EXPECT_TRUE(test, sg_dma_is_bus_address(sgt->sgl));
+	KUNIT_EXPECT_TRUE(test, sg_dma_is_swiotlb(sgt->sgl));
+#endif
+
+	if (!IS_ENABLED(CONFIG_DMABUF_DEBUG))
+		KUNIT_EXPECT_PTR_EQ(test, sgt, &buf->sgt);
+	else if (IS_ENABLED(CONFIG_DMABUF_DEBUG_WARN))
+		check_warn(test, sgt, &buf->sgt);
+	else
+		check_strict(test, sgt);
+
+	dma_buf_unmap_attachment_unlocked(attach, sgt, DMA_BIDIRECTIONAL);
+	KUNIT_EXPECT_TRUE(test, buf->unmapped);
+	KUNIT_EXPECT_EQ(test, buf->sgt.nents, 1U);
+	KUNIT_EXPECT_EQ(test, buf->sgt.orig_nents, param->orig_nents);
+
+out_detach:
+	dma_buf_detach(dmabuf, attach);
+out_put:
+	dma_buf_put(dmabuf);
+}
+
+static const struct mock_param mock_params[] = {
+	{ .desc = "one CPU entry", .orig_nents = 1 },
+	{ .desc = "two CPU entries merged", .orig_nents = 2 },
+	{ .desc = "no CPU side", .orig_nents = 0 },
+};
+
+KUNIT_ARRAY_PARAM_DESC(mock, mock_params, desc);
+
+static struct kunit_case dma_buf_test_cases[] = {
+	KUNIT_CASE_PARAM(test_debug_sg_table, mock_gen_params),
+	{}
+};
+
+static struct kunit_suite dma_buf_test_suite = {
+	.name = "dma-buf",
+	.test_cases = dma_buf_test_cases,
+};
+
+kunit_test_suite(dma_buf_test_suite);
+
+MODULE_IMPORT_NS("DMA_BUF");
-- 
2.53.0


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

end of thread, other threads:[~2026-10-05  6:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-05  6:41 [RFC PATCH 0/3] dma-buf: warn-only mode for DMABUF_DEBUG Karl Mehltretter
2026-10-05  6:41 ` [RFC PATCH 1/3] dma-buf: keep the DMA flags in the DMABUF_DEBUG copy Karl Mehltretter
2026-10-05  6:41 ` [RFC PATCH 2/3] dma-buf: add a warn-only mode to DMABUF_DEBUG Karl Mehltretter
2026-10-05  6:41 ` [RFC PATCH 3/3] dma-buf: test the debug scatterlist wrapper Karl Mehltretter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®