[syzbot] [mm?] WARNING: suspicious RCU usage in mas_walk (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: a901a3568fd2 Merge tag 'iomap-6.5-merge-1' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=105632c8a80000
kernel config: https://syzkaller.appspot.com/x/.config?x=db74cd5aa6878807
dashboard link: https://syzkaller.appspot.com/bug?extid=8645fe63c4d22c8d27b8
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

Unfortunately, I don't have any reproducer for this issue yet.

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/3e7b15164da6/disk-a901a356.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/9880c1d81d68/vmlinux-a901a356.xz
kernel image: https://storage.googleapis.com/syzbot-assets/6d5b0c0d9670/bzImage-a901a356.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8645fe...@syzkaller.appspotmail.com

SELinux: policy capability cgroup_seclabel=1
SELinux: policy capability nnp_nosuid_transition=1
SELinux: policy capability genfs_seclabel_symlinks=0
SELinux: policy capability ioctl_skip_cloexec=0
=============================
WARNING: suspicious RCU usage
6.4.0-syzkaller-10173-ga901a3568fd2 #0 Not tainted
-----------------------------
lib/maple_tree.c:860 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
no locks held by init/1.

stack backtrace:
CPU: 1 PID: 1 Comm: init Not tainted 6.4.0-syzkaller-10173-ga901a3568fd2 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/27/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x136/0x150 lib/dump_stack.c:106
lockdep_rcu_suspicious+0x208/0x3a0 kernel/locking/lockdep.c:6719
mas_root lib/maple_tree.c:860 [inline]
mas_root lib/maple_tree.c:858 [inline]
mas_start lib/maple_tree.c:1402 [inline]
mas_start lib/maple_tree.c:1392 [inline]
mas_state_walk lib/maple_tree.c:3861 [inline]
mas_walk+0x4e8/0x7c0 lib/maple_tree.c:4980
mas_find_setup lib/maple_tree.c:5924 [inline]
mas_find+0x1cb/0x340 lib/maple_tree.c:5965
vma_next include/linux/mm.h:865 [inline]
validate_mm+0xd2/0x470 mm/mmap.c:301
do_vmi_align_munmap+0x1199/0x1680 mm/mmap.c:2561
do_vmi_munmap+0x266/0x430 mm/mmap.c:2619
__vm_munmap+0x137/0x380 mm/mmap.c:2899
__do_sys_munmap mm/mmap.c:2916 [inline]
__se_sys_munmap mm/mmap.c:2913 [inline]
__x64_sys_munmap+0x62/0x80 mm/mmap.c:2913
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f36cc8e9817
Code: ff ff 76 10 48 8b 15 10 36 0d 00 f7 d8 64 89 02 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 b8 0b 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e1 35 0d 00 f7 d8 64 89 01 48
RSP: 002b:00007fff12a54cf8 EFLAGS: 00000246 ORIG_RAX: 000000000000000b
RAX: ffffffffffffffda RBX: 000000000000001f RCX: 00007f36cc8e9817
RDX: 0000000000000000 RSI: 00000000000415ce RDI: 00007f36cc6b1000
RBP: 00005613cf216bf0 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
R13: 0000000000000000 R14: 00007f36cc6b1000 R15: 00007f36cc72a16d
</TASK>

=============================
WARNING: suspicious RCU usage
6.4.0-syzkaller-10173-ga901a3568fd2 #0 Not tainted
-----------------------------
lib/maple_tree.c:816 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
no locks held by init/1.

stack backtrace:
CPU: 0 PID: 1 Comm: init Not tainted 6.4.0-syzkaller-10173-ga901a3568fd2 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/27/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x136/0x150 lib/dump_stack.c:106
lockdep_rcu_suspicious+0x208/0x3a0 kernel/locking/lockdep.c:6719
mt_slot lib/maple_tree.c:816 [inline]
mt_slot lib/maple_tree.c:813 [inline]
mtree_range_walk+0x602/0x940 lib/maple_tree.c:2976
mas_state_walk lib/maple_tree.c:3868 [inline]
mas_walk+0x393/0x7c0 lib/maple_tree.c:4980
mas_find_setup lib/maple_tree.c:5924 [inline]
mas_find+0x1cb/0x340 lib/maple_tree.c:5965
vma_next include/linux/mm.h:865 [inline]
validate_mm+0xd2/0x470 mm/mmap.c:301
do_vmi_align_munmap+0x1199/0x1680 mm/mmap.c:2561
do_vmi_munmap+0x266/0x430 mm/mmap.c:2619
__vm_munmap+0x137/0x380 mm/mmap.c:2899
__do_sys_munmap mm/mmap.c:2916 [inline]
__se_sys_munmap mm/mmap.c:2913 [inline]
__x64_sys_munmap+0x62/0x80 mm/mmap.c:2913
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f36cc8e9817
Code: ff ff 76 10 48 8b 15 10 36 0d 00 f7 d8 64 89 02 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 b8 0b 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e1 35 0d 00 f7 d8 64 89 01 48
RSP: 002b:00007fff12a54cf8 EFLAGS: 00000246 ORIG_RAX: 000000000000000b
RAX: ffffffffffffffda RBX: 000000000000001f RCX: 00007f36cc8e9817
RDX: 0000000000000000 RSI: 00000000000415ce RDI: 00007f36cc6b1000
RBP: 00005613cf216bf0 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
R13: 0000000000000000 R14: 00007f36cc6b1000 R15: 00007f36cc72a16d
</TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want to change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[Liam R. Howlett]

This is a duplicate of what was reported on lkml and fixed by Linus [1].

[1.] https://lore.kernel.org/lkml/ZKIsoMOT71uwCIZX@xsang-OptiPlex-9020/

* syzbot <syzbot+8645fe...@syzkaller.appspotmail.com> [230705 23:22]:

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit: 1e25dd777248 Add linux-next specific files for 20230725
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=10ce2c31a80000
kernel config: https://syzkaller.appspot.com/x/.config?x=f481ab36ce878b84
dashboard link: https://syzkaller.appspot.com/bug?extid=8645fe63c4d22c8d27b8
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1697cec9a80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1566986ea80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/e0c7408382e8/disk-1e25dd77.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/c4b0f14b8c88/vmlinux-1e25dd77.xz
kernel image: https://storage.googleapis.com/syzbot-assets/b38cfdfd7db8/bzImage-1e25dd77.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8645fe...@syzkaller.appspotmail.com

=============================
WARNING: suspicious RCU usage

6.5.0-rc3-next-20230725-syzkaller #0 Not tainted
-----------------------------
lib/maple_tree.c:839 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1

1 lock held by syz-executor244/5034:
#0: ffff88802b3f0148 (&vma->vm_lock->lock){++++}-{3:3}, at: vma_start_read include/linux/mm.h:654 [inline]
#0: ffff88802b3f0148 (&vma->vm_lock->lock){++++}-{3:3}, at: lock_vma_under_rcu+0x201/0x950 mm/memory.c:5585

stack backtrace:
CPU: 0 PID: 5034 Comm: syz-executor244 Not tainted 6.5.0-rc3-next-20230725-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2023

Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]

