Re: [RFC PATCH] mm/mmap: Fix uprobe anon page be overwritten when expanding vma during mremap

[Pu Lehui <pulehui@huaweicloud.com>]

On 2025/5/22 23:14, David Hildenbrand wrote:
> On 22.05.25 16:37, Pu Lehui wrote:
>>
>>
>> On 2025/5/21 18:25, David Hildenbrand wrote:
>>> On 21.05.25 11:25, Pu Lehui wrote:
>>>> From: Pu Lehui <pulehui@huawei.com>
>>>>
>>>> We encountered a BUG alert triggered by Syzkaller as follows:
>>>>      BUG: Bad rss-counter state mm:00000000b4a60fca type:MM_ANONPAGES
>>>> val:1
>>>>
>>>> And we can reproduce it with the following steps:
>>>> 1. register uprobe on file at zero offset
>>>> 2. mmap the file at zero offset:
>>>>      addr1 = mmap(NULL, 2 * 4096, PROT_NONE, MAP_PRIVATE, fd, 0);
>>>
>>> So, here we will install a uprobe.
>>>
>>>> 3. mremap part of vma1 to new vma2:
>>>>      addr2 = mremap(addr1, 4096, 2 * 4096, MREMAP_MAYMOVE);
>>>
>>> Okay, so we'll essentially move the uprobe as we mremap.
>>>
>>>
>>>> 4. mremap back to orig addr1:
>>>>      mremap(addr2, 4096, 4096, MREMAP_MAYMOVE | MREMAP_FIXED, addr1);
>>>
>>> And here, we would expect to move the uprobe again.
>>>
>>>>
>>>> In the step 3, the vma1 range [addr1, addr1 + 4096] will be remap to 
>>>> new
>>>> vma2 with range [addr2, addr2 + 8192], and remap uprobe anon page from
>>>> the vma1 to vma2, then unmap the vma1 range [addr1, addr1 + 4096].
>>>> In tht step 4, the vma2 range [addr2, addr2 + 4096] will be remap back
>>>> to the addr range [addr1, addr1 + 4096]. Since the addr range [addr1 +
>>>> 4096, addr1 + 8192] still maps the file, it will take
>>>> vma_merge_new_range to merge these two addr ranges, and then do
>>>> uprobe_mmap in vma_complete. Since the merged vma pgoff is also zero
>>>> offset, it will install uprobe anon page to the merged vma.
>>>
>>> Oh, so we're installing the uprobe into the extended VMA before moving
>>> the page tables.
>> Yep!
>>>
>>> Gah.
>>>
>>>> However, the
>>>> upcomming move_page_tables step, which use set_pte_at to remap the vma2
>>>> uprobe anon page to the merged vma, will over map the old uprobe anon
>>>> page in the merged vma, and lead the old uprobe anon page to be orphan.
>>>
>>> Right, when moving page tables we don't expect there to already be
>>> something from the uprobe code.
>>>
>>>>
>>>> Since the uprobe anon page will be remapped to the merged vma, we can
>>>> remove the unnecessary uprobe_mmap at merged vma, that is, do not
>>>> perform uprobe_mmap when there is no vma in the addr range to be
>>>> expaned.
>>>
>>> Hmmm, I'll have to think about other corner cases ....
>>>
>> looking forward to it
> 
> I think, the rule is that we must not install a uprobe for the range 
> that we will be actually moving the page tables for.
> 
> So, for the range we're effectively moving (not the one we're extending).
> 
> Because logically, the uprobe will be already handled by the existing 
> page tables that we're moving.
> 
> For the range we're extending, we must call uprobe handling code ...
> 
> 
> Alternatively, maybe we could call uprobe handling code after moving the 
> page tables. We'd probably find that the uprobe is already installed and 
> do nothing (so the theory :) ). ... if that would simplify anything.
> 

Hi David, Lorenzo, Oleg,

My apologies for the delay. Thanks for your reply.

To make things simpler, perhaps we could try post-processing, that is:

diff --git a/mm/mremap.c b/mm/mremap.c
index 83e359754961..46a757fd26dc 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -240,6 +240,11 @@ static int move_ptes(struct pagetable_move_control 
*pmc,
                 if (pte_none(ptep_get(old_pte)))
                         continue;

+               /* skip move pte when expanded range has uprobe */
+               if (unlikely(pte_present(*new_pte) &&
+                            vma_has_uprobes(pmc->new, new_addr, 
new_addr + PAGE_SIZE)))
+                       continue;
+
                 pte = ptep_get_and_clear(mm, old_addr, old_pte);
                 /*
                  * If we are remapping a valid PTE, make sure

What do you think?

Thanks,
Lehui