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

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

[Ren Wei]

From: Luxiao Xu <rakukuip@gmail.com>

Hi all,

This patch addresses an issue in generic_access_phys() where accessing memory
across page boundaries in PFNMAP VMAs assumes physical pages are contiguous,
which can lead to accessing unintended physical memory or exceeding VMA boundaries.

In v3, we address review feedback from David Hildenbrand on v2:
1. Revert changes to __access_remote_vm(): Capping the access length to
   (PAGE_SIZE - offset) inside generic_access_phys() is completely sufficient,
   as __access_remote_vm() already loops over the requested length and handles
   short transfers.
2. Drop the redundant 'len <= 0' check.
3. Add an explanatory comment clarifying that accesses are limited to one page
   at a time because follow_pfnmap_start() only validates a single PTE.
4. Regarding min() vs min_t(): min_t(int, ...) is retained because len is a
   signed int while PAGE_SIZE is unsigned long ((1UL) << PAGE_SHIFT), which
   triggers a compile-time "signedness error" if plain min() is used.

Thanks for the reviews and guidance.

v2 -> v3:
- Drop modification to __access_remote_vm(); capping in generic_access_phys()
  is sufficient because the caller loop already handles partial transfers
  (David Hildenbrand).
- Drop unnecessary 'len <= 0' check.
- Add comment explaining the single-page limitation (David Hildenbrand).
- v2 Link: https://lore.kernel.org/all/cover.1788531737.git.rakukuip@gmail.com/

v1 -> v2:
- Drop internal multi-page loop in generic_access_phys(); clamp the chunk
  size leveraging the existing caller loop (David Hildenbrand).
- Map only PAGE_SIZE in generic_access_phys().
- Add missing (resource_size_t) cast when checking args.pfn (Andrew Morton).

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

 mm/memory.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH v3 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.

Constrain the access in generic_access_phys() to at most the current page
boundary (PAGE_SIZE - offset) and map only a single PAGE_SIZE via
ioremap_prot(). Since the caller __access_remote_vm() already loops over
the requested length and handles partial transfers, it will naturally
iterate over the remaining pages.

Also 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>
---
v2 -> v3:
- Do not modify __access_remote_vm(); capping in generic_access_phys() is
  sufficient because the caller loop already handles partial transfers
  (David Hildenbrand).
- Drop unnecessary 'len <= 0' check.
- Add comment explaining the single-page limitation (David Hildenbrand).
- v2 Link: https://lore.kernel.org/all/cover.1788531737.git.rakukuip@gmail.com/

v1 -> v2:
- Drop internal multi-page loop in generic_access_phys(); clamp the chunk
  size leveraging the existing caller loop (David Hildenbrand).
- Map only PAGE_SIZE in generic_access_phys().
- Add missing (resource_size_t) cast when checking args.pfn (Andrew Morton).
---
 mm/memory.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..74fdf29c9c7e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6974,6 +6974,12 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 	bool writable;
 	struct follow_pfnmap_args args = { .vma = vma, .address = addr };
 