dump_stack_lvl+0x125/0x1b0 lib/dump_stack.c:106
lockdep_rcu_suspicious+0x20c/0x3b0 kernel/locking/lockdep.c:6719
mas_root lib/maple_tree.c:839 [inline]
mas_root lib/maple_tree.c:837 [inline]
mas_start lib/maple_tree.c:1381 [inline]
mas_state_walk lib/maple_tree.c:3832 [inline]
mas_walk+0x55e/0x7d0 lib/maple_tree.c:4973
find_mergeable_anon_vma+0x102/0x890 mm/mmap.c:1085
__anon_vma_prepare+0x7d/0x550 mm/rmap.c:200
anon_vma_prepare include/linux/rmap.h:159 [inline]
wp_page_copy mm/memory.c:3202 [inline]
do_wp_page+0x1bbf/0x2e10 mm/memory.c:3579
handle_pte_fault mm/memory.c:5133 [inline]
__handle_mm_fault+0x1704/0x4030 mm/memory.c:5257
handle_mm_fault+0x47a/0xa00 mm/memory.c:5422
do_user_addr_fault+0x2e7/0xfe0 arch/x86/mm/fault.c:1342
handle_page_fault arch/x86/mm/fault.c:1483 [inline]
exc_page_fault+0x5c/0xd0 arch/x86/mm/fault.c:1539
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
RIP: 0033:0x7fbc6b3fd6e5
Code: 6a 00 41 b8 12 10 01 00 be 00 00 00 20 4c 8b 0d 21 9a 0a 00 bf 09 00 00 00 e8 37 2a 03 00 b9 00 04 00 20 ba 46 ae 20 40 31 c0 <c7> 04 25 00 04 00 20 00 00 00 00 be ff ff ff ff bf 10 00 00 00 c7
RSP: 002b:00007ffd68c881f0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 00007ffd68c883c8 RCX: 0000000020000400
RDX: 000000004020ae46 RSI: 0000000000400000 RDI: 0000000020000000
RBP: 00007fbc6b4a3610 R08: 0000000000000003 R09: 0000000000000000
R10: 0000000000011012 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffd68c883b8 R14: 0000000000000001 R15: 0000000000000001

