[PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[David Hu]

From: David Hu <xuehaohu@google.com>

This series address two related issues in scatter-gather mapping,
specifically for the MMIO based dma-buf mapping. The fixes ensure
sgt mapping is correct, and proper for large MMIO regions.

Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
(Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
phys vec to sgt)
https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/

Patch 2 Splits sgl by largest page aligned chunk
(Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/

Changes in v8:
 - Combined the two patches into one unified series to avoid merge
   conflicts.
 - Collected Reviewed-by tag from Leon Romanovsky for Patch 2.

David Hu (2):
  dma-buf: Fix silent overflow for phys vec to sgt
  dma-buf: Split sgl by largest page-aligned chunk

 drivers/dma-buf/dma-buf-mapping.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH v8 1/2] dma-buf: Fix silent overflow for phys vec to sgt

[David Hu]

From: David Hu <xuehaohu@google.com>

In case MMIO size is bigger than 4G and peer2peer DMA goes
through host bridge, we trigger a code path that assigns the
total linked IOVA (which is greater than 4G) to mapped_len.

Previously, `mapped_len` was declared as 32-bit `unsigned int`.
When accumulating `size_t` lengths, this leads to a silent wrap-around.
This truncation causes truncated lengths to be passed to functions
like `fill_sg_entry()`.

Fix this by changing `mapped_len` to `size_t` (64-bit). While
at it, fix similar potential overflow issues in `calc_sg_nents`
by using `check_add_overflow()` for `nents` and using
`unsigned int` for the loop iterator in `fill_sg_entry` to match.

Fixes: 3aa31a8bb11e ("dma-buf: provide phys_vec to scatter-gather mapping routine")
Cc: stable@vger.kernel.org
Cc: iommu@lists.linux.dev
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: David Hu <xuehaohu@google.com>
---
Changes in v7:
 - Added a missing blank line after local variable declaration in
   `calc_sg_nents()` (Leon).
 - Collected Reviewed-by from Leon Romanovsky.

Changes in v6:
 - Used `check_add_overflow()` in `calc_sg_nents()` for safer
   accumulation (Leon).
 - Dropped explicit `!nents` check and added a comment noting that
   `sg_alloc_table` handles `nents == 0` (Leon).
 - Collected Reviewed-by from Kevin Tian.

Changes in v5:
 - Removed WARN_ON_ONCE from calc_sg_nents() to avoid log noise (Jason).
 - Added explicit check for `!nents` in dma_buf_phys_vec_to_sgt() to
   cleanly return -EINVAL on overflow (Jason).

Changes in v4:
 - Added WARN_ON_ONCE() to the nents overflow check to prevent silent
   failures (Claude Bot).

Changes in v3:
 - Removed leftover sentence fragment from the commit message.
 - Kept `nents = 0` initialization (previously stated as removed in the
   v2 changelog) as it is strictly required for the `+=` accumulation
   loop in `calc_sg_nents()`.

Changes in v2:
 - Fixed 'IVOA' -> 'IOVA' typo and expanded commit message (Claude Bot).
 - Added Reverse Xmas tree formatting (Pranjal).
 - Folded in extra bounds checking for calc_sg_nents() (Pranjal).
 - Folded in type consistency fix for fill_sg_entry() (Pranjal).
 - Collected Reviewed-by from Pranjal Shrivastava.

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

diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
index 794acff2546a..80f6ab2f4809 100644
--- a/drivers/dma-buf/dma-buf-mapping.c
+++ b/drivers/dma-buf/dma-buf-mapping.c
@@ -5,12 +5,13 @@
  */
 #include <linux/dma-buf-mapping.h>
 #include <linux/dma-resv.h>
+#include <linux/overflow.h>
 
 static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
 					 dma_addr_t addr)
 {
 	unsigned int len, nents;
-	int i;
+	unsigned int i;
 
 	nents = DIV_ROUND_UP(length, UINT_MAX);
 	for (i = 0; i < nents; i++) {
@@ -40,8 +41,12 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
 	size_t i;
 
 	if (!state || !dma_use_iova(state)) {
-		for (i = 0; i < nr_ranges; i++)
-			nents += DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);
+		for (i = 0; i < nr_ranges; i++) {
+			unsigned int added = DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);
+
+			if (check_add_overflow(nents, added, &nents))
+				return 0;
+		}
 	} else {
 		/*
 		 * In IOVA case, there is only one SG entry which spans
@@ -95,9 +100,10 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
 					 size_t nr_ranges, size_t size,
 					 enum dma_data_direction dir)
 {
-	unsigned int nents, mapped_len = 0;
 	struct dma_buf_dma *dma;
 	struct scatterlist *sgl;
+	size_t mapped_len = 0;
+	unsigned int nents;
 	dma_addr_t addr;
 	size_t i;
 	int ret;
@@ -133,6 +139,8 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
 	}
 
 	nents = calc_sg_nents(dma->state, phys_vec, nr_ranges, size);
+
+	/* sg_alloc_table will cleanly fail and return -EINVAL if nents == 0 */
 	ret = sg_alloc_table(&dma->sgt, nents, GFP_KERNEL | __GFP_ZERO);
 	if (ret)
 		goto err_free_state;
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH v8 1/2] dma-buf: Fix silent overflow for phys vec to sgt

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incorrect offset parameter passed to dma_iova_link() causes all physical vectors to overwrite each other at the same IOVA address.
- [Critical] Partial unmapping of DMA mappings via segmented scatterlist entries violates the DMA API.
- [High] DIV_ROUND_UP macro wraps around on 32-bit platforms, causing calc_sg_nents to evaluate to 0 and failing the mapping.
- [High] Setting sgt.orig_nents = 0 breaks sg_free_table, causing a silent memory leak of the scatterlist.
- [High] Error path unmaps all uninitialized scatterlist entries due to an incorrect loop iterator macro shadowing the index.
- [High] IOVA space allocated by dma_iova_try_alloc() is leaked on multiple error paths.
--

commit 3ea75e22b2b7ea7287fcbeb0fa39a0b99a722743
Author: David Hu <xuehaohu@google.com>

dma-buf: Fix silent overflow for phys vec to sgt
    
This commit changes mapped_len to a 64-bit size_t to prevent silent wrap-around
during length accumulation for MMIO greater than 4G. It also introduces
check_add_overflow() in calc_sg_nents() for safer scatterlist entry
accumulation.

> diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
> index 794acff2546a3..80f6ab2f4809d 100644
> --- a/drivers/dma-buf/dma-buf-mapping.c
> +++ b/drivers/dma-buf/dma-buf-mapping.c

[ ... ]

> @@ -40,8 +41,12 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
>  	size_t i;
>  
>  	if (!state || !dma_use_iova(state)) {
> -		for (i = 0; i < nr_ranges; i++)
> -			nents += DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);
> +		for (i = 0; i < nr_ranges; i++) {
> +			unsigned int added = DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);

[Severity: High]
This is a pre-existing issue, but could DIV_ROUND_UP wrap around on 32-bit
platforms here?

Since size_t is 32-bit on those architectures, phys_vec[i].len + UINT_MAX - 1
wraps around the maximum 32-bit value. For any chunk length larger than a page,
added will evaluate to 0, completely failing the DMA mapping.

> +
> +			if (check_add_overflow(nents, added, &nents))
> +				return 0;
> +		}

[ ... ]

> @@ -133,6 +139,8 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
>  	}
>  
>  	nents = calc_sg_nents(dma->state, phys_vec, nr_ranges, size);
> +
> +	/* sg_alloc_table will cleanly fail and return -EINVAL if nents == 0 */
>  	ret = sg_alloc_table(&dma->sgt, nents, GFP_KERNEL | __GFP_ZERO);
>  	if (ret)
>  		goto err_free_state;

[Severity: High]
This is a pre-existing issue, but is it possible this jump leaks the allocated
IOVA space?

If sg_alloc_table fails, the jump to err_free_state frees dma->state, but
doesn't call dma_iova_destroy().

Also, since this patch updates this function, I noticed a few other
pre-existing issues further down in dma_buf_phys_vec_to_sgt():

[Severity: Critical]
This is a pre-existing issue, but will the hardcoded zero offset overwrite
mappings at the start of the IOVA space?

