[syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    6a0e38264012 Merge tag 'for-6.11-rc2-tag' of git://git.ker..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13d4fc91980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=505ed4a1dd93463a
dashboard link: https://syzkaller.appspot.com/bug?extid=f7a1c2c2711e4a780f19
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=15fd9755980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1546c4e5980000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-6a0e3826.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/dd9ba9302e4d/vmlinux-6a0e3826.xz
kernel image: https://storage.googleapis.com/syzbot-assets/aa3ef19fbf4e/bzImage-6a0e3826.xz

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

R10: 0000000000000001 R11: 0000000000000246 R12: 00007f4aa48c4618
R13: 00007ffc0345d258 R14: 0000000000000001 R15: 0000000000000001
 </TASK>
==================================================================
BUG: KASAN: slab-use-after-free in consumer_del kernel/events/uprobes.c:772 [inline]
BUG: KASAN: slab-use-after-free in __uprobe_unregister+0x210/0x260 kernel/events/uprobes.c:1087
Read of size 8 at addr ffff888025a498b8 by task syz-executor380/5333

CPU: 2 UID: 0 PID: 5333 Comm: syz-executor380 Not tainted 6.11.0-rc2-syzkaller-00027-g6a0e38264012 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:93 [inline]
 dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:119
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0xc3/0x620 mm/kasan/report.c:488
 kasan_report+0xd9/0x110 mm/kasan/report.c:601
 consumer_del kernel/events/uprobes.c:772 [inline]
 __uprobe_unregister+0x210/0x260 kernel/events/uprobes.c:1087
 uprobe_unregister+0x45/0x70 kernel/events/uprobes.c:1111
 bpf_uprobe_unregister+0xfb/0x1d0 kernel/trace/bpf_trace.c:3187
 bpf_uprobe_multi_link_release+0x6d/0x180 kernel/trace/bpf_trace.c:3197
 bpf_link_free+0x12c/0x2b0 kernel/bpf/syscall.c:3067
 bpf_link_put_direct kernel/bpf/syscall.c:3107 [inline]
 bpf_link_release+0x63/0x80 kernel/bpf/syscall.c:3114
 __fput+0x408/0xbb0 fs/file_table.c:422
 task_work_run+0x14e/0x250 kernel/task_work.c:228
 exit_task_work include/linux/task_work.h:40 [inline]
 do_exit+0xaa3/0x2bb0 kernel/exit.c:882
 do_group_exit+0xd3/0x2a0 kernel/exit.c:1031
 __do_sys_exit_group kernel/exit.c:1042 [inline]
 __se_sys_exit_group kernel/exit.c:1040 [inline]
 __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1040
 x64_sys_call+0x14a9/0x16a0 arch/x86/include/generated/asm/syscalls_64.h:232
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f4aa48570e9
Code: Unable to access opcode bytes at 0x7f4aa48570bf.
RSP: 002b:00007ffc0345d048 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4aa48570e9
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007f4aa48ca350 R08: ffffffffffffffb8 R09: 00007f4aa48c0038
R10: 0000000000000001 R11: 0000000000000246 R12: 00007f4aa48ca350
R13: 0000000000000000 R14: 00007f4aa48cada0 R15: 00007f4aa48211a0
 </TASK>

Allocated by task 5333:
 kasan_save_stack+0x33/0x60 mm/kasan/common.c:47
 kasan_save_track+0x14/0x30 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
 __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:387
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 __do_kmalloc_node mm/slub.c:4158 [inline]
 __kmalloc_node_noprof+0x211/0x430 mm/slub.c:4164
 __kvmalloc_node_noprof+0x9d/0x1a0 mm/util.c:650
 kvmalloc_array_node_noprof include/linux/slab.h:833 [inline]
 bpf_uprobe_multi_link_attach+0x45d/0xe20 kernel/trace/bpf_trace.c:3439
 link_create kernel/bpf/syscall.c:5319 [inline]
 __sys_bpf+0x41ff/0x4a20 kernel/bpf/syscall.c:5780
 __do_sys_bpf kernel/bpf/syscall.c:5817 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5815 [inline]
 __x64_sys_bpf+0x78/0xc0 kernel/bpf/syscall.c:5815
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 5333:
 kasan_save_stack+0x33/0x60 mm/kasan/common.c:47
 kasan_save_track+0x14/0x30 mm/kasan/common.c:68
 kasan_save_free_info+0x3b/0x60 mm/kasan/generic.c:579
 poison_slab_object+0xf7/0x160 mm/kasan/common.c:240
 __kasan_slab_free+0x32/0x50 mm/kasan/common.c:256
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2252 [inline]
 slab_free mm/slub.c:4473 [inline]
 kfree+0x12a/0x3b0 mm/slub.c:4594
 kvfree+0x47/0x50 mm/util.c:696
 bpf_uprobe_multi_link_attach+0xaae/0xe20 kernel/trace/bpf_trace.c:3500
 link_create kernel/bpf/syscall.c:5319 [inline]
 __sys_bpf+0x41ff/0x4a20 kernel/bpf/syscall.c:5780
 __do_sys_bpf kernel/bpf/syscall.c:5817 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5815 [inline]
 __x64_sys_bpf+0x78/0xc0 kernel/bpf/syscall.c:5815
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

The buggy address belongs to the object at ffff888025a49880
 which belongs to the cache kmalloc-64 of size 64
The buggy address is located 56 bytes inside of
 freed 64-byte region [ffff888025a49880, ffff888025a498c0)

The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x25a49
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xfdffffff(slab)
raw: 00fff00000000000 ffff8880158428c0 dead000000000100 dead000000000122
raw: 0000000000000000 0000000080200020 00000001fdffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x52cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP), pid 11, tgid 11 (kworker/u32:0), ts 16060739565, free_ts 16025939467
 set_page_owner include/linux/page_owner.h:32 [inline]
 post_alloc_hook+0x2d1/0x350 mm/page_alloc.c:1493
 prep_new_page mm/page_alloc.c:1501 [inline]
 get_page_from_freelist+0x1351/0x2e50 mm/page_alloc.c:3442
 __alloc_pages_noprof+0x22b/0x2460 mm/page_alloc.c:4700
 __alloc_pages_node_noprof include/linux/gfp.h:269 [inline]
 alloc_pages_node_noprof include/linux/gfp.h:296 [inline]
 alloc_slab_page+0x4e/0xf0 mm/slub.c:2321
 allocate_slab mm/slub.c:2484 [inline]
 new_slab+0x84/0x260 mm/slub.c:2537
 ___slab_alloc+0xdac/0x1870 mm/slub.c:3723
 __slab_alloc.constprop.0+0x56/0xb0 mm/slub.c:3813
 __slab_alloc_node mm/slub.c:3866 [inline]
 slab_alloc_node mm/slub.c:4025 [inline]
 __kmalloc_cache_node_noprof+0xf1/0x350 mm/slub.c:4197
 kmalloc_node_noprof include/linux/slab.h:704 [inline]
 __get_vm_area_node+0xe1/0x2d0 mm/vmalloc.c:3109
 __vmalloc_node_range_noprof+0x276/0x1520 mm/vmalloc.c:3801
 alloc_thread_stack_node kernel/fork.c:313 [inline]
 dup_task_struct kernel/fork.c:1113 [inline]
 copy_process+0x2f3b/0x8de0 kernel/fork.c:2204
 kernel_clone+0xfd/0x980 kernel/fork.c:2781
 user_mode_thread+0xb4/0xf0 kernel/fork.c:2859
 call_usermodehelper_exec_work kernel/umh.c:172 [inline]
 call_usermodehelper_exec_work+0xcb/0x170 kernel/umh.c:158
 process_one_work+0x9c5/0x1b40 kernel/workqueue.c:3231
 process_scheduled_works kernel/workqueue.c:3312 [inline]
 worker_thread+0x6c8/0xf20 kernel/workqueue.c:3390