</TASK>

=============================
WARNING: suspicious RCU usage

6.5.0-rc3-next-20230725-syzkaller #0 Not tainted
-----------------------------
lib/maple_tree.c:795 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1

1 lock held by syz-executor244/5034:
#0: ffff88802b3f0148 (&vma->vm_lock->lock){++++}-{3:3}, at: vma_start_read include/linux/mm.h:654 [inline]
#0: ffff88802b3f0148 (&vma->vm_lock->lock){++++}-{3:3}, at: lock_vma_under_rcu+0x201/0x950 mm/memory.c:5585

stack backtrace:
CPU: 0 PID: 5034 Comm: syz-executor244 Not tainted 6.5.0-rc3-next-20230725-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2023

Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]

dump_stack_lvl+0x125/0x1b0 lib/dump_stack.c:106
lockdep_rcu_suspicious+0x20c/0x3b0 kernel/locking/lockdep.c:6719
mt_slot lib/maple_tree.c:795 [inline]
mt_slot lib/maple_tree.c:792 [inline]
mtree_range_walk+0x6c5/0x9b0 lib/maple_tree.c:2946
mas_state_walk lib/maple_tree.c:3839 [inline]
mas_walk+0x389/0x7d0 lib/maple_tree.c:4973
find_mergeable_anon_vma+0x102/0x890 mm/mmap.c:1085
__anon_vma_prepare+0x7d/0x550 mm/rmap.c:200
anon_vma_prepare include/linux/rmap.h:159 [inline]
wp_page_copy mm/memory.c:3202 [inline]
do_wp_page+0x1bbf/0x2e10 mm/memory.c:3579
handle_pte_fault mm/memory.c:5133 [inline]
__handle_mm_fault+0x1704/0x4030 mm/memory.c:5257
handle_mm_fault+0x47a/0xa00 mm/memory.c:5422
do_user_addr_fault+0x2e7/0xfe0 arch/x86/mm/fault.c:1342
handle_page_fault arch/x86/mm/fault.c:1483 [inline]
exc_page_fault+0x5c/0xd0 arch/x86/mm/fault.c:1539
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
RIP: 0033:0x7fbc6b3fd6e5
Code: 6a 00 41 b8 12 10 01 00 be 00 00 00 20 4c 8b 0d 21 9a 0a 00 bf 09 00 00 00 e8 37 2a 03 00 b9 00 04 00 20 ba 46 ae 20 40 31 c0 <c7> 04 25 00 04 00 20 00 00 00 00 be ff ff ff ff bf 10 00 00 00 c7
RSP: 002b:00007ffd68c881f0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 00007ffd68c883c8 RCX: 0000000020000400
RDX: 000000004020ae46 RSI: 0000000000400000 RDI: 0000000020000000
RBP: 00007fbc6b4a3610 R08: 0000000000000003 R09: 0000000000000000
R10: 000000000001101


---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

[syzbot]

syzbot has bisected this issue to:

commit a52f58b34afe095ebc5823684eb264404dad6f7b
Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
Date: Mon Jul 24 18:54:10 2023 +0000