In the loop later in this function:

    } else if (dma_use_iova(dma->state)) {
        ret = dma_iova_link(attach->dev, dma->state,
                            phys_vec[i].paddr, 0,
                            phys_vec[i].len, dir,
                            DMA_ATTR_MMIO);

Passing the literal 0 as the offset for every vector causes all physical
vectors to be mapped to the exact same starting IOVA address. Should this use
the accumulated mapped_len instead?

[Severity: Critical]
This is a pre-existing issue, but does this chunk-by-chunk unmap violate the
DMA API?

The code maps contiguous physical memory as a single chunk:

    addr = dma_map_phys(attach->dev, phys_vec[i].paddr,
                        phys_vec[i].len, dir, DMA_ATTR_MMIO);

But then in fill_sg_entry(), it splits this mapping into multiple scatterlist
entries. During teardown in dma_buf_free_sgt() and err_unmap_dma, it unmaps
these segmented pieces:

    for_each_sgtable_dma_sg(&dma->sgt, sgl, i)
        dma_unmap_phys(attach->dev, sg_dma_address(sgl),
                       sg_dma_len(sgl), dir, DMA_ATTR_MMIO);

Can this partial unmapping of a single mapped region corrupt IOMMU state
or cause undefined behavior?

[Severity: High]
This is a pre-existing issue, but does the error path iterate over
uninitialized scatterlist entries and leak IOVA space?

Looking at err_unmap_dma:

err_unmap_dma:
    if (!i || !dma->state) {
        ; /* Do nothing */

If an error occurs on the first loop iteration, the !i check safely returns but
skips dma_iova_destroy(), leaking the IOVA space here as well.

Furthermore, the loop down in that error path shadows the index variable:

    } else {
        for_each_sgtable_dma_sg(&dma->sgt, sgl, i)
            dma_unmap_phys(...)
    }

Because for_each_sgtable_dma_sg uses i as its internal iterator (iterating
up to dma->sgt.nents), it resets i to 0 and processes all entries, including
uninitialized zero-elements, rather than stopping at the count of successfully
mapped segments.

[Severity: High]
This is a pre-existing issue, but would clearing orig_nents cause a silent
memory leak?

At the end of dma_buf_phys_vec_to_sgt():

    /*
     * No CPU list included — set orig_nents = 0 so others can detect
     * this via SG table (use nents only).
     */
    dma->sgt.orig_nents = 0;

When dma_buf_free_sgt() eventually calls sg_free_table(), the underlying
__sg_free_table() uses table->orig_nents to determine how many chunks to free.
Since it is 0, won't the free loop terminate immediately and silently leak the
entire scatterlist memory allocation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901170849.4052816-1-dhu@x6u.co?part=1

[PATCH v8 2/2] dma-buf: Split sgl by largest page-aligned chunk

[David Hu]

From: David Hu <xuehaohu@google.com>

Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`.
This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the
first entry, resulting in non-page-aligned DMA addresses for all
subsequent entries.

While the underlying IOMMU mapping may be contiguous, hardware
DMA engines often require explicit address alignment (e.g., page,
cacheline, or storage sector boundaries). Passing unaligned
addresses and lengths can cause explicit failures in DMA descriptor
creation or silent data corruption if lower unaligned bits are
truncated.

In addition, a non-page-aligned sgl length will trigger an edge case
in `ib_umem_find_best_pgsz()`. In case of a discontinuity in later
buffers, we will have a `va` with lowest bit set to 1. That will lead
to `ib_umem_find_best_pgsz()` always return 0, and break the promise
to find best page size for the mapping on the NIC side.

Fix this by splitting the scatterlist by the largest possible page
aligned chunk within `UINT_MAX` (`ALIGN_DOWN(UINT_MAX, PAGE_SIZE)`).
This ensures all scatterlist DMA addresses and lengths remain page
aligned, while minimizing the total number of sgl entries.

Page-aligned entries allow the system to cleanly chunk payloads into
PCIe MaxPayloadSize (MPS) (e.g., 128 bytes, 256 bytes, 512 bytes).
As a result, this may help reduce TLP fragmentation in P2P transfers
and alleviate potential congestion within a logical PCIe switch
partition, especially when Relaxed Ordering is not possible due to
hardware constraints.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260609165431.778061F00893@smtp.kernel.org/
Fixes: 3aa31a8bb11e ("dma-buf: provide phys_vec to scatter-gather mapping routine")
Cc: stable@vger.kernel.org
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: David Hu <xuehaohu@google.com>
---
 Changes in v3:
 - Removed the type cast for `min` (David Laight)
 - Reverted max ent size to be `ALIGN_DOWN(UINT_MAX, PAGE_SIZE)` and
   updated commit message to reflect that (Jason Gunthorpe)
 - Updated commit message to reflect that this also fixes an edge case
   in `ib_umem_find_best_pgsz()`

 Changes in v2:
 - Updated commit title and message to reflect the switch to 2G chunks
 - Switch to using 2G as the max sg entry size as it naturally aligns
   with most hardware boundaries, while allowing compiler optimizations
   with bit shifts (David Laight)
 - Optimized away division calculation for `nent`, and multiplication
   calculation for sgl address, by dropping the `for` loop in favor of a
   `while (length)` loop (David Laight)
 - Dropped `min_t` in favor of `min()` to maintain a strict type
   checking safety net (David Laight)

 drivers/dma-buf/dma-buf-mapping.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
index 80f6ab2f4809..833be519e1e6 100644
--- a/drivers/dma-buf/dma-buf-mapping.c
+++ b/drivers/dma-buf/dma-buf-mapping.c
@@ -6,16 +6,17 @@
 #include <linux/dma-buf-mapping.h>
 #include <linux/dma-resv.h>
 #include <linux/overflow.h>
+#include <linux/align.h>
+
+#define MAX_SG_ENT_SZ ALIGN_DOWN(UINT_MAX, PAGE_SIZE)
 
 static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
 					 dma_addr_t addr)
 {
-	unsigned int len, nents;
-	unsigned int i;
+	size_t len;
 
-	nents = DIV_ROUND_UP(length, UINT_MAX);
-	for (i = 0; i < nents; i++) {
-		len = min_t(size_t, length, UINT_MAX);
+	while (length) {
+		len = min(length, MAX_SG_ENT_SZ);
 		length -= len;
 		/*
 		 * DMABUF abuses scatterlist to create a scatterlist
@@ -25,8 +26,10 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
 		 * does not require the CPU list for mapping or unmapping.
 		 */
 		sg_set_page(sgl, NULL, 0, 0);
-		sg_dma_address(sgl) = addr + (dma_addr_t)i * UINT_MAX;
+		sg_dma_address(sgl) = addr;
 		sg_dma_len(sgl) = len;
+		addr += len;
+		/* Unconditionally advance. On last segment, this becomes NULL */
 		sgl = sg_next(sgl);
 	}
 
@@ -42,7 +45,7 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
 
 	if (!state || !dma_use_iova(state)) {
 		for (i = 0; i < nr_ranges; i++) {
-			unsigned int added = DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);
+			unsigned int added = DIV_ROUND_UP(phys_vec[i].len, MAX_SG_ENT_SZ);
 
 			if (check_add_overflow(nents, added, &nents))
 				return 0;
@@ -53,7 +56,7 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
 		 * for whole IOVA address space, but we need to make sure
 		 * that it fits sg->length, maybe we need more.
 		 */
-		nents = DIV_ROUND_UP(size, UINT_MAX);
+		nents = DIV_ROUND_UP(size, MAX_SG_ENT_SZ);
 	}
 
 	return nents;
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH v8 2/2] dma-buf: Split sgl by largest page-aligned chunk

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Critical] Deterministic NULL pointer dereference on 32-bit architectures due to mismatch between scatterlist allocation and iteration count.

Pre-existing issues:
- [High] Piecewise unmapping of chunked DMA regions violates DMA API rules.
- [High] Error handling path shadows the loop counter and unmaps uninitialized scatterlist entries.
--

commit 352582e0a7698d4a99b7c620ded07fa0a65602c3
Author: David Hu <xuehaohu@google.com>

dma-buf: Split sgl by largest page-aligned chunk

This commit changes fill_sg_entry() to split scatterlists by
ALIGN_DOWN(UINT_MAX, PAGE_SIZE) rather than UINT_MAX to ensure entries
remain page-aligned. Page-aligned entries prevent failures in hardware
DMA engines that require explicit address alignment and avoid edge cases
in ib_umem_find_best_pgsz().

Link: https://lore.kernel.org/all/20260609165431.778061F00893@smtp.kernel.org/

> diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
> index 80f6ab2f4809d..833be519e1e6a 100644
> --- a/drivers/dma-buf/dma-buf-mapping.c
> +++ b/drivers/dma-buf/dma-buf-mapping.c
> @@ -6,16 +6,17 @@
>  #include <linux/dma-buf-mapping.h>
>  #include <linux/dma-resv.h>
>  #include <linux/overflow.h>
> +#include <linux/align.h>
> +
> +#define MAX_SG_ENT_SZ ALIGN_DOWN(UINT_MAX, PAGE_SIZE)
>  
>  static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
>  					 dma_addr_t addr)
>  {
> -	unsigned int len, nents;
> -	unsigned int i;
> +	size_t len;
>  
> -	nents = DIV_ROUND_UP(length, UINT_MAX);
> -	for (i = 0; i < nents; i++) {
> -		len = min_t(size_t, length, UINT_MAX);
> +	while (length) {
> +		len = min(length, MAX_SG_ENT_SZ);

[Severity: High]
This is a pre-existing issue, but does chunking the mapped physical region into
smaller pieces here cause a violation of the DMA API during teardown?

During dma_buf_free_sgt() or the err_unmap_dma path, the code iterates over this
scatterlist and calls dma_unmap_phys() on each split chunk individually:

drivers/dma-buf/dma-buf-mapping.c:dma_buf_free_sgt() {
    ...
		for_each_sgtable_dma_sg(sgt, sgl, i)
			dma_unmap_phys(attach->dev, sg_dma_address(sgl),
				       sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
    ...
}

The DMA API mandates that unmapping must use the exact identical start address
and length that was originally passed to dma_map_phys().

>  		length -= len;
>  		/*
>  		 * DMABUF abuses scatterlist to create a scatterlist
> @@ -25,8 +26,10 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length,
>  		 * does not require the CPU list for mapping or unmapping.
>  		 */
>  		sg_set_page(sgl, NULL, 0, 0);

[Severity: Critical]
Since calc_sg_nents() calculates too few entries for lengths > 4096 on 32-bit
architectures due to integer wrapping, does this sg_set_page() unconditionally
dereference the NULL pointer returned by sg_next() when the undersized
scatterlist allocation is exhausted?

> -		sg_dma_address(sgl) = addr + (dma_addr_t)i * UINT_MAX;
> +		sg_dma_address(sgl) = addr;
>  		sg_dma_len(sgl) = len;
> +		addr += len;
> +		/* Unconditionally advance. On last segment, this becomes NULL */
>  		sgl = sg_next(sgl);
>  	}
>  
> @@ -42,7 +45,7 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
>  
>  	if (!state || !dma_use_iova(state)) {
>  		for (i = 0; i < nr_ranges; i++) {
> -			unsigned int added = DIV_ROUND_UP(phys_vec[i].len, UINT_MAX);
> +			unsigned int added = DIV_ROUND_UP(phys_vec[i].len, MAX_SG_ENT_SZ);

[Severity: Critical]
Does this calculation severely under-calculate or return 0 for the number of
required SG entries on 32-bit architectures?

On 32-bit architectures, MAX_SG_ENT_SZ expands to 0xFFFFF000. For lengths
greater than 4096, DIV_ROUND_UP() expands to (len + 0xFFFFF000 - 1) / 0xFFFFF000,
which will wrap around the 32-bit addition and yield an artificially low or zero
number of entries.

[Severity: High]
This is a pre-existing issue, but does the error handling path following this
allocation unmap uninitialized scatterlist entries if dma_map_phys() fails?

When dma_map_phys() fails, the err_unmap_dma path in dma_buf_phys_vec_to_sgt()
attempts cleanup:

drivers/dma-buf/dma-buf-mapping.c:dma_buf_phys_vec_to_sgt() {
    ...
err_unmap_dma:
    ...
	} else {
		for_each_sgtable_dma_sg(&dma->sgt, sgl, i)
			dma_unmap_phys(attach->dev, sg_dma_address(sgl),
				       sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
	}
    ...
}

The for_each_sgtable_dma_sg macro internally shadows the outer failure index i
and initializes its own counter to 0. It then unconditionally iterates through
all sgt->nents allocated entries. Because the failure occurred early, trailing
entries remain zero-initialized, causing dma_unmap_phys() to be invoked on
addresses of 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901170849.4052816-1-dhu@x6u.co?part=2

Re: [PATCH v8 2/2] dma-buf: Split sgl by largest page-aligned chunk

[Jason Gunthorpe]

On Tue, Sep 01, 2026 at 05:08:49PM +0000, David Hu wrote:
> From: David Hu <xuehaohu@google.com>
> 
> Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`.
> This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the
> first entry, resulting in non-page-aligned DMA addresses for all
> subsequent entries.

This patch is fine, but pedenatically a scatterlist's entry limit
should be bounded to dma_get_max_seg_size(), though I don't think it
helps this. Operating scatterlists at the size limits has proven
problematic in a number of places already...

> In addition, a non-page-aligned sgl length will trigger an edge case
> in `ib_umem_find_best_pgsz()`. In case of a discontinuity in later
> buffers, we will have a `va` with lowest bit set to 1. That will lead
> to `ib_umem_find_best_pgsz()` always return 0, and break the promise
> to find best page size for the mapping on the NIC side.

That's an IB side bug, the newer logic that joins adjacent SGLs should
have avoided it?

Jason

Re: [PATCH v8 2/2] dma-buf: Split sgl by largest page-aligned chunk

[David Hu]

On Wed, Sep 2, 2026 at 8:08 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Tue, Sep 01, 2026 at 05:08:49PM +0000, David Hu wrote:
> > From: David Hu <xuehaohu@google.com>
> >
> > Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`.
> > This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the
> > first entry, resulting in non-page-aligned DMA addresses for all
> > subsequent entries.
>
> This patch is fine, but pedenatically a scatterlist's entry limit
> should be bounded to dma_get_max_seg_size(), though I don't think it
> helps this. Operating scatterlists at the size limits has proven
> problematic in a number of places already...
>
> > In addition, a non-page-aligned sgl length will trigger an edge case
> > in `ib_umem_find_best_pgsz()`. In case of a discontinuity in later
> > buffers, we will have a `va` with lowest bit set to 1. That will lead
> > to `ib_umem_find_best_pgsz()` always return 0, and break the promise
> > to find best page size for the mapping on the NIC side.
>
> That's an IB side bug, the newer logic that joins adjacent SGLs should
> have avoided it?

Hi Jason,

Thank you for the review. I think you are right on both counts.
Regarding the IB side, the new SGL joining logic in
`ib_umem_find_best_pgsz()` indeed avoids the issue. `mask |= va` is
skipped for artifically split, contiguous SGLs. Since Christian has
already pulled v8 into drm-misc-next, I won't spin a v9 to avoid
creating unnecessary noise on the list. If you prefer a followup,
please let me know.

Thanks again for catching the IB logic detail!

Regards,
David

Re: [PATCH v8 2/2] dma-buf: Split sgl by largest page-aligned chunk

[Jason Gunthorpe]

On Wed, Sep 02, 2026 at 07:07:50PM -0400, David Hu wrote:
> On Wed, Sep 2, 2026 at 8:08 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Tue, Sep 01, 2026 at 05:08:49PM +0000, David Hu wrote:
> > > From: David Hu <xuehaohu@google.com>
> > >
> > > Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`.
> > > This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the
> > > first entry, resulting in non-page-aligned DMA addresses for all
> > > subsequent entries.
> >
> > This patch is fine, but pedenatically a scatterlist's entry limit
> > should be bounded to dma_get_max_seg_size(), though I don't think it
> > helps this. Operating scatterlists at the size limits has proven
> > problematic in a number of places already...
> >
> > > In addition, a non-page-aligned sgl length will trigger an edge case
> > > in `ib_umem_find_best_pgsz()`. In case of a discontinuity in later
> > > buffers, we will have a `va` with lowest bit set to 1. That will lead
> > > to `ib_umem_find_best_pgsz()` always return 0, and break the promise
> > > to find best page size for the mapping on the NIC side.
> >
> > That's an IB side bug, the newer logic that joins adjacent SGLs should
> > have avoided it?
> 
> Hi Jason,
> 
> Thank you for the review. I think you are right on both counts.
> Regarding the IB side, the new SGL joining logic in
> `ib_umem_find_best_pgsz()` indeed avoids the issue. `mask |= va` is
> skipped for artifically split, contiguous SGLs. Since Christian has
> already pulled v8 into drm-misc-next, I won't spin a v9 to avoid
> creating unnecessary noise on the list. If you prefer a followup,
> please let me know.

Nope, I'm fine, it just explains why it wasn't seen in other
tested. You were backporting and mix&matched things. It confirms the
upstream kernel was fine from the start.

There are other importers besides RDMA, so I still think that this is
a good change regardless. Having each segment remain page aligned, and
staying away from ULONG_MAX that might trigger overflows is a friendly
and robust thing to do for less sophisticated importers.

Jason

Re: [PATCH v8 2/2] dma-buf: Split sgl by largest page-aligned chunk

[Leon Romanovsky]

On Thu, Sep 03, 2026 at 10:49:38AM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 02, 2026 at 07:07:50PM -0400, David Hu wrote:
> > On Wed, Sep 2, 2026 at 8:08 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Tue, Sep 01, 2026 at 05:08:49PM +0000, David Hu wrote:
> > > > From: David Hu <xuehaohu@google.com>
> > > >
> > > > Currently, `fill_sg_entry()` splits the scatterlist using `UINT_MAX`.
> > > > This creates a non-page-aligned DMA length (`0xFFFFFFFF`) for the
> > > > first entry, resulting in non-page-aligned DMA addresses for all
> > > > subsequent entries.
> > >
> > > This patch is fine, but pedenatically a scatterlist's entry limit
> > > should be bounded to dma_get_max_seg_size(), though I don't think it
> > > helps this. Operating scatterlists at the size limits has proven
> > > problematic in a number of places already...
> > >
> > > > In addition, a non-page-aligned sgl length will trigger an edge case
> > > > in `ib_umem_find_best_pgsz()`. In case of a discontinuity in later
> > > > buffers, we will have a `va` with lowest bit set to 1. That will lead
> > > > to `ib_umem_find_best_pgsz()` always return 0, and break the promise
> > > > to find best page size for the mapping on the NIC side.
> > >
> > > That's an IB side bug, the newer logic that joins adjacent SGLs should
> > > have avoided it?
> > 
> > Hi Jason,
> > 
> > Thank you for the review. I think you are right on both counts.
> > Regarding the IB side, the new SGL joining logic in
> > `ib_umem_find_best_pgsz()` indeed avoids the issue. `mask |= va` is
> > skipped for artifically split, contiguous SGLs. Since Christian has
> > already pulled v8 into drm-misc-next, I won't spin a v9 to avoid
> > creating unnecessary noise on the list. If you prefer a followup,
> > please let me know.
> 
> Nope, I'm fine, it just explains why it wasn't seen in other
> tested. You were backporting and mix&matched things. It confirms the
> upstream kernel was fine from the start.

All that time, I wondered why our testing didn't uncover any issues like
this, given that the requirement to support large BARs was raised almost
immediately during testing.

Thanks

> 
> There are other importers besides RDMA, so I still think that this is
> a good change regardless. Having each segment remain page aligned, and
> staying away from ULONG_MAX that might trigger overflows is a friendly
> and robust thing to do for less sophisticated importers.
> 
> Jason
> 

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Christian König]