page last free pid 986 tgid 986 stack trace:
 reset_page_owner include/linux/page_owner.h:25 [inline]
 free_pages_prepare mm/page_alloc.c:1094 [inline]
 free_unref_page+0x64a/0xe40 mm/page_alloc.c:2612
 vfree+0x181/0x7a0 mm/vmalloc.c:3364
 delayed_vfree_work+0x56/0x70 mm/vmalloc.c:3285
 process_one_work+0x9c5/0x1b40 kernel/workqueue.c:3231
 process_scheduled_works kernel/workqueue.c:3312 [inline]
 worker_thread+0x6c8/0xf20 kernel/workqueue.c:3390
 kthread+0x2c1/0x3a0 kernel/kthread.c:389
 ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244

Memory state around the buggy address:
 ffff888025a49780: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff888025a49800: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
>ffff888025a49880: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
                                        ^
 ffff888025a49900: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff888025a49980: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
==================================================================


---
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 syzkaller@googlegroups.com.

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

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

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.

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

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

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

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Oleg Nesterov]

Hmm, bpf_uprobe_multi_link_attach() looks obviously wrong.

bpf_link_prime() is called after the

	for (i = 0; i < cnt; i++) {
		uprobe_register(...);
		...
	}

loop. If bpf_link_prime() fails, bpf_uprobe_multi_link_attach() just do
kvfree(uprobes) without _unregister(). In particular, this leaks the freed
bpf_uprobe->consumer in the uprobe->consumers list.