+	/*
+	 * 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);
+
 retry:
 	if (follow_pfnmap_start(&args))
 		return -EINVAL;
@@ -6985,7 +6991,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;
 
@@ -6993,7 +6999,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);
-- 
2.43.0

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

[Andrew Morton]

On Thu, 10 Sep 2026 11:42:50 +0800 Ren Wei <weir@nebusec.ai> 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.
> 
> Constrain the access in generic_access_phys() to at most the current page
> boundary (PAGE_SIZE - offset) and map only a single PAGE_SIZE via
> ioremap_prot(). Since the caller __access_remote_vm() already loops over
> the requested length and handles partial transfers, it will naturally
> iterate over the remaining pages.
> 
> Also add a missing (resource_size_t) cast during PFN re-validation to avoid
> truncation on 32-bit PAE systems.

Thanks.

When fixing a bug, please ensure that the changelog always clearly
describes the userspace-visible runtime effects of that bug.

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

Especially when proposing a backport.

The cover letter tells us a bit more:

  This patch addresses an issue in generic_access_phys() where
  accessing memory across page boundaries in PFNMAP VMAs assumes
  physical pages are contiguous, which can lead to accessing unintended
  physical memory or exceeding VMA boundaries.

But how does this manifest?  What does the user see?  Has it ever
happened?  Is there a Closes:?  Any reproducer?

> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: LLM

Please update your LLM prompts with my above sentence "When fixing...".
Let's get this fixed for the future.

> +	len = min_t(int, len, PAGE_SIZE - offset);
> -	    (phys_addr != (args.pfn << PAGE_SHIFT)) ||
> +	    (phys_addr != ((resource_size_t)args.pfn << PAGE_SHIFT)) ||

hoo boy we've made a mess of the types in there, but I don't see a
feasible improvement in the context of this patch.

Also, a [0/N] isn't needed or desirable when N==1!  I'll consolidate
both into a singleton patch.

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

[Yuan Tan]

On 9/9/26 23:18, Andrew Morton wrote:
> On Thu, 10 Sep 2026 11:42:50 +0800 Ren Wei <weir@nebusec.ai> 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.
>>
>> Constrain the access in generic_access_phys() to at most the current page
>> boundary (PAGE_SIZE - offset) and map only a single PAGE_SIZE via
>> ioremap_prot(). Since the caller __access_remote_vm() already loops over
>> the requested length and handles partial transfers, it will naturally
>> iterate over the remaining pages.
>>
>> Also add a missing (resource_size_t) cast during PFN re-validation to avoid
>> truncation on 32-bit PAE systems.
> Thanks.
>
> When fixing a bug, please ensure that the changelog always clearly
> describes the userspace-visible runtime effects of that bug.
>
>> Fixes: 9cb12d7b4cca ("mm/memory.c: actually remap enough memory")
>> Cc: stable@vger.kernel.org
> Especially when proposing a backport.
>
> The cover letter tells us a bit more:
>
>   This patch addresses an issue in generic_access_phys() where
>   accessing memory across page boundaries in PFNMAP VMAs assumes
>   physical pages are contiguous, which can lead to accessing unintended
>   physical memory or exceeding VMA boundaries.
>
> But how does this manifest?  What does the user see?  Has it ever
> happened?  Is there a Closes:?  Any reproducer?
>
>> Reported-by: Vega <vega@nebusec.ai>
>> Assisted-by: LLM
> Please update your LLM prompts with my above sentence "When fixing...".
> Let's get this fixed for the future.
>
>> +	len = min_t(int, len, PAGE_SIZE - offset);
>> -	    (phys_addr != (args.pfn << PAGE_SHIFT)) ||
>> +	    (phys_addr != ((resource_size_t)args.pfn << PAGE_SHIFT)) ||
> hoo boy we've made a mess of the types in there, but I don't see a
> feasible improvement in the context of this patch.
>
> Also, a [0/N] isn't needed or desirable when N==1!  I'll consolidate
> both into a singleton patch.

Hi,


Luxiao is helping us fix some of the bugs found by our bug finding tool.



Normally, our patch series includes a reproducer in the cover letter, so
we usually send these patches with a cover letter. I believe Luxiao
simply forgot to include the reproducer in this version. The reproducer
for this bug can be found here:
https://lore.kernel.org/all/cover.1784428532.git.rakukuip@gmail.com/



For future patches, if we are sending a single patch without a cover
letter, what would be the preferred way to include the reproducer?
Should we put it directly in the commit message, or place it below the
--- separator so that the reproducer itself does not become part of the
permanent git history?



Also, I have been collecting the bug report by llm into a syzbot-like
tracking system[1].
The tracker aggregates bug reports from multiple sources, including
Sashiko, and then attempts to validate the reports and generate
reproducers to make sure it is not a false positive . It also
automatically tracks whether a bug has been fixed.



For the networking side, I have already imported the Sashiko reports
into the bug tracker and shared it with the net maintainers. I am
planning to do the same for mm, but haven't gotten to it yet. There are
already a few mm bugs in the tracker that were found by our own tool,
and none of them seem to have security implications.

If you have a chance, I'd be interested to hear what you think. Any
suggestions on how to make it more useful for mm maintainers would be
very welcome.


Thanks,

Yuan



[1]
https://lore.kernel.org/all/20260830115546.3942129-1-yuantan098@gmail.com/

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

[Andrew Morton]

On Sun, 13 Sep 2026 01:28:18 -0700 Yuan Tan <yuant@nebusec.ai> wrote:

> 
> >> +	    (phys_addr != ((resource_size_t)args.pfn << PAGE_SHIFT)) ||
> > hoo boy we've made a mess of the types in there, but I don't see a
> > feasible improvement in the context of this patch.
> >
> > Also, a [0/N] isn't needed or desirable when N==1!  I'll consolidate
> > both into a singleton patch.
> 
> Hi,
> 
> 
> Luxiao is helping us fix some of the bugs found by our bug finding tool.

> 
> 
> Normally, our patch series includes a reproducer in the cover letter, so
> we usually send these patches with a cover letter. I believe Luxiao
> simply forgot to include the reproducer in this version. The reproducer
> for this bug can be found here:
> https://lore.kernel.org/all/cover.1784428532.git.rakukuip@gmail.com/


Great, thanks.

See, what I'm always looking for in bug fixes is a solid description
of the significance/impact/risk which that bug has upon our users. 

This impact ranges between "Impossibly improbable thing which some AI
scan found but which nobody is ever going to hit" through to "this
crashes our entire fleet 12 times per day".

The range really is that broad and this information matters.  And
people care about it.  I look to authors to help us understand and
assess this impact.  So please do whatever you can (organization-wide)
to ensure that this info is made available to its potential audience.


> For future patches, if we are sending a single patch without a cover
> letter, what would be the preferred way to include the reproducer?
> Should we put it directly in the commit message, or place it below the
> --- separator so that the reproducer itself does not become part of the
> permanent git history?


It depends.

Formally, the reproducer should be made part of selftests/, so the bug
can never reappear.  I think that's excessive for this project and the
risk of reintroduction is so low that this isn't worthwhile.

Pasting it in to the formal changelog is reasonable.  Putting it below
the --- is probably better, as long as the permanent changelog mentions
its existence.  Then highly motivated people can follow the Link: and
find the reproducer.

The main value in the reproducer is knowing that it exists!  That this
bug can really be hit from userspace.

> 
> Also, I have been collecting the bug report by llm into a syzbot-like
> tracking system[1].
> The tracker aggregates bug reports from multiple sources, including
> Sashiko, and then attempts to validate the reports and generate
> reproducers to make sure it is not a false positive . It also
> automatically tracks whether a bug has been fixed.


That's a great initiative, thanks.

> 
> For the networking side, I have already imported the Sashiko reports
> into the bug tracker and shared it with the net maintainers. I am
> planning to do the same for mm, but haven't gotten to it yet. There are
> already a few mm bugs in the tracker that were found by our own tool,
> and none of them seem to have security implications.
> 
> If you have a chance, I'd be interested to hear what you think. Any
> suggestions on how to make it more useful for mm maintainers would be
> very welcome.

I saw.  I'd love to spend time with this but you know how it is.  We're
all overwhelmed by the bot invasion at present, and none more than me. 
I view your contributions as "parallelization of effort".  Keep the
fixes coming and I'll do my best to get them through our pipeline and
out to our users.

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

[David Hildenbrand (Arm)]

On 9/10/26 05:42, 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.
> 
> Constrain the access in generic_access_phys() to at most the current page
> boundary (PAGE_SIZE - offset) and map only a single PAGE_SIZE via
> ioremap_prot(). Since the caller __access_remote_vm() already loops over
> the requested length and handles partial transfers, it will naturally
> iterate over the remaining pages.
> 
> Also 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>
> ---

Yes, LGTM!

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David