On 9/1/26 19:08, David Hu wrote:
> From: David Hu <xuehaohu@google.com>
> 
> This series address two related issues in scatter-gather mapping,
> specifically for the MMIO based dma-buf mapping. The fixes ensure
> sgt mapping is correct, and proper for large MMIO regions.
> 
> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> phys vec to sgt)
> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> 
> Patch 2 Splits sgl by largest page aligned chunk
> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/

*sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.

I'm going to push those fixes to drm-misc-next now, but when there are more issues like that will mark the code as abandoned and not maintained.

Regards,
Christian.

> 
> Changes in v8:
>  - Combined the two patches into one unified series to avoid merge
>    conflicts.
>  - Collected Reviewed-by tag from Leon Romanovsky for Patch 2.
> 
> David Hu (2):
>   dma-buf: Fix silent overflow for phys vec to sgt
>   dma-buf: Split sgl by largest page-aligned chunk
> 
>  drivers/dma-buf/dma-buf-mapping.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> --
> 2.55.0.897.gb25b4bd76c-goog
> 

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Leon Romanovsky]

On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
> On 9/1/26 19:08, David Hu wrote:
> > From: David Hu <xuehaohu@google.com>
> > 
> > This series address two related issues in scatter-gather mapping,
> > specifically for the MMIO based dma-buf mapping. The fixes ensure
> > sgt mapping is correct, and proper for large MMIO regions.
> > 
> > Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> > (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> > phys vec to sgt)
> > https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> > 
> > Patch 2 Splits sgl by largest page aligned chunk
> > (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> > https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
> 
> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.

And this is why so many in the kernel community want to get rid of SG
lists. It would be great if DMA-BUF could also eliminate the need to
convert to an SGL, like Jason proposed.

The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
which is the one that depends on it.

Thanks

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Christian König]