After that another _unregister() on the same uprobe can hit the problem.

I guess we need a simple patch for -stable...

Oleg.

On 08/10, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    6a0e38264012 Merge tag 'for-6.11-rc2-tag' of git://git.ker..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=13d4fc91980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=505ed4a1dd93463a
> dashboard link: https://syzkaller.appspot.com/bug?extid=f7a1c2c2711e4a780f19
> 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=15fd9755980000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1546c4e5980000
>
> Downloadable assets:
> disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-6a0e3826.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/dd9ba9302e4d/vmlinux-6a0e3826.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/aa3ef19fbf4e/bzImage-6a0e3826.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+f7a1c2c2711e4a780f19@syzkaller.appspotmail.com
>
> R10: 0000000000000001 R11: 0000000000000246 R12: 00007f4aa48c4618
> R13: 00007ffc0345d258 R14: 0000000000000001 R15: 0000000000000001
>  </TASK>
> ==================================================================
> BUG: KASAN: slab-use-after-free in consumer_del kernel/events/uprobes.c:772 [inline]
> BUG: KASAN: slab-use-after-free in __uprobe_unregister+0x210/0x260 kernel/events/uprobes.c:1087
> Read of size 8 at addr ffff888025a498b8 by task syz-executor380/5333
>
> CPU: 2 UID: 0 PID: 5333 Comm: syz-executor380 Not tainted 6.11.0-rc2-syzkaller-00027-g6a0e38264012 #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:93 [inline]
>  dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:119
>  print_address_description mm/kasan/report.c:377 [inline]
>  print_report+0xc3/0x620 mm/kasan/report.c:488
>  kasan_report+0xd9/0x110 mm/kasan/report.c:601
>  consumer_del kernel/events/uprobes.c:772 [inline]
>  __uprobe_unregister+0x210/0x260 kernel/events/uprobes.c:1087
>  uprobe_unregister+0x45/0x70 kernel/events/uprobes.c:1111
>  bpf_uprobe_unregister+0xfb/0x1d0 kernel/trace/bpf_trace.c:3187
>  bpf_uprobe_multi_link_release+0x6d/0x180 kernel/trace/bpf_trace.c:3197
>  bpf_link_free+0x12c/0x2b0 kernel/bpf/syscall.c:3067
>  bpf_link_put_direct kernel/bpf/syscall.c:3107 [inline]
>  bpf_link_release+0x63/0x80 kernel/bpf/syscall.c:3114
>  __fput+0x408/0xbb0 fs/file_table.c:422
>  task_work_run+0x14e/0x250 kernel/task_work.c:228
>  exit_task_work include/linux/task_work.h:40 [inline]
>  do_exit+0xaa3/0x2bb0 kernel/exit.c:882
>  do_group_exit+0xd3/0x2a0 kernel/exit.c:1031
>  __do_sys_exit_group kernel/exit.c:1042 [inline]
>  __se_sys_exit_group kernel/exit.c:1040 [inline]
>  __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1040
>  x64_sys_call+0x14a9/0x16a0 arch/x86/include/generated/asm/syscalls_64.h:232
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f4aa48570e9
> Code: Unable to access opcode bytes at 0x7f4aa48570bf.
> RSP: 002b:00007ffc0345d048 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4aa48570e9
> RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
> RBP: 00007f4aa48ca350 R08: ffffffffffffffb8 R09: 00007f4aa48c0038
> R10: 0000000000000001 R11: 0000000000000246 R12: 00007f4aa48ca350
> R13: 0000000000000000 R14: 00007f4aa48cada0 R15: 00007f4aa48211a0
>  </TASK>
>
> Allocated by task 5333:
>  kasan_save_stack+0x33/0x60 mm/kasan/common.c:47
>  kasan_save_track+0x14/0x30 mm/kasan/common.c:68
>  poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
>  __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:387
>  kasan_kmalloc include/linux/kasan.h:211 [inline]
>  __do_kmalloc_node mm/slub.c:4158 [inline]
>  __kmalloc_node_noprof+0x211/0x430 mm/slub.c:4164
>  __kvmalloc_node_noprof+0x9d/0x1a0 mm/util.c:650
>  kvmalloc_array_node_noprof include/linux/slab.h:833 [inline]
>  bpf_uprobe_multi_link_attach+0x45d/0xe20 kernel/trace/bpf_trace.c:3439
>  link_create kernel/bpf/syscall.c:5319 [inline]
>  __sys_bpf+0x41ff/0x4a20 kernel/bpf/syscall.c:5780
>  __do_sys_bpf kernel/bpf/syscall.c:5817 [inline]
>  __se_sys_bpf kernel/bpf/syscall.c:5815 [inline]
>  __x64_sys_bpf+0x78/0xc0 kernel/bpf/syscall.c:5815
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Freed by task 5333:
>  kasan_save_stack+0x33/0x60 mm/kasan/common.c:47
>  kasan_save_track+0x14/0x30 mm/kasan/common.c:68
>  kasan_save_free_info+0x3b/0x60 mm/kasan/generic.c:579
>  poison_slab_object+0xf7/0x160 mm/kasan/common.c:240
>  __kasan_slab_free+0x32/0x50 mm/kasan/common.c:256
>  kasan_slab_free include/linux/kasan.h:184 [inline]
>  slab_free_hook mm/slub.c:2252 [inline]
>  slab_free mm/slub.c:4473 [inline]
>  kfree+0x12a/0x3b0 mm/slub.c:4594
>  kvfree+0x47/0x50 mm/util.c:696
>  bpf_uprobe_multi_link_attach+0xaae/0xe20 kernel/trace/bpf_trace.c:3500
>  link_create kernel/bpf/syscall.c:5319 [inline]
>  __sys_bpf+0x41ff/0x4a20 kernel/bpf/syscall.c:5780
>  __do_sys_bpf kernel/bpf/syscall.c:5817 [inline]
>  __se_sys_bpf kernel/bpf/syscall.c:5815 [inline]
>  __x64_sys_bpf+0x78/0xc0 kernel/bpf/syscall.c:5815
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xcd/0x250 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> The buggy address belongs to the object at ffff888025a49880
>  which belongs to the cache kmalloc-64 of size 64
> The buggy address is located 56 bytes inside of
>  freed 64-byte region [ffff888025a49880, ffff888025a498c0)
>
> The buggy address belongs to the physical page:
> page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x25a49
> flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
> page_type: 0xfdffffff(slab)
> raw: 00fff00000000000 ffff8880158428c0 dead000000000100 dead000000000122
> raw: 0000000000000000 0000000080200020 00000001fdffffff 0000000000000000
> page dumped because: kasan: bad access detected
> page_owner tracks the page as allocated
> page last allocated via order 0, migratetype Unmovable, gfp_mask 0x52cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP), pid 11, tgid 11 (kworker/u32:0), ts 16060739565, free_ts 16025939467
>  set_page_owner include/linux/page_owner.h:32 [inline]
>  post_alloc_hook+0x2d1/0x350 mm/page_alloc.c:1493
>  prep_new_page mm/page_alloc.c:1501 [inline]
>  get_page_from_freelist+0x1351/0x2e50 mm/page_alloc.c:3442
>  __alloc_pages_noprof+0x22b/0x2460 mm/page_alloc.c:4700
>  __alloc_pages_node_noprof include/linux/gfp.h:269 [inline]
>  alloc_pages_node_noprof include/linux/gfp.h:296 [inline]
>  alloc_slab_page+0x4e/0xf0 mm/slub.c:2321
>  allocate_slab mm/slub.c:2484 [inline]
>  new_slab+0x84/0x260 mm/slub.c:2537
>  ___slab_alloc+0xdac/0x1870 mm/slub.c:3723
>  __slab_alloc.constprop.0+0x56/0xb0 mm/slub.c:3813
>  __slab_alloc_node mm/slub.c:3866 [inline]
>  slab_alloc_node mm/slub.c:4025 [inline]
>  __kmalloc_cache_node_noprof+0xf1/0x350 mm/slub.c:4197
>  kmalloc_node_noprof include/linux/slab.h:704 [inline]
>  __get_vm_area_node+0xe1/0x2d0 mm/vmalloc.c:3109
>  __vmalloc_node_range_noprof+0x276/0x1520 mm/vmalloc.c:3801
>  alloc_thread_stack_node kernel/fork.c:313 [inline]
>  dup_task_struct kernel/fork.c:1113 [inline]
>  copy_process+0x2f3b/0x8de0 kernel/fork.c:2204
>  kernel_clone+0xfd/0x980 kernel/fork.c:2781
>  user_mode_thread+0xb4/0xf0 kernel/fork.c:2859
>  call_usermodehelper_exec_work kernel/umh.c:172 [inline]
>  call_usermodehelper_exec_work+0xcb/0x170 kernel/umh.c:158
>  process_one_work+0x9c5/0x1b40 kernel/workqueue.c:3231
>  process_scheduled_works kernel/workqueue.c:3312 [inline]
>  worker_thread+0x6c8/0xf20 kernel/workqueue.c:3390
> page last free pid 986 tgid 986 stack trace:
>  reset_page_owner include/linux/page_owner.h:25 [inline]
>  free_pages_prepare mm/page_alloc.c:1094 [inline]
>  free_unref_page+0x64a/0xe40 mm/page_alloc.c:2612
>  vfree+0x181/0x7a0 mm/vmalloc.c:3364
>  delayed_vfree_work+0x56/0x70 mm/vmalloc.c:3285
>  process_one_work+0x9c5/0x1b40 kernel/workqueue.c:3231
>  process_scheduled_works kernel/workqueue.c:3312 [inline]
>  worker_thread+0x6c8/0xf20 kernel/workqueue.c:3390
>  kthread+0x2c1/0x3a0 kernel/kthread.c:389
>  ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
>
> Memory state around the buggy address:
>  ffff888025a49780: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>  ffff888025a49800: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
> >ffff888025a49880: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>                                         ^
>  ffff888025a49900: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>  ffff888025a49980: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
> ==================================================================
>
>
> ---
> 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 syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> 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.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Oleg Nesterov]

