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

[Pu Lehui <pulehui@huaweicloud.com>]

On 2025/5/28 17:03, David Hildenbrand wrote:
> On 27.05.25 15:38, Pu Lehui wrote:
>> Hi David,
>>
>> On 2025/5/27 2:46, David Hildenbrand wrote:
>>> On 26.05.25 17:48, Oleg Nesterov wrote:
>>>> Hi Lehui,
>>>>
>>>> As I said, I don't understand mm/, so can't comment, but...
>>>>
>>>> On 05/26, Pu Lehui wrote:
>>>>>
>>>>> 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;
>>>>> +
>>>>
>>>> I was thinking about
>>>>
>>>>      WARN_ON(!pte_none(*new_pte))
>>>>
>>>> at the start of the main loop.
>>>>
>>>> Obviously not to fix the problem, but rather to make it more explicit.
>>>
>>> Yeah, WARN_ON_ONCE().
>>>
>>> We really should fix the code to not install uprobes into the area we
>>> are moving.
>> Alright, so let's try this direction.
>>
>>>
>>> Likely, the correct fix will be to pass the range as well to
>>> uprobe_mmap(), and passing that range to build_probe_list().
>>
>> It will be great. But IIUC, the range we expand to is already included
>> when entering uprobe_mmap and also build_probe_list.
> 
> Right, you'd have to communicate that information through all layers 
> (expanded range).
> 
> As an alternative, maybe we can really call handle_vma_uprobe() after 
> moving the pages.

Hi David,

Not sure if this is possible, but I think it would be appropriate to not 
handle this uprobe_mmap at the source, and maybe we should make it clear 
that new_pte must be NULL when move_ptes, otherwise it should be an 
exception?

> 
> uprobe_write_opcode() should detect that the uprobe is already installed 
> (verify_opcode() will return 0) and just return.
>