Re: [PATCH] mm: fix COW mapping handing in generic_access_phys

[Jinjiang Tu <tujinjiang@huawei.com>]

在 2025/5/28 17:59, David Hildenbrand 写道:
> On 28.05.25 10:59, David Hildenbrand wrote:
>> On 28.05.25 03:56, Jinjiang Tu wrote:
>>> Syzkaller reports a below BUG:
>>>    ioremap on RAM at 0x0000000022727000 - 0x0000000022727fff
>>>    WARNING: CPU: 3 PID: 3609 at arch/x86/mm/ioremap.c:216 
>>> __ioremap_caller+0x644/0x7f0 arch/x86/mm/ioremap.c:216
>>>    Modules linked in:
>>>    CPU: 3 PID: 3609 Comm: syz.2.577 Not tainted 6.6.0+ #63
>>>    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
>>> rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
>>>    RIP: 0010:__ioremap_caller+0x644/0x7f0 arch/x86/mm/ioremap.c:216
>>>    Call Trace:
>>>     <TASK>
>>>     generic_access_phys+0x241/0x480 mm/memory.c:6458
>>>     __access_remote_vm+0x6af/0x970 mm/memory.c:6535
>>>     access_process_vm+0x53/0x80 mm/memory.c:6600
>>>     get_cmdline+0x192/0x380 mm/util.c:1041
>>>     audit_log_proctitle kernel/auditsc.c:1620 [inline]
>>>     audit_log_exit+0x1424/0x18c0 kernel/auditsc.c:1811
>>>     __audit_syscall_exit+0x252/0x2f0 kernel/auditsc.c:2079
>>>     audit_syscall_exit include/linux/audit.h:356 [inline]
>>>     syscall_exit_work+0x10f/0x130 kernel/entry/common.c:166
>>>     __syscall_exit_to_user_mode_work kernel/entry/common.c:205 [inline]
>>>     syscall_exit_to_user_mode+0x10/0x1e0 kernel/entry/common.c:218
>>>     do_syscall_64+0x66/0x110 arch/x86/entry/common.c:87
>>>     entry_SYSCALL_64_after_hwframe+0x78/0xe2
>>>
>>> The /dev/mem is mapped with COW mapping, and mremap at the 
>>> mm->args_start.
>>> The special pfn mapping is replaced by anon folios due to COW.
>>> generic_access_phys() is supposed to handle iomem, instead of RAM pfn,
>>> thus trigger a WARN_ON.
>>>
>>> Similar to commit 04c35ab3bdae ("x86/mm/pat: fix VM_PAT handling in
>>> COW mappings"). check if the pte is special to reject Cowed anon 
>>> folios.
>>>
>>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>>> ---
>>>    mm/memory.c | 7 +++++++
>>>    1 file changed, 7 insertions(+)
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 49199410805c..e1dac84536ee 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -6840,6 +6840,13 @@ int generic_access_phys(struct vm_area_struct 
>>> *vma, unsigned long addr,
>>>    retry:
>>>        if (follow_pfnmap_start(&args))
>>>            return -EINVAL;
>>> +
>>> +    /* Never return PFNs of anon folios in COW mappings. */
>>> +    if (!args.special) {
>>> +        follow_pfnmap_end(&args);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>>        prot = args.pgprot;
>>>        phys_addr = (resource_size_t)args.pfn << PAGE_SHIFT;
>>>        writable = args.writable;
>>
>> I assume we trigger this through vma->vm_ops->access, when the vm_ops 
>> have generic_access_phys set.
>>
>> I still dislike exposing the "special" bit here, as it is absolutely 
>> not what we should care about in the caller.
>>
>> In case our arch does not support pte_special, you fix will not catch 
>> that case ...
>>
>> The following might be better:
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 37d8738f5e12e..810adb8d1a53b 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -6681,6 +6681,14 @@ int generic_access_phys(struct vm_area_struct 
>> *vma, unsigned long addr,
>>           prot = args.pgprot;
>>           phys_addr = (resource_size_t)args.pfn << PAGE_SHIFT;
>>           writable = args.writable;
>> +
>> +       /* Refuse (refcounted) anonymous pages in CoW mappings. */
>> +       if (is_cow_mapping(vma->vm_flags) &&
>> +           vm_normal_page(vma, addr, ptep_get(args.ptep))) {
>> +               follow_pfnmap_end(&args);
>> +               return -EINVAL;
>> +       }
>> +
>
> Thinking again, we might have a PMD/PUD mapping, so maybe 
> follow_pfnmap_start() should really just refuse any refcounted pages.
Thanks for reviewing, I will investigate it.