On 08/11, Oleg Nesterov wrote:
>
> Hmm, bpf_uprobe_multi_link_attach() looks obviously wrong.
>
> bpf_link_prime() is called after the
>
> 	for (i = 0; i < cnt; i++) {
> 		uprobe_register(...);
> 		...
> 	}
>
> loop. If bpf_link_prime() fails, bpf_uprobe_multi_link_attach() just do
> kvfree(uprobes) without _unregister(). In particular, this leaks the freed
> bpf_uprobe->consumer in the uprobe->consumers list.
>
> After that another _unregister() on the same uprobe can hit the problem.
>
> I guess we need a simple patch for -stable...

Something like below on top of perf/core. But I don't like the usage of
"i" in the +error_unregister path...

Oleg.

--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3486,17 +3486,19 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 						    &uprobes[i].consumer);
 		if (IS_ERR(uprobes[i].uprobe)) {
 			err = PTR_ERR(uprobes[i].uprobe);
-			bpf_uprobe_unregister(uprobes, i);
-			goto error_free;
+			goto error_unregister;
 		}
 	}
 
 	err = bpf_link_prime(&link->link, &link_primer);
 	if (err)
-		goto error_free;
+		goto error_unregister;
 
 	return bpf_link_settle(&link_primer);
 
+error_unregister:
+	bpf_uprobe_unregister(uprobes, i);
+
 error_free:
 	kvfree(uprobes);
 	kfree(link);

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Andrii Nakryiko]

