On 11.07.24 23:33, David Hildenbrand wrote:
On 11.07.24 07:13, Pei Li wrote:
This patch fixes this warning by acquiring read lock before entering
untrack_pfn() while write lock is not held.
syzbot has tested the proposed patch and the reproducer did not
trigger any issue.
Reported-by: syzbot+35a4414f6e247f515443@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=35a4414f6e247f515443
Tested-by: syzbot+35a4414f6e247f515443@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Pei Li <peili.dev@xxxxxxxxx>
---
Syzbot reported the following warning in follow_pte():
WARNING: CPU: 3 PID: 5192 at include/linux/rwsem.h:195 rwsem_assert_held include/linux/rwsem.h:195 [inline]
WARNING: CPU: 3 PID: 5192 at include/linux/rwsem.h:195 mmap_assert_locked include/linux/mmap_lock.h:65 [inline]
WARNING: CPU: 3 PID: 5192 at include/linux/rwsem.h:195 follow_pte+0x414/0x4c0 mm/memory.c:5980
This is because we are assuming that mm->mmap_lock should be held when
entering follow_pte(). This is added in commit c5541ba378e3 (mm:
follow_pte() improvements).
However, in the following call stack, we are not acquring the lock:
follow_phys arch/x86/mm/pat/memtype.c:957 [inline]
get_pat_info+0xf2/0x510 arch/x86/mm/pat/memtype.c:991
untrack_pfn+0xf7/0x4d0 arch/x86/mm/pat/memtype.c:1104
unmap_single_vma+0x1bd/0x2b0 mm/memory.c:1819
zap_page_range_single+0x326/0x560 mm/memory.c:1920
That implies that unmap_vmas() is called without the mmap lock in read
mode, correct?
Do we know how this happens?
* exit_mmap() holds the mmap lock in read mode
* unmap_region is documented to hold the mmap lock in read mode
I think this is it (missed the call from zap_page_range_single()):
follow_phys arch/x86/mm/pat/memtype.c:957 [inline]
get_pat_info+0xf2/0x510 arch/x86/mm/pat/memtype.c:991
untrack_pfn+0xf7/0x4d0 arch/x86/mm/pat/memtype.c:1104
unmap_single_vma+0x1bd/0x2b0 mm/memory.c:1819
zap_page_range_single+0x326/0x560 mm/memory.c:1920
unmap_mapping_range_vma mm/memory.c:3684 [inline]
unmap_mapping_range_tree mm/memory.c:3701 [inline]
unmap_mapping_pages mm/memory.c:3767 [inline]
unmap_mapping_range+0x1ee/0x280 mm/memory.c:3804
truncate_pagecache+0x53/0x90 mm/truncate.c:731
simple_setattr+0xf2/0x120 fs/libfs.c:886
notify_change+0xec6/0x11f0 fs/attr.c:499
do_truncate+0x15c/0x220 fs/open.c:65
handle_truncate fs/namei.c:3308 [inline]
I think Peter recently questioned whether untrack_pfn() should be even
called from the place, but I might misremember things.
Fix should work (I suspect we are not violating some locking rules?),
PFNMAP should not happen there too often that we really care.
If everything fails, we could drop the assert, but I'm hoping we can
avoid that. We really want most users of follow_pte() to do the right thing.
--
Cheers,
David / dhildenb
