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

[Yuan Tan <yuant@nebusec.ai>]

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/