On Sun, Aug 11, 2024 at 5:35 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/11, Oleg Nesterov wrote:
> >
> > Hmm, bpf_uprobe_multi_link_attach() looks obviously wrong.
> >
> > bpf_link_prime() is called after the
> >
> >       for (i = 0; i < cnt; i++) {
> >               uprobe_register(...);
> >               ...
> >       }
> >
> > loop. If bpf_link_prime() fails, bpf_uprobe_multi_link_attach() just do
> > kvfree(uprobes) without _unregister(). In particular, this leaks the freed
> > bpf_uprobe->consumer in the uprobe->consumers list.
> >
> > After that another _unregister() on the same uprobe can hit the problem.
> >
> > I guess we need a simple patch for -stable...
>
> Something like below on top of perf/core. But I don't like the usage of
> "i" in the +error_unregister path...
>

Wouldn't the below be cleaner?

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index cd098846e251..3ca65454f888 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3491,8 +3491,10 @@ int bpf_uprobe_multi_link_attach(const union
bpf_attr *attr, struct bpf_prog *pr
        }

        err = bpf_link_prime(&link->link, &link_primer);
-       if (err)
+       if (err) {
+               bpf_uprobe_unregister(&path, uprobes, cnt);
                goto error_free;
+       }

        return bpf_link_settle(&link_primer);


