Re: [PATCH v3] mm/hugetlb: preserve mremap address delta when skipping page tables

[David Hildenbrand (Arm)]

On 9/14/26 15:23, Jaewook You wrote:
> move_hugetlb_page_tables() optimizes mremap() by advancing to the last
> entry in the page table when the source page table does not exist, either
> initially or after unsharing a PMD table.  The common loop increment then
> steps to the first entry in the next page table.
> 
> However, the code advances both the source and destination addresses to
> the last entries in their respective page tables, which is wrong.  The
> destination address must be advanced only by the same amount as the source
> address.
> 
> If the source and destination offsets within their page tables differ, the
> destination address can be advanced too far, causing follow-up issues.  Fix
> this by advancing the destination address by the source advance distance.
> 
> With a reproducer, we were able to trigger a kernel panic on x86-64.  With
> this fix in place, we can no longer reproduce the issue.
> 
> Fixes: e95a9851787b ("hugetlb: skip to end of PT page mapping when pte not present")
> Fixes: 4ddb4d91b82f ("hugetlb: do not update address in huge_pmd_unshare")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Jaewook You <jaewook376@xxxxxxxxx>
> ---
> Changes in v3:
> - Clarify that the optimization advances to the last entry in the current
>   page table before the common loop increment steps to the next entry.
> - Rename remaining_size to offset_to_last_entry as suggested by David.
> 
> v2: https://lore.kernel.org/20260911182408.75821-1-jaewook376@xxxxxxxxx/
> 

v2 does not exist publicly ;)

This would have been better send as a v1 upstream (I mentioned this in private
but you might have missed it).

>  mm/hugetlb.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 4f6f58bf3db6c..5749f6270fb1f 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5161,18 +5161,21 @@ int move_hugetlb_page_tables(struct vm_area_struct *vma,
>  	hugetlb_vma_lock_write(vma);
>  	i_mmap_lock_write(mapping);
>  	for (; old_addr < old_end; old_addr += sz, new_addr += sz) {
> +		const unsigned long offset_to_last_entry =
> +			(old_addr | last_addr_mask) - old_addr;
> +
>  		src_pte = hugetlb_walk(vma, old_addr, sz);
>  		if (!src_pte) {
> -			old_addr |= last_addr_mask;
> -			new_addr |= last_addr_mask;
> +			old_addr += offset_to_last_entry;
> +			new_addr += offset_to_last_entry;
>  			continue;
>  		}
>  		if (huge_pte_none(huge_ptep_get(mm, old_addr, src_pte)))
>  			continue;
>  
>  		if (huge_pmd_unshare(&tlb, vma, old_addr, src_pte)) {
> -			old_addr |= last_addr_mask;
> -			new_addr |= last_addr_mask;
> +			old_addr += offset_to_last_entry;
> +			new_addr += offset_to_last_entry;
>  			continue;

As Oscar expressed, he might prefer some comment about the situation. But maybe
that can be deferred to some proper cleanups here.

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David