mm: handle faults that merely update the accessed bit under the VMA lock

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
start commit: [unknown]
git tree: linux-next
final oops: https://syzkaller.appspot.com/x/report.txt?x=1443585ea80000
console output: https://syzkaller.appspot.com/x/log.txt?x=1043585ea80000

kernel config: https://syzkaller.appspot.com/x/.config?x=f481ab36ce878b84
dashboard link: https://syzkaller.appspot.com/bug?extid=8645fe63c4d22c8d27b8

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1697cec9a80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1566986ea80000

Reported-by: syzbot+8645fe...@syzkaller.appspotmail.com
Fixes: a52f58b34afe ("mm: handle faults that merely update the accessed bit under the VMA lock")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

[Liam R. Howlett]

* syzbot <syzbot+8645fe...@syzkaller.appspotmail.com> [230726 02:57]:

This is caused by walking the maple tree without holding the mmap or rcu
read lock when per-vma locking is used for the page fault.

We could wrap the find_mergeable_anon_vma() walk with an rcu read lock,
but I am unsure if that's the correct way to handle this as the anon_vma
lock is taken later in __anon_vma_prepare(). Note that the anon_vma
lock is per-anon_vma, so we cannot just relocate that lock.

I'm wondering if we need find_mergeable_anon_vma() to take a read lock
on the VMA which contains the anon_vma to ensure it doesn't go away?
Maybe a find_and_lock_mergeable_anon_vma() and return a locked anon_vma?
Basically lock_vma_under_rcu(), anon_vma_lock_write(), vma_end_read().

Thoughts?

Thanks,
Liam

[Suren Baghdasaryan]

Hmm. lock_vma_under_rcu() specifically checks for vma->anon_vma==NULL
condition (see [1]) to avoid going into find_mergeable_anon_vma() (a
check inside anon_vma_prepare() should prevent that). So, it should
fall back to mmap_lock'ing.
Jann Horn is fixing an issue with this check in [2] which happens
before we take the vma lock. So, it's possible that this race is
causing a call to find_mergeable_anon_vma() while holding per-VMA
lock. Another possibility is that the recent addition of vma_is_tcp()
is messing things up here... Either way, find_mergeable_anon_vma()
should never be called under per-VMA locks because it relies on
neighboring VMAs to be stable and we do not lock those.

[1] https://elixir.bootlin.com/linux/v6.5-rc3/source/mm/memory.c#L5396
[2] https://lore.kernel.org/all/20230726214103....@google.com/

[Jann Horn]

This syzkaller report applies to a tree with Willy's in-progress patch
series, where lock_vma_under_rcu() only checks for vma->anon_vma if
vma_is_anonymous() is true - it permits private non-anonymous VMAs
(which require an anon_vma for handling write faults) even if they
don't have an anon_vma.

The commit bisected by syzkaller
(https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a52f58b34afe095ebc5823684eb264404dad6f7b)
removes the vma_is_anonymous() check in handle_pte_fault(), so it lets
us reach do_wp_page() with a non-anonymous private VMA without
anon_vma, even though that requires allocation of an anon_vma.

So I think this is pretty clearly an issue with Willy's in-progress
patch series that syzkaller blamed correctly.

[Liam R. Howlett]

* Jann Horn <ja...@google.com> [230727 14:00]:

Thanks for the analysis and quick response Jann and Suren.

I'll stop digging now.

Regards,
Liam

[Matthew Wilcox]

On Thu, Jul 27, 2023 at 07:59:33PM +0200, Jann Horn wrote:
> On Thu, Jul 27, 2023 at 7:22 PM Suren Baghdasaryan <sur...@google.com> wrote:
> > Hmm. lock_vma_under_rcu() specifically checks for vma->anon_vma==NULL
> > condition (see [1]) to avoid going into find_mergeable_anon_vma() (a
> > check inside anon_vma_prepare() should prevent that). So, it should
> > fall back to mmap_lock'ing.
>
> This syzkaller report applies to a tree with Willy's in-progress patch
> series, where lock_vma_under_rcu() only checks for vma->anon_vma if
> vma_is_anonymous() is true - it permits private non-anonymous VMAs
> (which require an anon_vma for handling write faults) even if they
> don't have an anon_vma.
>
> The commit bisected by syzkaller
> (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a52f58b34afe095ebc5823684eb264404dad6f7b)
> removes the vma_is_anonymous() check in handle_pte_fault(), so it lets
> us reach do_wp_page() with a non-anonymous private VMA without
> anon_vma, even though that requires allocation of an anon_vma.
>
> So I think this is pretty clearly an issue with Willy's in-progress
> patch series that syzkaller blamed correctly.