We should probably route this through the bpf tree, I don't think it
will conflict with your changes, right?


> Oleg.
>
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3486,17 +3486,19 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>                                                     &uprobes[i].consumer);
>                 if (IS_ERR(uprobes[i].uprobe)) {
>                         err = PTR_ERR(uprobes[i].uprobe);
> -                       bpf_uprobe_unregister(uprobes, i);
> -                       goto error_free;
> +                       goto error_unregister;
>                 }
>         }
>
>         err = bpf_link_prime(&link->link, &link_primer);
>         if (err)
> -               goto error_free;
> +               goto error_unregister;
>
>         return bpf_link_settle(&link_primer);
>
> +error_unregister:
> +       bpf_uprobe_unregister(uprobes, i);
> +
>  error_free:
>         kvfree(uprobes);
>         kfree(link);
>

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Oleg Nesterov]

On 08/11, Andrii Nakryiko wrote:
>
> On Sun, Aug 11, 2024 at 5:35 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > Something like below on top of perf/core. But I don't like the usage of
> > "i" in the +error_unregister path...
> >
>
> Wouldn't the below be cleaner?
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index cd098846e251..3ca65454f888 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3491,8 +3491,10 @@ int bpf_uprobe_multi_link_attach(const union
> bpf_attr *attr, struct bpf_prog *pr
>         }
>
>         err = bpf_link_prime(&link->link, &link_primer);
> -       if (err)
> +       if (err) {
> +               bpf_uprobe_unregister(&path, uprobes, cnt);

I disagree. This code already uses the "goto error_xxx" pattern, why
duplicate bpf_uprobe_unregister() ? What if another "can fail" code
comes between register and bpf_link_prime() ?

See the patch below, on top of perf/core.

> We should probably route this through the bpf tree, I don't think it
> will conflict with your changes, right?

It will conflict, and in this sense it is even worse than the "#syz test"
patch I sent in https://lore.kernel.org/all/20240811125816.GC30068@redhat.com/

Because with your version above the necessary change

	-	bpf_uprobe_unregister(&path, uprobes, cnt);
	+	bpf_uprobe_unregister(uprobes, cnt);

won't be noticed during the merge, I guess.

So can we route this fix through the perf/core ? I'll add "cc: stable",
in the next merge window the Greg's scripts will report the "FAILED"
status of the -stable patch, I'll send the trivial backport in reply.

No?

Oleg.
---

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 4e391daafa64..90cd30e9723e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3484,17 +3484,20 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 						    &uprobes[i].consumer);
 		if (IS_ERR(uprobes[i].uprobe)) {
 			err = PTR_ERR(uprobes[i].uprobe);
-			bpf_uprobe_unregister(uprobes, i);
-			goto error_free;
+			link->cnt = i;
+			goto error_unregister;
 		}
 	}
 
 	err = bpf_link_prime(&link->link, &link_primer);
 	if (err)
-		goto error_free;
+		goto error_unregister;
 
 	return bpf_link_settle(&link_primer);
 
+error_unregister:
+	bpf_uprobe_unregister(uprobes, link->cnt);
+
 error_free:
 	kvfree(uprobes);
 	kfree(link);

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Andrii Nakryiko]

