Re: [syzbot] [mm?] possible deadlock in __mmap_lock_do_trace_released

Re: [syzbot] [mm?] possible deadlock in __mmap_lock_do_trace_released

[Tetsuo Handa]

The local lock itself will be removed by

mm: mmap_lock: replace get_memcg_path_buf() with on-stack buffer

but is there possibility that this bpf program forms an infinite
recursion (kernel stack overflow) bug?

On 2024/07/03 3:54, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    a12978712d90 selftests/bpf: Move ARRAY_SIZE to bpf_misc.h
> git tree:       bpf-next
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=130457fa980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=736daf12bd72e034
> dashboard link: https://syzkaller.appspot.com/bug?extid=16b6ab88e66b34d09014
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=125718be980000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14528876980000

Re: [syzbot] [mm?] possible deadlock in __mmap_lock_do_trace_released

[Axel Rasmussen]

On Tue, Jul 2, 2024 at 3:09 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> The local lock itself will be removed by
>
> mm: mmap_lock: replace get_memcg_path_buf() with on-stack buffer
>
> but is there possibility that this bpf program forms an infinite
> recursion (kernel stack overflow) bug?

It looks like the answer is "sort of yes", based on this stack from
the syzkaller dashboard:

[...]
[   54.017745][ T4691]  stack_map_get_build_id_offset+0x252/0x360
[   54.023698][ T4691]  __bpf_get_stack+0x1d7/0x240
[   54.028425][ T4691]  ___bpf_prog_run+0x5f6/0x2280
[   54.033415][ T4691]  __bpf_prog_run32+0xbb/0xe0
[   54.038058][ T4691]  ? migrate_disable+0x38/0xb0
[   54.042785][ T4691]  ? trace_call_bpf+0x4b/0x3d0
[   54.047944][ T4691]  trace_call_bpf+0x164/0x3d0
[   54.052580][ T4691]  ? trace_call_bpf+0x4b/0x3d0
[   54.057302][ T4691]  perf_trace_run_bpf_submit+0x3b/0xa0
[   54.062809][ T4691]  perf_trace_mmap_lock_acquire_returned+0x141/0x170
[   54.069877][ T4691]  ? __mmap_lock_do_trace_acquire_returned+0x3e/0x200
[   54.076596][ T4691]  __mmap_lock_do_trace_acquire_returned+0x1e1/0x200
[...]

__mmap_lock_do_trace_acquire_returned is called with mmap_lock held,
which apparently in this setup eventually calls down into
stack_map_get_build_id_offset, which also tries to take mmap_lock.

But, note that stack_map_get_build_id calls mmap_read_trylock, so I
would expect in the recursive case that call will simply fail, and
then stack_map_get_build_id_offset appears to deal gracefully with
that?

I would guess this is not unique to the mmap_lock tracepoints though.
I think the same issue would exist any time a tracepoint is triggered
with mmap_lock held? With ftrace on the right function I would expect
it to be easy to trigger this.

>
> On 2024/07/03 3:54, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit:    a12978712d90 selftests/bpf: Move ARRAY_SIZE to bpf_misc.h
> > git tree:       bpf-next
> > console+strace: https://syzkaller.appspot.com/x/log.txt?x=130457fa980000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=736daf12bd72e034
> > dashboard link: https://syzkaller.appspot.com/bug?extid=16b6ab88e66b34d09014
> > compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=125718be980000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14528876980000
>

Re: [syzbot] [mm?] possible deadlock in __mmap_lock_do_trace_released

[Tetsuo Handa]

On 2024/07/03 7:35, Axel Rasmussen wrote:
> But, note that stack_map_get_build_id calls mmap_read_trylock, so I
> would expect in the recursive case that call will simply fail, and
> then stack_map_get_build_id_offset appears to deal gracefully with
> that?

Unless that mmap was already held for write (or someone has started waiting
to hold it for write), recursive mmap_read_trylock() will succeed. Thus,
unless there is a guarantee of no infinite recursion, we should implement
a safeguard based on the worst scenario.

Re: [syzbot] [mm?] possible deadlock in __mmap_lock_do_trace_released

[syzbot]

syzbot suspects this issue was fixed by commit:

commit 7d6be67cfdd4a53cea7147313ca13c531e3a470f
Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date:   Fri Jun 21 01:08:41 2024 +0000

    mm: mmap_lock: replace get_memcg_path_buf() with on-stack buffer

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=12d48893980000
start commit:   a12978712d90 selftests/bpf: Move ARRAY_SIZE to bpf_misc.h
git tree:       bpf-next
kernel config:  https://syzkaller.appspot.com/x/.config?x=736daf12bd72e034
dashboard link: https://syzkaller.appspot.com/bug?extid=16b6ab88e66b34d09014
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=125718be980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14528876980000

If the result looks correct, please mark the issue as fixed by replying with:

#syz fix: mm: mmap_lock: replace get_memcg_path_buf() with on-stack buffer

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