dma_common_find_pages() previously assumed that the CPU virtual address
always pointed to the start of a DMA-coherent allocation. This fails when
memory is allocated via dma_alloc_attrs() without DMA_ATTR_FORCE_CONTIGUOUS
and then subdivided into smaller blocks using a gen_pool, relevant only
when an IOMMU is enabled.

In such cases, userspace may request a mapping via dma_mmap_attrs()
for a CPU address that is offset inside the original allocation. The
previous code could return the wrong struct page pointer.

Example scenario:

  - Allocate a large DMA buffer with dma_alloc_attrs() (non-contiguous)
  - Create a gen_pool using this buffer
  - Take sub-allocations from the gen_pool
  - Map the sub-allocations to userspace via dma_mmap_attrs()
  - dma_common_find_pages() must return the correct struct page for the offset

This patch computes the page index relative to the vm_struct backing
the DMA allocation:

    page_index = (vaddr - area->addr) >> PAGE_SHIFT

Bounds checks ensure the CPU address is within the vm_struct and
page_index < area->nr_pages.

This ensures correct behavior for sub-allocated regions from gen_pool,
without affecting allocations starting at the base address or allocations
with DMA_ATTR_FORCE_CONTIGUOUS.

Signed-off-by: Andrei-Edward Popa <andrei.popa105@yahoo.com>
---
 kernel/dma/remap.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c