adding bpf ML, given it's bpf's code base

On Mon, Aug 12, 2024 at 3:00 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/11, Andrii Nakryiko wrote:
> >
> > On Sun, Aug 11, 2024 at 5:35 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > Something like below on top of perf/core. But I don't like the usage of
> > > "i" in the +error_unregister path...
> > >
> >
> > Wouldn't the below be cleaner?
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index cd098846e251..3ca65454f888 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -3491,8 +3491,10 @@ int bpf_uprobe_multi_link_attach(const union
> > bpf_attr *attr, struct bpf_prog *pr
> >         }
> >
> >         err = bpf_link_prime(&link->link, &link_primer);
> > -       if (err)
> > +       if (err) {
> > +               bpf_uprobe_unregister(&path, uprobes, cnt);
>
> I disagree. This code already uses the "goto error_xxx" pattern, why

Well, if you have strong preferences, so be it (it's too trivial code
to argue about). We do have quite a lot of "hybrid" error handling
code that combines undoing the last step (especially if it's a simple
function call) and then doing goto for the rest of common error
handling, so I didn't (and still don't) see any problem with that.

> duplicate bpf_uprobe_unregister() ? What if another "can fail" code
> comes between register and bpf_link_prime() ?
>
> See the patch below, on top of perf/core.
>
> > We should probably route this through the bpf tree, I don't think it
> > will conflict with your changes, right?
>
> It will conflict, and in this sense it is even worse than the "#syz test"
> patch I sent in https://lore.kernel.org/all/20240811125816.GC30068@redhat.com/
>
> Because with your version above the necessary change
>
>         -       bpf_uprobe_unregister(&path, uprobes, cnt);
>         +       bpf_uprobe_unregister(uprobes, cnt);
>
> won't be noticed during the merge, I guess.
>

Yeah, my bad, I forgot that the signature of bpf_uprobe_unregister()
also changed with your patches.

> So can we route this fix through the perf/core ? I'll add "cc: stable",
> in the next merge window the Greg's scripts will report the "FAILED"
> status of the -stable patch, I'll send the trivial backport in reply.

Yep, absolutely, given the bpf_uprobe_unregister() change, I don't see
any problem for it to go together with your refactorings.

For the fix:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>
> No?
>
> Oleg.
> ---
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 4e391daafa64..90cd30e9723e 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3484,17 +3484,20 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>                                                     &uprobes[i].consumer);
>                 if (IS_ERR(uprobes[i].uprobe)) {
>                         err = PTR_ERR(uprobes[i].uprobe);
> -                       bpf_uprobe_unregister(uprobes, i);
> -                       goto error_free;
> +                       link->cnt = i;
> +                       goto error_unregister;
>                 }
>         }
>
>         err = bpf_link_prime(&link->link, &link_primer);
>         if (err)
> -               goto error_free;
> +               goto error_unregister;
>
>         return bpf_link_settle(&link_primer);
>
> +error_unregister:
> +       bpf_uprobe_unregister(uprobes, link->cnt);
> +
>  error_free:
>         kvfree(uprobes);
>         kfree(link);
>

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Oleg Nesterov]

On 08/12, Andrii Nakryiko wrote:
>
> adding bpf ML, given it's bpf's code base

Thanks,

> On Mon, Aug 12, 2024 at 3:00 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -3491,8 +3491,10 @@ int bpf_uprobe_multi_link_attach(const union
> > > bpf_attr *attr, struct bpf_prog *pr
> > >         }
> > >
> > >         err = bpf_link_prime(&link->link, &link_primer);
> > > -       if (err)
> > > +       if (err) {
> > > +               bpf_uprobe_unregister(&path, uprobes, cnt);
> >
> > I disagree. This code already uses the "goto error_xxx" pattern, why
>
> Well, if you have strong preferences,

Well, YES and NO ;) please see below.

> so be it (it's too trivial code
> to argue about).

Agreed. On a closer look both the code and the problem look very trivial.

But note that nobody noticed this trivial problem before. Including me who
had to change this trivial code to adapt to the recent API changes.

May be this means that we should keep the error handling in this function
more consistent ;)

> We do have quite a lot of "hybrid" error handling

And YES, I don't like this kind of error handling.