On 9/2/26 09:39, Leon Romanovsky wrote:
> On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
>> On 9/1/26 19:08, David Hu wrote:
>>> From: David Hu <xuehaohu@google.com>
>>>
>>> This series address two related issues in scatter-gather mapping,
>>> specifically for the MMIO based dma-buf mapping. The fixes ensure
>>> sgt mapping is correct, and proper for large MMIO regions.
>>>
>>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
>>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
>>> phys vec to sgt)
>>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
>>>
>>> Patch 2 Splits sgl by largest page aligned chunk
>>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
>>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
>>
>> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
> 
> And this is why so many in the kernel community want to get rid of SG
> lists. It would be great if DMA-BUF could also eliminate the need to
> convert to an SGL, like Jason proposed.
> 
> The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
> which is the one that depends on it.

I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.

Regards,
Christian.

> 
> Thanks

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Leon Romanovsky]

On Wed, Sep 02, 2026 at 09:56:06AM +0200, Christian König wrote:
> On 9/2/26 09:39, Leon Romanovsky wrote:
> > On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
> >> On 9/1/26 19:08, David Hu wrote:
> >>> From: David Hu <xuehaohu@google.com>
> >>>
> >>> This series address two related issues in scatter-gather mapping,
> >>> specifically for the MMIO based dma-buf mapping. The fixes ensure
> >>> sgt mapping is correct, and proper for large MMIO regions.
> >>>
> >>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> >>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> >>> phys vec to sgt)
> >>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> >>>
> >>> Patch 2 Splits sgl by largest page aligned chunk
> >>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> >>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
> >>
> >> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
> > 
> > And this is why so many in the kernel community want to get rid of SG
> > lists. It would be great if DMA-BUF could also eliminate the need to
> > convert to an SGL, like Jason proposed.
> > 
> > The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
> > which is the one that depends on it.
> 
> I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.