Agreed. What do we think the right solution is?

Option 1:

+++ b/mm/memory.c
@@ -3197,6 +3197,12 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
struct mmu_notifier_range range;
int ret;

+ if (!vma->anon_vma) {
+ // check if there are other things to undo here
+ vma_end_read(vmf->vma);
+ return VM_FAULT_RETRY;
+ }
+
delayacct_wpcopy_start();

Option 2:

@@ -5581,7 +5587,8 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
goto inval;

/* find_mergeable_anon_vma uses adjacent vmas which are not locked */
- if (vma_is_anonymous(vma) && !vma->anon_vma)
+ if ((vma_is_anonymous(vma) ||
+ vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) && !vma->anon_vma)
goto inval;

The problem with option 2 is that we don't know whether this is a write
fault or not, so we'll handle read faults on private file
mappings under the mmap_lock UNTIL somebody writes to the mapping, which
might be never. That seems like a bad idea.

We could pass FAULT_FLAG_WRITE into lock_vma_under_rcu(), but that also
seems like a bad idea. I dunno. Three bad ideas. Anyone think of a
good one?

[Suren Baghdasaryan]

On Thu, Jul 27, 2023 at 11:17 AM Matthew Wilcox <wi...@infradead.org> wrote:
>
> On Thu, Jul 27, 2023 at 07:59:33PM +0200, Jann Horn wrote:
> > On Thu, Jul 27, 2023 at 7:22 PM Suren Baghdasaryan <sur...@google.com> wrote:
> > > Hmm. lock_vma_under_rcu() specifically checks for vma->anon_vma==NULL
> > > condition (see [1]) to avoid going into find_mergeable_anon_vma() (a
> > > check inside anon_vma_prepare() should prevent that). So, it should
> > > fall back to mmap_lock'ing.
> >
> > This syzkaller report applies to a tree with Willy's in-progress patch
> > series, where lock_vma_under_rcu() only checks for vma->anon_vma if
> > vma_is_anonymous() is true - it permits private non-anonymous VMAs
> > (which require an anon_vma for handling write faults) even if they
> > don't have an anon_vma.
> >
> > The commit bisected by syzkaller
> > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a52f58b34afe095ebc5823684eb264404dad6f7b)
> > removes the vma_is_anonymous() check in handle_pte_fault(), so it lets
> > us reach do_wp_page() with a non-anonymous private VMA without
> > anon_vma, even though that requires allocation of an anon_vma.
> >
> > So I think this is pretty clearly an issue with Willy's in-progress
> > patch series that syzkaller blamed correctly.

A comment for __anon_vma_prepare() says "This must be called with the
mmap_lock held for reading."
I think adding an explicit mmap_assert_locked() in this function would
help catch such issues.

>
> Agreed. What do we think the right solution is?
>
> Option 1:
>
> +++ b/mm/memory.c
> @@ -3197,6 +3197,12 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
> struct mmu_notifier_range range;
> int ret;
>
> + if (!vma->anon_vma) {
> + // check if there are other things to undo here
> + vma_end_read(vmf->vma);
> + return VM_FAULT_RETRY;
> + }
> +

This one bails out later but if the path is not taken too often I
think it's cleaner.

[Matthew Wilcox]

On Thu, Jul 27, 2023 at 11:31:01AM -0700, Suren Baghdasaryan wrote:
> A comment for __anon_vma_prepare() says "This must be called with the
> mmap_lock held for reading."
> I think adding an explicit mmap_assert_locked() in this function would
> help catch such issues.

Would it catch them faster than syzbot?

