[PATCH v2 0/1] mm/memory: constrain generic_access_phys() to page boundary

[PATCH v2 0/1] mm/memory: constrain generic_access_phys() to page boundary

[Ren Wei]

From: Luxiao Xu <rakukuip@gmail.com>

Hi all,

This is v2 of the fix for generic_access_phys() out-of-bounds mapping.

In v1, we attempted to handle multi-page ranges by adding an internal
chunk loop inside generic_access_phys(). Following feedback from David
Hildenbrand, we realized that caller (__access_remote_vm) already has an
outer loop and handles normal pages chunk-by-chunk.

Instead of duplicating the loop logic in generic_access_phys(), v2
clamps the access length passed to ->access() in __access_remote_vm() to
not cross the page boundary (PAGE_SIZE - offset_in_page(addr)). Because
all VMAs are page-aligned, this also naturally ensures accesses never
cross VMA boundaries. Each page access is then driven by the outer loop,
looking up the VMA and resolving PFN/protections per page.

Additionally, we fixed the missing (resource_size_t) cast in PFN
comparison as pointed out by Andrew Morton.

Thanks,
Luxiao / Ren Wei

Luxiao Xu (1):
  mm/memory: constrain generic_access_phys() to page boundary

 mm/memory.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v2 1/1] mm/memory: constrain generic_access_phys() to page boundary

[Ren Wei]

From: Luxiao Xu <rakukuip@gmail.com>

generic_access_phys() improperly validates the memory access range: it
only validates the start address using follow_pfnmap_start() and passes
PAGE_ALIGN(len + offset) to ioremap_prot().

This poses two problems:
1. In PFNMAP VMAs, consecutive virtual pages are not guaranteed to be
   physically contiguous, and individual PTEs may have different access
   permissions or writability.
2. The mapping may cross VMA boundaries if len extends beyond vma->vm_end.

Since __access_remote_vm() already loops over the requested length and
accesses normal (struct page) memory page-by-page, constrain the ->access()
callback in __access_remote_vm() to at most the current page boundary.
Because VMA boundaries are always page-aligned, this also ensures the
access never exceeds the current VMA.

In generic_access_phys(), defensively clamp len to the page boundary as
well, map only a single PAGE_SIZE via ioremap_prot(), and add a missing
(resource_size_t) cast during PFN re-validation to avoid truncation on
32-bit PAE systems.

Fixes: 9cb12d7b4cca ("mm/memory.c: actually remap enough memory")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Suggested-by: David Hildenbrand <david@kernel.org>
Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
Signed-off-by: Ren Wei <weir@nebusec.ai>
---
v1 -> v2:
- Drop internal multi-page loop in generic_access_phys(); clamp the chunk
  size in __access_remote_vm() to PAGE_SIZE - offset instead, leveraging
  the existing caller loop (David Hildenbrand).
- Defensively clamp len in generic_access_phys() and only map PAGE_SIZE.
- Add missing (resource_size_t) cast when checking args.pfn (Andrew Morton).
Link: https://lore.kernel.org/r/f618e2f57ad0086a8d7a95cb17dcf5a1f0cf0e45.1784428532.git.rakukuip@gmail.com
---
 mm/memory.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 8b0c2c735d3d..09390e51432f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -7164,6 +7164,11 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	bool writable;
 	struct follow_pfnmap_args args = { .vma = vma, .address = addr };
 
+	if (len <= 0)
+		return 0;
+
+	len = min_t(int, len, PAGE_SIZE - offset);
+
 retry:
 	if (follow_pfnmap_start(&args))
 		return -EINVAL;
@@ -7175,7 +7180,7 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	if ((write & FOLL_WRITE) && !writable)
 		return -EINVAL;
 
-	maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
+	maddr = ioremap_prot(phys_addr, PAGE_SIZE, prot);
 	if (!maddr)
 		return -ENOMEM;
 
@@ -7183,7 +7188,7 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 		goto out_unmap;
 
 	if ((pgprot_val(prot) != pgprot_val(args.pgprot)) ||
-	    (phys_addr != (args.pfn << PAGE_SHIFT)) ||
+	    (phys_addr != ((resource_size_t)args.pfn << PAGE_SHIFT)) ||
 	    (writable != args.writable)) {
 		follow_pfnmap_end(&args);
 		iounmap(maddr);
@@ -7254,7 +7259,8 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
 #ifdef CONFIG_HAVE_IOREMAP_PROT
 			if (vma->vm_ops && vma->vm_ops->access)
 				bytes = vma->vm_ops->access(vma, addr, buf,
-							    len, write);
+							    min_t(int, len, PAGE_SIZE - offset_in_page(addr)),
+							    write);
 #endif
 			if (bytes <= 0)
 				break;
-- 
2.43.0

Re: [PATCH v2 1/1] mm/memory: constrain generic_access_phys() to page boundary

[Andrew Morton]

On Sat,  5 Sep 2026 22:12:17 +0800 Ren Wei <weir@nebusec.ai> wrote:

> generic_access_phys() improperly validates the memory access range: it
> only validates the start address using follow_pfnmap_start() and passes
> PAGE_ALIGN(len + offset) to ioremap_prot().
> 
> This poses two problems:
> 1. In PFNMAP VMAs, consecutive virtual pages are not guaranteed to be
>    physically contiguous, and individual PTEs may have different access
>    permissions or writability.
> 2. The mapping may cross VMA boundaries if len extends beyond vma->vm_end.
> 
> Since __access_remote_vm() already loops over the requested length and
> accesses normal (struct page) memory page-by-page, constrain the ->access()
> callback in __access_remote_vm() to at most the current page boundary.
> Because VMA boundaries are always page-aligned, this also ensures the
> access never exceeds the current VMA.
> 
> In generic_access_phys(), defensively clamp len to the page boundary as
> well, map only a single PAGE_SIZE via ioremap_prot(), and add a missing
> (resource_size_t) cast during PFN re-validation to avoid truncation on
> 32-bit PAE systems.

Thanks.

> Fixes: 9cb12d7b4cca ("mm/memory.c: actually remap enough memory")
> Cc: stable@vger.kernel.org

Why are we proposing a backport?  To do that we should provide
recipients with a clear description of the userspace-visible runtime
affects of this bug and I'm not seeing that in the changelog?

Re: [PATCH v2 1/1] mm/memory: constrain generic_access_phys() to page boundary

[David Hildenbrand (Arm)]

On 9/5/26 16:12, Ren Wei wrote:
> From: Luxiao Xu <rakukuip@gmail.com>
> 
> generic_access_phys() improperly validates the memory access range: it
> only validates the start address using follow_pfnmap_start() and passes
> PAGE_ALIGN(len + offset) to ioremap_prot().
> 
> This poses two problems:
> 1. In PFNMAP VMAs, consecutive virtual pages are not guaranteed to be
>    physically contiguous, and individual PTEs may have different access
>    permissions or writability.
> 2. The mapping may cross VMA boundaries if len extends beyond vma->vm_end.
> 
> Since __access_remote_vm() already loops over the requested length and
> accesses normal (struct page) memory page-by-page, constrain the ->access()
> callback in __access_remote_vm() to at most the current page boundary.
> Because VMA boundaries are always page-aligned, this also ensures the
> access never exceeds the current VMA.
> 
> In generic_access_phys(), defensively clamp len to the page boundary as
> well, map only a single PAGE_SIZE via ioremap_prot(), and add a missing
> (resource_size_t) cast during PFN re-validation to avoid truncation on
> 32-bit PAE systems.
> 
> Fixes: 9cb12d7b4cca ("mm/memory.c: actually remap enough memory")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: LLM
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
> Signed-off-by: Ren Wei <weir@nebusec.ai>
> ---
> v1 -> v2:
> - Drop internal multi-page loop in generic_access_phys(); clamp the chunk
>   size in __access_remote_vm() to PAGE_SIZE - offset instead, leveraging
>   the existing caller loop (David Hildenbrand).
> - Defensively clamp len in generic_access_phys() and only map PAGE_SIZE.
> - Add missing (resource_size_t) cast when checking args.pfn (Andrew Morton).
> Link: https://lore.kernel.org/r/f618e2f57ad0086a8d7a95cb17dcf5a1f0cf0e45.1784428532.git.rakukuip@gmail.com
> ---
>  mm/memory.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 8b0c2c735d3d..09390e51432f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -7164,6 +7164,11 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  	bool writable;
>  	struct follow_pfnmap_args args = { .vma = vma, .address = addr };
>  
> +	if (len <= 0)
> +		return 0;

Why is that required? Why would it be valid to call this function with a size of 0?

> +

We should add a comment here explaining the situation:

/*
 * Limit access to one page at a time, as that's what follow_pfnmap_start()
 * guarantees; expect the caller to retry to read larger ranges.
 */

> +	len = min_t(int, len, PAGE_SIZE - offset);

Do we really need the "min_t" here? (can't we use min?)

> +
>  retry:
>  	if (follow_pfnmap_start(&args))
>  		return -EINVAL;
> @@ -7175,7 +7180,7 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  	if ((write & FOLL_WRITE) && !writable)
>  		return -EINVAL;
>  
> -	maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
> +	maddr = ioremap_prot(phys_addr, PAGE_SIZE, prot);
>  	if (!maddr)
>  		return -ENOMEM;
>  
> @@ -7183,7 +7188,7 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  		goto out_unmap;
>  
>  	if ((pgprot_val(prot) != pgprot_val(args.pgprot)) ||
> -	    (phys_addr != (args.pfn << PAGE_SHIFT)) ||
> +	    (phys_addr != ((resource_size_t)args.pfn << PAGE_SHIFT)) ||
>  	    (writable != args.writable)) {
>  		follow_pfnmap_end(&args);
>  		iounmap(maddr);
> @@ -7254,7 +7259,8 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
>  #ifdef CONFIG_HAVE_IOREMAP_PROT
>  			if (vma->vm_ops && vma->vm_ops->access)
>  				bytes = vma->vm_ops->access(vma, addr, buf,
> -							    len, write);
> +							    min_t(int, len, PAGE_SIZE - offset_in_page(addr)),

Why isn't it sufficient to cap in generic_access_phys() ?


-- 
Cheers,

David