You are proposing the same thing as an SGL, just in a different format.
It does not address the issue that dma_addr_t is expected to hold a DMA
address, while that is not always the case. For example, in the P2P case,
the addresses are not DMA addresses.

Jason's proposal:
https://lore.kernel.org/all/0-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com/

Thanks

> 
> Regards,
> Christian.
> 
> > 
> > Thanks
> 
> 

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Christian König]

On 9/2/26 10:32, Leon Romanovsky wrote:
> On Wed, Sep 02, 2026 at 09:56:06AM +0200, Christian König wrote:
>> On 9/2/26 09:39, Leon Romanovsky wrote:
>>> On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
>>>> On 9/1/26 19:08, David Hu wrote:
>>>>> From: David Hu <xuehaohu@google.com>
>>>>>
>>>>> This series address two related issues in scatter-gather mapping,
>>>>> specifically for the MMIO based dma-buf mapping. The fixes ensure
>>>>> sgt mapping is correct, and proper for large MMIO regions.
>>>>>
>>>>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
>>>>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
>>>>> phys vec to sgt)
>>>>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
>>>>>
>>>>> Patch 2 Splits sgl by largest page aligned chunk
>>>>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
>>>>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
>>>>
>>>> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
>>>
>>> And this is why so many in the kernel community want to get rid of SG
>>> lists. It would be great if DMA-BUF could also eliminate the need to
>>> convert to an SGL, like Jason proposed.
>>>
>>> The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
>>> which is the one that depends on it.
>>
>> I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.
> 
> You are proposing the same thing as an SGL, just in a different format.

Yes, because that is the right thing todo as far as I can see.

> It does not address the issue that dma_addr_t is expected to hold a DMA
> address, while that is not always the case. For example, in the P2P case,
> the addresses are not DMA addresses.

Yes they are. They must be DMA addresses because that is the only thing the importer needs to do it's DMA.

It can be that those are DMA addresses on private interconnects between devices, but it should *never* be a phys_addr_t because that is limited to the address space the CPU can see.

> Jason's proposal:
> https://lore.kernel.org/all/0-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com/

Yeah, I have commented quite a bit on that.

Regards,
Christian.

> 
> Thanks
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> Thanks
>>
>>

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Leon Romanovsky]

On Wed, Sep 02, 2026 at 10:44:59AM +0200, Christian König wrote:
> On 9/2/26 10:32, Leon Romanovsky wrote:
> > On Wed, Sep 02, 2026 at 09:56:06AM +0200, Christian König wrote:
> >> On 9/2/26 09:39, Leon Romanovsky wrote:
> >>> On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
> >>>> On 9/1/26 19:08, David Hu wrote:
> >>>>> From: David Hu <xuehaohu@google.com>
> >>>>>
> >>>>> This series address two related issues in scatter-gather mapping,
> >>>>> specifically for the MMIO based dma-buf mapping. The fixes ensure
> >>>>> sgt mapping is correct, and proper for large MMIO regions.
> >>>>>
> >>>>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> >>>>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> >>>>> phys vec to sgt)
> >>>>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> >>>>>
> >>>>> Patch 2 Splits sgl by largest page aligned chunk
> >>>>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> >>>>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
> >>>>
> >>>> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
> >>>
> >>> And this is why so many in the kernel community want to get rid of SG
> >>> lists. It would be great if DMA-BUF could also eliminate the need to
> >>> convert to an SGL, like Jason proposed.
> >>>
> >>> The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
> >>> which is the one that depends on it.
> >>
> >> I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.
> > 
> > You are proposing the same thing as an SGL, just in a different format.
> 
> Yes, because that is the right thing todo as far as I can see.
> 
> > It does not address the issue that dma_addr_t is expected to hold a DMA
> > address, while that is not always the case. For example, in the P2P case,
> > the addresses are not DMA addresses.
> 
> Yes they are. They must be DMA addresses because that is the only thing the importer needs to do it's DMA.

They can perform DMA, but that still does not make them suitable for the
dma_addr_t type. For the PCI_P2PDMA_MAP_BUS_ADDR flow, these addresses
follow completely different rules: they are not unmapped, require no cache
synchronization, are valid only for peer access, and require separate error
handling.

All of this information is lost if only the dma_addr_t is stored.

> 
> It can be that those are DMA addresses on private interconnects between devices, but it should *never* be a phys_addr_t because that is limited to the address space the CPU can see.
> 
> > Jason's proposal:
> > https://lore.kernel.org/all/0-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com/
> 
> Yeah, I have commented quite a bit on that.

Right, I posted it for reference.

Thanks

> 
> Regards,
> Christian.
> 
> > 
> > Thanks
> > 
> >>
> >> Regards,
> >> Christian.
> >>
> >>>
> >>> Thanks
> >>
> >>
> 
> 

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Christian König]

