All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] udmabuf: respect the device's maximum segment size
@ 2026-09-29  5:48 Karl Mehltretter
  2026-09-29  6:00 ` sashiko-bot
  2026-09-29 12:10 ` [PATCH v2] udmabuf: respect the device's maximum segment size Christian König
  0 siblings, 2 replies; 6+ messages in thread
From: Karl Mehltretter @ 2026-09-29  5:48 UTC (permalink / raw)
  To: Gerd Hoffmann, Vivek Kasireddy
  Cc: Karl Mehltretter, Sumit Semwal, Christian König,
	Jason Gunthorpe, dri-devel, linux-media, linaro-mm-sig,
	linux-kernel

get_sg_table() merges physically contiguous pages without accounting
for the mapping device's maximum segment size. This affects both
importer mappings and the udmabuf misc device mapping used for CPU
access.

With DMA_API_DEBUG enabled, DMA_BUF_IOCTL_SYNC on a 64 MiB udmabuf
reports:

  DMA-API: misc udmabuf: mapping sg segment longer than device claims to
  support [len=65884160] [max=65536]

Use sg_alloc_table_from_pages_segment() with the mapping device's
maximum segment size. Return -EINVAL if the limit is smaller than
PAGE_SIZE because the page-based allocator cannot honor it.

Before commit 5bf888673e0d ("udmabuf: Do not create malformed
scatterlists"), each entry covered one page.

Fixes: 5bf888673e0d ("udmabuf: Do not create malformed scatterlists")
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Notes:
    Changes in v2:
    - Return -EINVAL when the maximum segment size reported by the device is
      smaller than PAGE_SIZE instead of clamping it. (Christian)
    - Add Jason Gunthorpe's Reviewed-by tag.
    
    Tested on v7.3-rc4-70-gfe2ec83746e5 in QEMU (x86_64, TCG) with
    DMA_API_DEBUG (all_errors=1) and DMABUF_DEBUG, A/B against the same
    base:
    
                                            before        after
      DMA_BUF_IOCTL_SYNC, 64 MiB udmabuf    1 report      0
      vivid import, 4 MiB udmabuf           2 reports     0
      vivid import, 2 MiB hugetlb udmabuf   2 reports     0
        frames captured                     5/5           5/5
    
    vb2-dma-contig rejected the non-contiguous import in both runs.
    
    For v2, a focused importer advertising PAGE_SIZE / 2 returned
    -EINVAL under KASAN and DMA_API_DEBUG. No warning, BUG, or DMA-API
    report was emitted.

 drivers/dma-buf/udmabuf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
index df6dd00462423..018937435356e 100644
--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -133,15 +133,22 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
 {
 	struct udmabuf *ubuf = buf->priv;
 	struct sg_table *sg;
+	unsigned int max_segment;
 	int ret;
 
+	max_segment = dma_get_max_seg_size(dev);
+	/* The SG allocator requires a segment limit of at least PAGE_SIZE. */
+	if (max_segment < PAGE_SIZE)
+		return ERR_PTR(-EINVAL);
+
 	sg = kzalloc_obj(*sg);
 	if (!sg)
 		return ERR_PTR(-ENOMEM);
 
-	ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, 0,
-					ubuf->pagecount << PAGE_SHIFT,
-					GFP_KERNEL);
+	ret = sg_alloc_table_from_pages_segment(sg, ubuf->pages, ubuf->pagecount,
+						0, ubuf->pagecount << PAGE_SHIFT,
+						max_segment,
+						GFP_KERNEL);
 	if (ret < 0)
 		goto err_alloc;
 

base-commit: fe2ec83746e501645709761605c2464a44fd2929
-- 
2.53.0


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

* Re: [PATCH v2] udmabuf: respect the device's maximum segment size
  2026-09-29  5:48 [PATCH v2] udmabuf: respect the device's maximum segment size Karl Mehltretter
@ 2026-09-29  6:00 ` sashiko-bot
  2026-09-29  6:56   ` Karl Mehltretter
  2026-09-29 12:10 ` [PATCH v2] udmabuf: respect the device's maximum segment size Christian König
  1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-09-29  6:00 UTC (permalink / raw)
  To: Karl Mehltretter; +Cc: media-ci, dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dma-buf: udmabuf CPU mapping fails on arches with PAGE_SIZE > 64KB
--

commit 8fabdc29555a534a611bda125124d9a796d8f300
Author: Karl Mehltretter <kmehltretter@gmail.com>

udmabuf: respect the device's maximum segment size

The commit fixes how get_sg_table() allocates scatterlists by passing the
device's maximum segment size to the scatterlist allocator. It returns
-EINVAL if the limit is smaller than PAGE_SIZE.

> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index df6dd00462423..018937435356e 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -133,15 +133,22 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
>  {
>  	struct udmabuf *ubuf = buf->priv;
>  	struct sg_table *sg;
> +	unsigned int max_segment;
>  	int ret;
>  
> +	max_segment = dma_get_max_seg_size(dev);
> +	/* The SG allocator requires a segment limit of at least PAGE_SIZE. */
> +	if (max_segment < PAGE_SIZE)
> +		return ERR_PTR(-EINVAL);

[Severity: High]
Does this unintentionally cause CPU mappings to unconditionally fail on
architectures with a PAGE_SIZE larger than 64KB?

When dma_buf_begin_cpu_access() is called for a udmabuf, it invokes
get_sg_table() using udmabuf_misc.this_device.

Because this misc device is initialized without setting up dma_parms or
max_segment_size:

drivers/dma-buf/udmabuf.c:udmabuf_dev_init() {
    ...
	ret = dma_coerce_mask_and_coherent(udmabuf_misc.this_device,
					   DMA_BIT_MASK(64));
    ...
}

dma_get_max_seg_size() blindly returns the fallback default of SZ_64K.

On architectures with a 256KB PAGE_SIZE (such as Hexagon or PowerPC), this
new validation evaluates to 65536 < 262144, unconditionally returning
-EINVAL and breaking all CPU mappings.

> +
>  	sg = kzalloc_obj(*sg);
>  	if (!sg)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, 0,
> -					ubuf->pagecount << PAGE_SHIFT,
> -					GFP_KERNEL);
> +	ret = sg_alloc_table_from_pages_segment(sg, ubuf->pages, ubuf->pagecount,
> +						0, ubuf->pagecount << PAGE_SHIFT,
> +						max_segment,
> +						GFP_KERNEL);
>  	if (ret < 0)
>  		goto err_alloc;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929054835.94118-1-kmehltretter@gmail.com?part=1

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

* Re: [PATCH v2] udmabuf: respect the device's maximum segment size
  2026-09-29  6:00 ` sashiko-bot
@ 2026-09-29  6:56   ` Karl Mehltretter
  2026-09-29 12:03     ` Jason Gunthorpe
  0 siblings, 1 reply; 6+ messages in thread
From: Karl Mehltretter @ 2026-09-29  6:56 UTC (permalink / raw)
  To: sashiko-bot
  Cc: Karl Mehltretter, Gerd Hoffmann, Vivek Kasireddy, Sumit Semwal,
	Christian König, Jason Gunthorpe, dri-devel, linux-media,
	linaro-mm-sig, linux-kernel

On Tue, Sep 29, 2026 at 06:00:27AM +0000, sashiko-bot@kernel.org wrote:
> On architectures with a 256KB PAGE_SIZE (such as Hexagon or PowerPC), this
> new validation evaluates to 65536 < 262144, unconditionally returning
> -EINVAL and breaking all CPU mappings.

Right, the misc device has no dma_parms, so it gets the 64K default and
sync fails with 256K pages

The misc device has no real segment limit, so for v3 I'll give it
dma_parms with an unlimited max segment size and keep the -EINVAL for
importers that report a limit below PAGE_SIZE.

Karl

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

* Re: [PATCH v2] udmabuf: respect the device's maximum segment size
  2026-09-29  6:56   ` Karl Mehltretter
@ 2026-09-29 12:03     ` Jason Gunthorpe
  2026-09-29 19:53       ` [PATCH v2] udmabuf: respect the device's maximum segment sizeg Karl Mehltretter
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Gunthorpe @ 2026-09-29 12:03 UTC (permalink / raw)
  To: Karl Mehltretter
  Cc: sashiko-bot, Gerd Hoffmann, Vivek Kasireddy, Sumit Semwal,
	Christian König, dri-devel, linux-media, linaro-mm-sig,
	linux-kernel

On Tue, Sep 29, 2026 at 08:56:01AM +0200, Karl Mehltretter wrote:
> On Tue, Sep 29, 2026 at 06:00:27AM +0000, sashiko-bot@kernel.org wrote:
> > On architectures with a 256KB PAGE_SIZE (such as Hexagon or PowerPC), this
> > new validation evaluates to 65536 < 262144, unconditionally returning
> > -EINVAL and breaking all CPU mappings.
> 
> Right, the misc device has no dma_parms, so it gets the 64K default and
> sync fails with 256K pages

I would ignore this, it seems fictional to me, and if someone really
needs to support this rediculous combination it should be done inside
sg_alloc_table_from_pages_segment(:)
 
> The misc device has no real segment limit, so for v3 I'll give it
> dma_parms with an unlimited max segment size and keep the -EINVAL for
> importers that report a limit below PAGE_SIZE.

The params have to come from the *importing* device, not the udmabuf
misc device. You don't get to control what they are, and there is no
reason to put a dma_parms on a misc deivce.

Jason

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

* Re: [PATCH v2] udmabuf: respect the device's maximum segment size
  2026-09-29  5:48 [PATCH v2] udmabuf: respect the device's maximum segment size Karl Mehltretter
  2026-09-29  6:00 ` sashiko-bot
@ 2026-09-29 12:10 ` Christian König
  1 sibling, 0 replies; 6+ messages in thread
From: Christian König @ 2026-09-29 12:10 UTC (permalink / raw)
  To: Karl Mehltretter, Gerd Hoffmann, Vivek Kasireddy
  Cc: Sumit Semwal, Jason Gunthorpe, dri-devel, linux-media,
	linaro-mm-sig, linux-kernel

On 9/29/26 07:48, Karl Mehltretter wrote:
> get_sg_table() merges physically contiguous pages without accounting
> for the mapping device's maximum segment size. This affects both
> importer mappings and the udmabuf misc device mapping used for CPU
> access.
> 
> With DMA_API_DEBUG enabled, DMA_BUF_IOCTL_SYNC on a 64 MiB udmabuf
> reports:
> 
>   DMA-API: misc udmabuf: mapping sg segment longer than device claims to
>   support [len=65884160] [max=65536]
> 
> Use sg_alloc_table_from_pages_segment() with the mapping device's
> maximum segment size. Return -EINVAL if the limit is smaller than
> PAGE_SIZE because the page-based allocator cannot honor it.
> 
> Before commit 5bf888673e0d ("udmabuf: Do not create malformed
> scatterlists"), each entry covered one page.
> 
> Fixes: 5bf888673e0d ("udmabuf: Do not create malformed scatterlists")
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

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

> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> 
> Notes:
>     Changes in v2:
>     - Return -EINVAL when the maximum segment size reported by the device is
>       smaller than PAGE_SIZE instead of clamping it. (Christian)
>     - Add Jason Gunthorpe's Reviewed-by tag.
>     
>     Tested on v7.3-rc4-70-gfe2ec83746e5 in QEMU (x86_64, TCG) with
>     DMA_API_DEBUG (all_errors=1) and DMABUF_DEBUG, A/B against the same
>     base:
>     
>                                             before        after
>       DMA_BUF_IOCTL_SYNC, 64 MiB udmabuf    1 report      0
>       vivid import, 4 MiB udmabuf           2 reports     0
>       vivid import, 2 MiB hugetlb udmabuf   2 reports     0
>         frames captured                     5/5           5/5
>     
>     vb2-dma-contig rejected the non-contiguous import in both runs.
>     
>     For v2, a focused importer advertising PAGE_SIZE / 2 returned
>     -EINVAL under KASAN and DMA_API_DEBUG. No warning, BUG, or DMA-API
>     report was emitted.
> 
>  drivers/dma-buf/udmabuf.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index df6dd00462423..018937435356e 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -133,15 +133,22 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf,
>  {
>  	struct udmabuf *ubuf = buf->priv;
>  	struct sg_table *sg;
> +	unsigned int max_segment;
>  	int ret;
>  
> +	max_segment = dma_get_max_seg_size(dev);
> +	/* The SG allocator requires a segment limit of at least PAGE_SIZE. */
> +	if (max_segment < PAGE_SIZE)
> +		return ERR_PTR(-EINVAL);
> +
>  	sg = kzalloc_obj(*sg);
>  	if (!sg)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, 0,
> -					ubuf->pagecount << PAGE_SHIFT,
> -					GFP_KERNEL);
> +	ret = sg_alloc_table_from_pages_segment(sg, ubuf->pages, ubuf->pagecount,
> +						0, ubuf->pagecount << PAGE_SHIFT,
> +						max_segment,
> +						GFP_KERNEL);
>  	if (ret < 0)
>  		goto err_alloc;
>  
> 
> base-commit: fe2ec83746e501645709761605c2464a44fd2929


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

* Re: [PATCH v2] udmabuf: respect the device's maximum segment sizeg
  2026-09-29 12:03     ` Jason Gunthorpe
@ 2026-09-29 19:53       ` Karl Mehltretter
  0 siblings, 0 replies; 6+ messages in thread
From: Karl Mehltretter @ 2026-09-29 19:53 UTC (permalink / raw)
  To: Jason Gunthorpe, g
  Cc: sashiko-bot, Gerd Hoffmann, Vivek Kasireddy, Sumit Semwal,
	Christian König, dri-devel, linux-media, linaro-mm-sig,
	linux-kernel

On Tue, Sep 29, 2026 at 09:03:17AM +0100, Jason Gunthorpe wrote:
> On Tue, Sep 29, 2026 at 08:56:01AM +0200, Karl Mehltretter wrote:
> > Right, the misc device has no dma_parms, so it gets the 64K default and
> > sync fails with 256K pages
> 
> I would ignore this, it seems fictional to me, and if someone really
> needs to support this rediculous combination it should be done inside
> sg_alloc_table_from_pages_segment(:)
>  

Fair enough, Sashiko got me there. Looking around, 256K pages seem
to be barely supported today: they were already disabled for 47x, and
I couldn't get userspace running on a 256K-page 44x kernel in QEMU.

I'll leave v2 as is. Thanks for the review.

Karl

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

end of thread, other threads:[~2026-09-29 19:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-29  5:48 [PATCH v2] udmabuf: respect the device's maximum segment size Karl Mehltretter
2026-09-29  6:00 ` sashiko-bot
2026-09-29  6:56   ` Karl Mehltretter
2026-09-29 12:03     ` Jason Gunthorpe
2026-09-29 19:53       ` [PATCH v2] udmabuf: respect the device's maximum segment sizeg Karl Mehltretter
2026-09-29 12:10 ` [PATCH v2] udmabuf: respect the device's maximum segment size Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.