But, at the same time: NO, I never-never argue with the maintainers when it
comes to "cosmetic" issues.

My main point was (and you seem to agree) that this simpler patch above won't
simplify the routing. I too thought about the change above initially.

-------------------------------------------------------------------------------
> Yep, absolutely, given the bpf_uprobe_unregister() change, I don't see
> any problem for it to go together with your refactorings.
>
> For the fix:
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Thanks! I'll write the changelog and send this patch with your ack included
tomorrow.

Oleg.

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Andrii Nakryiko]

On Mon, Aug 12, 2024 at 12:25 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 08/12, Andrii Nakryiko wrote:
> >
> > adding bpf ML, given it's bpf's code base
>
> Thanks,
>
> > On Mon, Aug 12, 2024 at 3:00 AM Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > > --- a/kernel/trace/bpf_trace.c
> > > > +++ b/kernel/trace/bpf_trace.c
> > > > @@ -3491,8 +3491,10 @@ int bpf_uprobe_multi_link_attach(const union
> > > > bpf_attr *attr, struct bpf_prog *pr
> > > >         }
> > > >
> > > >         err = bpf_link_prime(&link->link, &link_primer);
> > > > -       if (err)
> > > > +       if (err) {
> > > > +               bpf_uprobe_unregister(&path, uprobes, cnt);
> > >
> > > I disagree. This code already uses the "goto error_xxx" pattern, why
> >
> > Well, if you have strong preferences,
>
> Well, YES and NO ;) please see below.
>
> > so be it (it's too trivial code
> > to argue about).
>
> Agreed. On a closer look both the code and the problem look very trivial.
>
> But note that nobody noticed this trivial problem before. Including me who
> had to change this trivial code to adapt to the recent API changes.

Yep, error handling problems tend to go unnoticed frequently. The good
thing is that this is quite unlikely in practice for bpf_link_prime()
to fail, which is why it was a syzbot that found this.

>
> May be this means that we should keep the error handling in this function
> more consistent ;)
>
> > We do have quite a lot of "hybrid" error handling
>
> And YES, I don't like this kind of error handling.
>
> But, at the same time: NO, I never-never argue with the maintainers when it
> comes to "cosmetic" issues.
>
> My main point was (and you seem to agree) that this simpler patch above won't
> simplify the routing. I too thought about the change above initially.
>

Agreed. Just stick to your code, it's fine. Thanks!

> -------------------------------------------------------------------------------
> > Yep, absolutely, given the bpf_uprobe_unregister() change, I don't see
> > any problem for it to go together with your refactorings.
> >
> > For the fix:
> >
> > Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> Thanks! I'll write the changelog and send this patch with your ack included
> tomorrow.
>
> Oleg.
>

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[Oleg Nesterov]

On 08/10, syzbot wrote:
>
> syzbot found the following issue on:
>
> HEAD commit:    6a0e38264012 Merge tag 'for-6.11-rc2-tag' of git://git.ker..
> git tree:       upstream

#syz test: upstream 6a0e38264012

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index cd098846e251..5d9c96c69733 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3485,17 +3485,19 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 					     uprobes[i].ref_ctr_offset,
 					     &uprobes[i].consumer);
 		if (err) {
-			bpf_uprobe_unregister(&path, uprobes, i);
-			goto error_free;
+			goto error_unregister;
 		}
 	}
 
 	err = bpf_link_prime(&link->link, &link_primer);
 	if (err)
-		goto error_free;
+		goto error_unregister;
 
 	return bpf_link_settle(&link_primer);
 
+error_unregister:
+	bpf_uprobe_unregister(&path, uprobes, i);
+
 error_free:
 	kvfree(uprobes);
 	kfree(link);

Re: [syzbot] [perf?] KASAN: slab-use-after-free Read in __uprobe_unregister

[syzbot]

Hello,

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

Reported-by: syzbot+f7a1c2c2711e4a780f19@syzkaller.appspotmail.com
Tested-by: syzbot+f7a1c2c2711e4a780f19@syzkaller.appspotmail.com

Tested on:

commit:         6a0e3826 Merge tag 'for-6.11-rc2-tag' of git://git.ker..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1430267d980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=505ed4a1dd93463a
dashboard link: https://syzkaller.appspot.com/bug?extid=f7a1c2c2711e4a780f19
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=1685014b980000

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