> > +++ b/mm/memory.c
> > @@ -3197,6 +3197,12 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
> > struct mmu_notifier_range range;
> > int ret;
> >
> > + if (!vma->anon_vma) {
> > + // check if there are other things to undo here
> > + vma_end_read(vmf->vma);
> > + return VM_FAULT_RETRY;
> > + }
> > +
>
> This one bails out later but if the path is not taken too often I
> think it's cleaner.

OK, then I think the right place to put this check is slightly before
this function is called, to avoid bouncing the folio refcount:

+++ b/mm/memory.c
@@ -3567,6 +3567,12 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
return 0;
}
copy:
+ if ((vmf->flags & FAULT_FLAG_VMA_LOCK) && !vma->anon_vma) {
+ pte_unmap_unlock(vmf->pte, vmf->ptl);

+ vma_end_read(vmf->vma);
+ return VM_FAULT_RETRY;
+ }
+

/*
* Ok, we need to copy. Oh, well..
*/

... it compiles. Guess I should figure out the syntax to ask syzbot to
test it.

[syzbot]

> On Tue, Jul 25, 2023 at 11:57:22PM -0700, syzbot wrote:
>> syzbot has bisected this issue to:
>

> FWIW, this is unrelated to the previous issue. It's the caller of
> mas_walk() that has violated the locking constraints, and mas_walk() is
> simply reporting the issue. Is there a way to get syzbot to understand
> that it needs to unwind the call-stack further to decide who to blame?

>
>> commit a52f58b34afe095ebc5823684eb264404dad6f7b
>> Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
>> Date: Mon Jul 24 18:54:10 2023 +0000
>>
>> mm: handle faults that merely update the accessed bit under the VMA lock
>>
>> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
>> start commit: [unknown]
>> git tree: linux-next
>> final oops: https://syzkaller.appspot.com/x/report.txt?x=1443585ea80000
>> console output: https://syzkaller.appspot.com/x/log.txt?x=1043585ea80000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=f481ab36ce878b84
>> dashboard link: https://syzkaller.appspot.com/bug?extid=8645fe63c4d22c8d27b8
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1697cec9a80000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1566986ea80000
>

> #syz test

want 2 args (repo, branch), got 4

>
> diff --git a/mm/memory.c b/mm/memory.c
> index 20a2e9ed4aeb..57b271108bdc 100644
> --- a/mm/memory.c

[Matthew Wilcox]

On Tue, Jul 25, 2023 at 11:57:22PM -0700, syzbot wrote:

> syzbot has bisected this issue to:

FWIW, this is unrelated to the previous issue. It's the caller of
mas_walk() that has violated the locking constraints, and mas_walk() is
simply reporting the issue. Is there a way to get syzbot to understand
that it needs to unwind the call-stack further to decide who to blame?

> commit a52f58b34afe095ebc5823684eb264404dad6f7b
> Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
> Date: Mon Jul 24 18:54:10 2023 +0000
>
> mm: handle faults that merely update the accessed bit under the VMA lock
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
> start commit: [unknown]
> git tree: linux-next
> final oops: https://syzkaller.appspot.com/x/report.txt?x=1443585ea80000
> console output: https://syzkaller.appspot.com/x/log.txt?x=1043585ea80000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f481ab36ce878b84
> dashboard link: https://syzkaller.appspot.com/bug?extid=8645fe63c4d22c8d27b8
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1697cec9a80000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1566986ea80000

#syz test

[syzbot]

> On Tue, Jul 25, 2023 at 11:57:22PM -0700, syzbot wrote:
>> syzbot has bisected this issue to:
>>

>> commit a52f58b34afe095ebc5823684eb264404dad6f7b
>> Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
>> Date: Mon Jul 24 18:54:10 2023 +0000
>>
>> mm: handle faults that merely update the accessed bit under the VMA lock
>>
>> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
>> start commit: [unknown]
>> git tree: linux-next
>

> #syz test linux-next a52f58b34afe095ebc5823684eb264404dad6f7b

"linux-next" does not look like a valid git repo address.

[Matthew Wilcox]

On Tue, Jul 25, 2023 at 11:57:22PM -0700, syzbot wrote:

> syzbot has bisected this issue to:
>
> commit a52f58b34afe095ebc5823684eb264404dad6f7b
> Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
> Date: Mon Jul 24 18:54:10 2023 +0000
>
> mm: handle faults that merely update the accessed bit under the VMA lock
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
> start commit: [unknown]
> git tree: linux-next

#syz test linux-next a52f58b34afe095ebc5823684eb264404dad6f7b

[Matthew Wilcox]

On Thu, Jul 27, 2023 at 11:53:02AM -0700, syzbot wrote:
> > On Tue, Jul 25, 2023 at 11:57:22PM -0700, syzbot wrote:
> >> syzbot has bisected this issue to:
> >>
> >> commit a52f58b34afe095ebc5823684eb264404dad6f7b
> >> Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
> >> Date: Mon Jul 24 18:54:10 2023 +0000
> >>
> >> mm: handle faults that merely update the accessed bit under the VMA lock
> >>
> >> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
> >> start commit: [unknown]
> >> git tree: linux-next
> >
> > #syz test linux-next a52f58b34afe095ebc5823684eb264404dad6f7b
>
> "linux-next" does not look like a valid git repo address.

Well, now, that's your fault, isn't it? You're abbreviating the git
tree in the report, but then refusing to accept the abbreviation in
the response.

[Matthew Wilcox]

On Tue, Jul 25, 2023 at 11:57:22PM -0700, syzbot wrote:

> syzbot has bisected this issue to:
>
> commit a52f58b34afe095ebc5823684eb264404dad6f7b
> Author: Matthew Wilcox (Oracle) <wi...@infradead.org>
> Date: Mon Jul 24 18:54:10 2023 +0000
>
> mm: handle faults that merely update the accessed bit under the VMA lock
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1783585ea80000
> start commit: [unknown]
> git tree: linux-next

#syz test git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next next-20230727

[Aleksandr Nogikh]

Good point.
There's an issue about this
(https://github.com/google/syzkaller/issues/2265), I've added it to my
backlog.

--
Aleksandr

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+8645fe...@syzkaller.appspotmail.com

Tested on:

commit: 451cc82b Add linux-next specific files for 20230727
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next next-20230727
console output: https://syzkaller.appspot.com/x/log.txt?x=120b5655a80000
kernel config: https://syzkaller.appspot.com/x/.config?x=23e0802de3cfc61f

dashboard link: https://syzkaller.appspot.com/bug?extid=8645fe63c4d22c8d27b8
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=12145a36a80000

Note: testing is done by a robot and is best-effort only.

[Jann Horn]

One kinda straightforward option would be to pass the vmf (or NULL if
it's not in fault context) to anon_vma_prepare(), teach it to bail if
it runs under the mm lock, and propagate a VM_FAULT_RETRY all the way
up? It can already fail due to OOM, so the bailout paths exist, though
you'd have to work a bit to plumb the right error code up.

And if you're feeling adventurous, you could try to build a way to
opportunistically upgrade from vma lock to mmap lock, to avoid having
to bail out all the way back up and then dive back in when that
happens. Something that does mmap_read_trylock(); on failure, bail out
with VM_FAULT_RETRY; on success, drop the VMA lock and change
vmf->flags to note the changed locking context.

[Matthew Wilcox]

On Thu, Jul 27, 2023 at 09:20:33PM +0200, Jann Horn wrote:
> One kinda straightforward option would be to pass the vmf (or NULL if
> it's not in fault context) to anon_vma_prepare(), teach it to bail if
> it runs under the mm lock, and propagate a VM_FAULT_RETRY all the way
> up? It can already fail due to OOM, so the bailout paths exist, though
> you'd have to work a bit to plumb the right error code up.
>
> And if you're feeling adventurous, you could try to build a way to
> opportunistically upgrade from vma lock to mmap lock, to avoid having
> to bail out all the way back up and then dive back in when that
> happens. Something that does mmap_read_trylock(); on failure, bail out
> with VM_FAULT_RETRY; on success, drop the VMA lock and change
> vmf->flags to note the changed locking context.

I think that's all a little more adventurous than I'd be comfortable
with right now ;-) I just sent the fix patch that syzbot tested to
Andrew for integration.

[syzbot]

Auto-closing this bug as obsolete.
No recent activity, existing reproducers are no longer triggering the issue.