On 9/2/26 11:53, Leon Romanovsky wrote:
> On Wed, Sep 02, 2026 at 10:44:59AM +0200, Christian König wrote:
>> On 9/2/26 10:32, Leon Romanovsky wrote:
>>> On Wed, Sep 02, 2026 at 09:56:06AM +0200, Christian König wrote:
>>>> On 9/2/26 09:39, Leon Romanovsky wrote:
>>>>> On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
>>>>>> On 9/1/26 19:08, David Hu wrote:
>>>>>>> From: David Hu <xuehaohu@google.com>
>>>>>>>
>>>>>>> This series address two related issues in scatter-gather mapping,
>>>>>>> specifically for the MMIO based dma-buf mapping. The fixes ensure
>>>>>>> sgt mapping is correct, and proper for large MMIO regions.
>>>>>>>
>>>>>>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
>>>>>>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
>>>>>>> phys vec to sgt)
>>>>>>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
>>>>>>>
>>>>>>> Patch 2 Splits sgl by largest page aligned chunk
>>>>>>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
>>>>>>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
>>>>>>
>>>>>> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
>>>>>
>>>>> And this is why so many in the kernel community want to get rid of SG
>>>>> lists. It would be great if DMA-BUF could also eliminate the need to
>>>>> convert to an SGL, like Jason proposed.
>>>>>
>>>>> The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
>>>>> which is the one that depends on it.
>>>>
>>>> I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.
>>>
>>> You are proposing the same thing as an SGL, just in a different format.
>>
>> Yes, because that is the right thing todo as far as I can see.
>>
>>> It does not address the issue that dma_addr_t is expected to hold a DMA
>>> address, while that is not always the case. For example, in the P2P case,
>>> the addresses are not DMA addresses.
>>
>> Yes they are. They must be DMA addresses because that is the only thing the importer needs to do it's DMA.
> 
> They can perform DMA, but that still does not make them suitable for the
> dma_addr_t type. For the PCI_P2PDMA_MAP_BUS_ADDR flow, these addresses
> follow completely different rules: they are not unmapped, require no cache
> synchronization, are valid only for peer access, and require separate error
> handling.

The PCI_P2PDMA_MAP_BUS_ADDR is not supported by DMA-buf and as far as I can see is a complete dead end.

> All of this information is lost if only the dma_addr_t is stored.

Yes and that is fully intentional.

DMA-buf handles that cleanly on the buffer object level and not like PCI_P2PDMA_MAP_BUS_ADDR as a completely broken design on a per address/page basis.

Technical background is that the PCI_P2PDMA_MAP_BUS_ADDR approach can only be handled by a very very small subset of HW.

Regards,
Christian.

> 
>>
>> It can be that those are DMA addresses on private interconnects between devices, but it should *never* be a phys_addr_t because that is limited to the address space the CPU can see.
>>
>>> Jason's proposal:
>>> https://lore.kernel.org/all/0-v1-b5cab63049c0+191af-dmabuf_map_type_jgg@nvidia.com/
>>
>> Yeah, I have commented quite a bit on that.
> 
> Right, I posted it for reference.
> 
> Thanks
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> Thanks
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Thanks
>>>>
>>>>
>>
>>

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Leon Romanovsky]

On Wed, Sep 02, 2026 at 12:00:25PM +0200, Christian König wrote:
> On 9/2/26 11:53, Leon Romanovsky wrote:
> > On Wed, Sep 02, 2026 at 10:44:59AM +0200, Christian König wrote:
> >> On 9/2/26 10:32, Leon Romanovsky wrote:
> >>> On Wed, Sep 02, 2026 at 09:56:06AM +0200, Christian König wrote:
> >>>> On 9/2/26 09:39, Leon Romanovsky wrote:
> >>>>> On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
> >>>>>> On 9/1/26 19:08, David Hu wrote:
> >>>>>>> From: David Hu <xuehaohu@google.com>
> >>>>>>>
> >>>>>>> This series address two related issues in scatter-gather mapping,
> >>>>>>> specifically for the MMIO based dma-buf mapping. The fixes ensure
> >>>>>>> sgt mapping is correct, and proper for large MMIO regions.
> >>>>>>>
> >>>>>>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> >>>>>>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> >>>>>>> phys vec to sgt)
> >>>>>>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> >>>>>>>
> >>>>>>> Patch 2 Splits sgl by largest page aligned chunk
> >>>>>>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> >>>>>>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
> >>>>>>
> >>>>>> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
> >>>>>
> >>>>> And this is why so many in the kernel community want to get rid of SG
> >>>>> lists. It would be great if DMA-BUF could also eliminate the need to
> >>>>> convert to an SGL, like Jason proposed.
> >>>>>
> >>>>> The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
> >>>>> which is the one that depends on it.
> >>>>
> >>>> I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.
> >>>
> >>> You are proposing the same thing as an SGL, just in a different format.
> >>
> >> Yes, because that is the right thing todo as far as I can see.
> >>
> >>> It does not address the issue that dma_addr_t is expected to hold a DMA
> >>> address, while that is not always the case. For example, in the P2P case,
> >>> the addresses are not DMA addresses.
> >>
> >> Yes they are. They must be DMA addresses because that is the only thing the importer needs to do it's DMA.
> > 
> > They can perform DMA, but that still does not make them suitable for the
> > dma_addr_t type. For the PCI_P2PDMA_MAP_BUS_ADDR flow, these addresses
> > follow completely different rules: they are not unmapped, require no cache
> > synchronization, are valid only for peer access, and require separate error
> > handling.
> 
> The PCI_P2PDMA_MAP_BUS_ADDR is not supported by DMA-buf and as far as I can see is a complete dead end.

Maybe you mean DRM, but VFIO works perfectly with PCI_P2PDMA_MAP_BUS_ADDR flow in DMA-buf.
What am I missing?

Thanks

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Christian König]

On 9/2/26 12:59, Leon Romanovsky wrote:
...
>> The PCI_P2PDMA_MAP_BUS_ADDR is not supported by DMA-buf and as far as I can see is a complete dead end.
> 
> Maybe you mean DRM, but VFIO works perfectly with PCI_P2PDMA_MAP_BUS_ADDR flow in DMA-buf.
> What am I missing?

Well not quite.

The approach PCI_P2PDMA_MAP_BUS_ADDR takes is fundamentally tied to only map a physical CPU address into a PCI address and doesn't take into account that in a lot of configurations the CPU can't access all resources.

So when VFIO uses PCI_P2PDMA_MAP_BUS_ADDR it only works for a small subset of the use cases DMA-buf supports, but yeah as always in DMA-buf pretty much everything is optional.

Regards,
Christian.

> 
> Thanks

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Jason Gunthorpe]

On Wed, Sep 02, 2026 at 03:34:18PM +0200, Christian König wrote:
> On 9/2/26 12:59, Leon Romanovsky wrote:
> ...
> >> The PCI_P2PDMA_MAP_BUS_ADDR is not supported by DMA-buf and as
> >> far as I can see is a complete dead end.
> > 
> > Maybe you mean DRM, but VFIO works perfectly with
> > PCI_P2PDMA_MAP_BUS_ADDR flow in DMA-buf.  What am I missing?
> 
> Well not quite.
> 
> The approach PCI_P2PDMA_MAP_BUS_ADDR takes is fundamentally tied to
> only map a physical CPU address into a PCI address and doesn't take
> into account that in a lot of configurations the CPU can't access
> all resources.