index b7c1c0c92d0c..d27477e32ed8 100644
--- a/kernel/dma/remap.c
+++ b/kernel/dma/remap.c
@@ -9,12 +9,25 @@
 struct page **dma_common_find_pages(void *cpu_addr)
 {
 	struct vm_struct *area = find_vm_area(cpu_addr);
+	unsigned long vaddr, area_vaddr;
+	size_t page_index;
 
 	if (!area || !(area->flags & VM_DMA_COHERENT))
 		return NULL;
 	WARN(area->flags != VM_DMA_COHERENT,
 	     "unexpected flags in area: %p\n", cpu_addr);
-	return area->pages;
+
+	vaddr = (unsigned long)cpu_addr;
+	area_vaddr = (unsigned long)area->addr;
+	if (unlikely(vaddr < area_vaddr ||
+		     vaddr >= area_vaddr + area->size))
+		return NULL;
+
+	page_index = (vaddr - area_vaddr) >> PAGE_SHIFT;
+	if (unlikely(page_index >= area->nr_pages))
+		return NULL;
+
+	return &area->pages[page_index];
 }
 
 /*
-- 
2.52.0

On 2025-12-12 8:09 pm, Andrei-Edward Popa wrote:
> dma_common_find_pages() previously assumed that the CPU virtual address
> always pointed to the start of a DMA-coherent allocation. This fails when
> memory is allocated via dma_alloc_attrs() without DMA_ATTR_FORCE_CONTIGUOUS
> and then subdivided into smaller blocks using a gen_pool, relevant only
> when an IOMMU is enabled.
> 
> In such cases, userspace may request a mapping via dma_mmap_attrs()
> for a CPU address that is offset inside the original allocation. The
> previous code could return the wrong struct page pointer.
> 
> Example scenario:
> 
>    - Allocate a large DMA buffer with dma_alloc_attrs() (non-contiguous)
>    - Create a gen_pool using this buffer
>    - Take sub-allocations from the gen_pool
>    - Map the sub-allocations to userspace via dma_mmap_attrs()

That's where the bug is. If a driver wants to subdivide an allocation 
then it needs to keep track of what it's done, and handle mmap properly 
to offset the VMA relative to the original allocation before passing the 
request through. See the kerneldoc for dma_mmap_attrs():

  * @cpu_addr: kernel CPU-view address returned from dma_alloc_attrs
  * @dma_addr: device-view address returned from dma_alloc_attrs
  * @size: size of memory originally requested in dma_alloc_attrs

"address returned from dma_alloc_attrs" does not mean "any arbitrary 
address within the bounds of the original address/size", it means 
exactly what it says, i.e. the original address.

Thanks
Robin.

>    - dma_common_find_pages() must return the correct struct page for the offset
> 
> This patch computes the page index relative to the vm_struct backing
> the DMA allocation:
> 
>      page_index = (vaddr - area->addr) >> PAGE_SHIFT
> 
> Bounds checks ensure the CPU address is within the vm_struct and
> page_index < area->nr_pages.
> 
> This ensures correct behavior for sub-allocated regions from gen_pool,
> without affecting allocations starting at the base address or allocations
> with DMA_ATTR_FORCE_CONTIGUOUS.
> 
> Signed-off-by: Andrei-Edward Popa <andrei.popa105@yahoo.com>
> ---
>   kernel/dma/remap.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c
> index b7c1c0c92d0c..d27477e32ed8 100644
> --- a/kernel/dma/remap.c
> +++ b/kernel/dma/remap.c
> @@ -9,12 +9,25 @@
>   struct page **dma_common_find_pages(void *cpu_addr)
>   {
>   	struct vm_struct *area = find_vm_area(cpu_addr);
> +	unsigned long vaddr, area_vaddr;
> +	size_t page_index;
>   
>   	if (!area || !(area->flags & VM_DMA_COHERENT))
>   		return NULL;
>   	WARN(area->flags != VM_DMA_COHERENT,
>   	     "unexpected flags in area: %p\n", cpu_addr);
> -	return area->pages;
> +
> +	vaddr = (unsigned long)cpu_addr;
> +	area_vaddr = (unsigned long)area->addr;
> +	if (unlikely(vaddr < area_vaddr ||
> +		     vaddr >= area_vaddr + area->size))
> +		return NULL;
> +
> +	page_index = (vaddr - area_vaddr) >> PAGE_SHIFT;
> +	if (unlikely(page_index >= area->nr_pages))
> +		return NULL;
> +
> +	return &area->pages[page_index];
>   }
>   
>   /*

On Fri, Dec 12, 2025 at 10:09:14PM +0200, Andrei-Edward Popa wrote:
> dma_common_find_pages() previously assumed that the CPU virtual address
> always pointed to the start of a DMA-coherent allocation. This fails when
> memory is allocated via dma_alloc_attrs() without DMA_ATTR_FORCE_CONTIGUOUS
> and then subdivided into smaller blocks using a gen_pool, relevant only
> when an IOMMU is enabled.
> 
> In such cases, userspace may request a mapping via dma_mmap_attrs()
> for a CPU address that is offset inside the original allocation. The
> previous code could return the wrong struct page pointer.

No, you can't mmap part of a dma coherent allocation.  What caller is
trying to do this?  It needs to be fixed instead.

On 15.12.2025 06:57, Christoph Hellwig wrote:
> On Fri, Dec 12, 2025 at 10:09:14PM +0200, Andrei-Edward Popa wrote:
>> dma_common_find_pages() previously assumed that the CPU virtual address
>> always pointed to the start of a DMA-coherent allocation. This fails when
>> memory is allocated via dma_alloc_attrs() without DMA_ATTR_FORCE_CONTIGUOUS
>> and then subdivided into smaller blocks using a gen_pool, relevant only
>> when an IOMMU is enabled.
>>
>> In such cases, userspace may request a mapping via dma_mmap_attrs()
>> for a CPU address that is offset inside the original allocation. The
>> previous code could return the wrong struct page pointer.
> No, you can't mmap part of a dma coherent allocation.  What caller is
> trying to do this?  It needs to be fixed instead.

I wonder if this was ever explicitly stated.

dma_mmap_coherent() was initially added for mmapeing a 
dma_alloc_coherent()-allocated buffer for fbdev and alsa, and at least 
the first one allowed to mmap the buffer partially or starting at 
non-zero offset. I doubt that this feature was useful for anything, but 
I'm quite sure this was at least allowed and there were some comments in 
the code about that.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

On 2025-12-15 8:13 am, Marek Szyprowski wrote:
> On 15.12.2025 06:57, Christoph Hellwig wrote:
>> On Fri, Dec 12, 2025 at 10:09:14PM +0200, Andrei-Edward Popa wrote:
>>> dma_common_find_pages() previously assumed that the CPU virtual address
>>> always pointed to the start of a DMA-coherent allocation. This fails when
>>> memory is allocated via dma_alloc_attrs() without DMA_ATTR_FORCE_CONTIGUOUS
>>> and then subdivided into smaller blocks using a gen_pool, relevant only
>>> when an IOMMU is enabled.
>>>
>>> In such cases, userspace may request a mapping via dma_mmap_attrs()
>>> for a CPU address that is offset inside the original allocation. The
>>> previous code could return the wrong struct page pointer.
>> No, you can't mmap part of a dma coherent allocation.  What caller is
>> trying to do this?  It needs to be fixed instead.
> 
> I wonder if this was ever explicitly stated.

Seems it never made it into the text documentation, but it is stated in 
the kerneldoc.

> dma_mmap_coherent() was initially added for mmapeing a
> dma_alloc_coherent()-allocated buffer for fbdev and alsa, and at least
> the first one allowed to mmap the buffer partially or starting at
> non-zero offset. I doubt that this feature was useful for anything, but
> I'm quite sure this was at least allowed and there were some comments in
> the code about that.

Mapping part of a buffer in general is fine, provided the fd advertised 
to userspace represents the entire buffer - then the offset/size of the 
VMA determine how much of it actually gets mapped. What you can't do is 
advertise parts of a buffer *as separate fds* and then expect to pass 
mmap calls on those straight through, without adjusting them to be 
relative to the whole buffer.

We don't provide such a generic helper for mmap'ing from a generic 
dma_pool, largely because the main point of those is for drivers that 
want to allocate lots of buffers smaller than PAGE_SIZE, such that 
trying to mmap them to userspace would most likely be a giant security 
hole anyway.

Thanks,
Robin.