It is called PCI P2P. If the flow only uses PCI devices it should
always work because it relies on PCI spec things that happen outside
the CPU.

It does not try to solve the general embedded problem of arbitary
communication between blocks inside a SOC, scale up networks, and so
on.

So, yes, it does not solve every problem DMA buf faces, but no it is
not a "Dead end" as it 100% solves PCI to PCI communication which is
still a very common use case.

DMA buf users that are exporting from a PCI device, over PCI MMIO
should be using this API. It is the only way to do PCI to PCI
communication correctly in the kernel.

Jason

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Pranjal Shrivastava]

On Wed, Sep 02, 2026 at 12:00:25PM +0200, Christian König wrote:
> On 9/2/26 11:53, Leon Romanovsky wrote:
> > On Wed, Sep 02, 2026 at 10:44:59AM +0200, Christian König wrote:
> >> On 9/2/26 10:32, Leon Romanovsky wrote:
> >>> On Wed, Sep 02, 2026 at 09:56:06AM +0200, Christian König wrote:
> >>>> On 9/2/26 09:39, Leon Romanovsky wrote:
> >>>>> On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
> >>>>>> On 9/1/26 19:08, David Hu wrote:
> >>>>>>> From: David Hu <xuehaohu@google.com>
> >>>>>>>
> >>>>>>> This series address two related issues in scatter-gather mapping,
> >>>>>>> specifically for the MMIO based dma-buf mapping. The fixes ensure
> >>>>>>> sgt mapping is correct, and proper for large MMIO regions.
> >>>>>>>
> >>>>>>> Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> >>>>>>> (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> >>>>>>> phys vec to sgt)
> >>>>>>> https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> >>>>>>>
> >>>>>>> Patch 2 Splits sgl by largest page aligned chunk
> >>>>>>> (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> >>>>>>> https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
> >>>>>>
> >>>>>> *sigh* such issues are exactly the reason why I didn't wanted the dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.
> >>>>>
> >>>>> And this is why so many in the kernel community want to get rid of SG
> >>>>> lists. It would be great if DMA-BUF could also eliminate the need to
> >>>>> convert to an SGL, like Jason proposed.
> >>>>>
> >>>>> The DMA layer no longer needs SGL. These bugs belong to the DMA-BUF layer,
> >>>>> which is the one that depends on it.
> >>>>
> >>>> I'm all fine using an array/xarray of dma_addr_t in DMA-buf, just phys_vec is a clear no-go.
> >>>
> >>> You are proposing the same thing as an SGL, just in a different format.
> >>
> >> Yes, because that is the right thing todo as far as I can see.
> >>
> >>> It does not address the issue that dma_addr_t is expected to hold a DMA
> >>> address, while that is not always the case. For example, in the P2P case,
> >>> the addresses are not DMA addresses.
> >>
> >> Yes they are. They must be DMA addresses because that is the only thing the importer needs to do it's DMA.
> > 
> > They can perform DMA, but that still does not make them suitable for the
> > dma_addr_t type. For the PCI_P2PDMA_MAP_BUS_ADDR flow, these addresses
> > follow completely different rules: they are not unmapped, require no cache
> > synchronization, are valid only for peer access, and require separate error
> > handling.
> 
> The PCI_P2PDMA_MAP_BUS_ADDR is not supported by DMA-buf and as far as I can see is a complete dead end.
> 
> > All of this information is lost if only the dma_addr_t is stored.
> 
> Yes and that is fully intentional.
> 
> DMA-buf handles that cleanly on the buffer object level and not like PCI_P2PDMA_MAP_BUS_ADDR as a completely broken design on a per address/page basis.
> 
> Technical background is that the PCI_P2PDMA_MAP_BUS_ADDR approach can only be handled by a very very small subset of HW.
> 

With the rise of accelerators, the P2PDMA_MAP_BUS_ADDR is going to be 
increasingly more common where accelerators directly transfer data to
NICs, storage devices, other accelerators etc. 

With VFIO gaining a DMABUF exporter the use has already spread to RDMA &
NVMe devices (w/ SPDK). In fact, we found these bugs while trying to map
large BAR regions for RDMA.

Thus, it would be great if we could find alignment here. 
AFAICT, I foresee the use of dmabufs to only increase for
PCI_P2PDMA_MAP_BUS_ADDR. IIRC when the network stack moved to net_iovs
to support dmabufs, the SGL became a primary concern and partly the 
reason why we have "unreadable" skbs for memory we can indeed access
if mapped correctly.

Thanks,
Praan

Re: [PATCH v8 0/2] dma-buf: Fix silent overflow and alignment

[Jason Gunthorpe]

On Wed, Sep 02, 2026 at 09:00:46AM +0200, Christian König wrote:
> On 9/1/26 19:08, David Hu wrote:
> > From: David Hu <xuehaohu@google.com>
> > 
> > This series address two related issues in scatter-gather mapping,
> > specifically for the MMIO based dma-buf mapping. The fixes ensure
> > sgt mapping is correct, and proper for large MMIO regions.
> > 
> > Patch 1 fixes a silent integer overflow for mapping length exceeding 4G
> > (Previously submitted as [PATCH v7] dma-buf: Fix silent overflow for
> > phys vec to sgt)
> > https://lore.kernel.org/all/20260609164047.486227-1-xuehaohu@google.com/
> > 
> > Patch 2 Splits sgl by largest page aligned chunk
> > (Previously submitted as [PATCH v3] dma-buf: Split sgl by largest page-aligned chunk)
> > https://lore.kernel.org/all/20260722233806.3922093-1-dhu@x6u.co/
> 
> *sigh* such issues are exactly the reason why I didn't wanted the
> *dma-mapping stuff inside DMA-buf. That clearly doesn't belong here.

So you'd rather have these tricky bugs in every driver using dmabuf?

The scatterlist construction in most of the DRM drivers is not in good
shape. They should be converting to call these APIs so they can do it
properly.

> I'm going to push those fixes to drm-misc-next now, but when there
> are more issues like that will mark the code as abandoned and not
> maintained.

Code that is getting bugs fix is not abandoned, please be reasonable.

Jason