[syzbot] [bpf?] KASAN: slab-use-after-free Read in do_check

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 19a60293b992 Add linux-next specific files for 20250611
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=15472d70580000
kernel config: https://syzkaller.appspot.com/x/.config?x=76ed3656d7159e27
dashboard link: https://syzkaller.appspot.com/bug?extid=b5eb72a560b8149a1885
compiler: Debian clang version 20.1.6 (++20250514063057+1e4d39e07757-1~exp1~20250514183223.118), Debian LLD 20.1.6
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16af860c580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=174db60c580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/c453c11565fa/disk-19a60293.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/4034ded42b2e/vmlinux-19a60293.xz
kernel image: https://storage.googleapis.com/syzbot-assets/5355903cdb8f/bzImage-19a60293.xz

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

==================================================================
BUG: KASAN: slab-use-after-free in do_check+0xb388/0xe170 kernel/bpf/verifier.c:19756
Read of size 1 at addr ffff88801deeef79 by task syz-executor672/5842

CPU: 1 UID: 0 PID: 5842 Comm: syz-executor672 Not tainted 6.16.0-rc1-next-20250611-syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
Call Trace:
<TASK>
dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:408 [inline]
print_report+0xd2/0x2b0 mm/kasan/report.c:521
kasan_report+0x118/0x150 mm/kasan/report.c:634
do_check+0xb388/0xe170 kernel/bpf/verifier.c:19756
do_check_common+0x168d/0x20b0 kernel/bpf/verifier.c:22905
do_check_main kernel/bpf/verifier.c:22996 [inline]
bpf_check+0x1381e/0x19e50 kernel/bpf/verifier.c:24162
bpf_prog_load+0x1318/0x1930 kernel/bpf/syscall.c:2972
__sys_bpf+0x5f1/0x860 kernel/bpf/syscall.c:5978
__do_sys_bpf kernel/bpf/syscall.c:6085 [inline]
__se_sys_bpf kernel/bpf/syscall.c:6083 [inline]
__x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:6083
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f7586cdbeb9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 c1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc2e683128 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7586cdbeb9
RDX: 0000000000000094 RSI: 0000200000000840 RDI: 0000000000000005
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000006
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000001
</TASK>

Allocated by task 5842:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3e/0x80 mm/kasan/common.c:68
poison_kmalloc_redzone mm/kasan/common.c:377 [inline]
__kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:394
kasan_kmalloc include/linux/kasan.h:260 [inline]
__kmalloc_cache_noprof+0x230/0x3d0 mm/slub.c:4359
kmalloc_noprof include/linux/slab.h:905 [inline]
kzalloc_noprof include/linux/slab.h:1039 [inline]
do_check_common+0x13f/0x20b0 kernel/bpf/verifier.c:22798
do_check_main kernel/bpf/verifier.c:22996 [inline]
bpf_check+0x1381e/0x19e50 kernel/bpf/verifier.c:24162
bpf_prog_load+0x1318/0x1930 kernel/bpf/syscall.c:2972
__sys_bpf+0x5f1/0x860 kernel/bpf/syscall.c:5978
__do_sys_bpf kernel/bpf/syscall.c:6085 [inline]
__se_sys_bpf kernel/bpf/syscall.c:6083 [inline]
__x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:6083
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 5842:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3e/0x80 mm/kasan/common.c:68
kasan_save_free_info+0x46/0x50 mm/kasan/generic.c:576
poison_slab_object mm/kasan/common.c:247 [inline]
__kasan_slab_free+0x62/0x70 mm/kasan/common.c:264
kasan_slab_free include/linux/kasan.h:233 [inline]
slab_free_hook mm/slub.c:2381 [inline]
slab_free mm/slub.c:4643 [inline]
kfree+0x18e/0x440 mm/slub.c:4842
push_stack+0x247/0x3c0 kernel/bpf/verifier.c:2069
check_cond_jmp_op+0x1069/0x2340 kernel/bpf/verifier.c:16562
do_check_insn kernel/bpf/verifier.c:19621 [inline]
do_check+0x672c/0xe170 kernel/bpf/verifier.c:19755
do_check_common+0x168d/0x20b0 kernel/bpf/verifier.c:22905
do_check_main kernel/bpf/verifier.c:22996 [inline]
bpf_check+0x1381e/0x19e50 kernel/bpf/verifier.c:24162
bpf_prog_load+0x1318/0x1930 kernel/bpf/syscall.c:2972
__sys_bpf+0x5f1/0x860 kernel/bpf/syscall.c:5978
__do_sys_bpf kernel/bpf/syscall.c:6085 [inline]
__se_sys_bpf kernel/bpf/syscall.c:6083 [inline]
__x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:6083
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f

The buggy address belongs to the object at ffff88801deeef00
which belongs to the cache kmalloc-192 of size 192
The buggy address is located 121 bytes inside of
freed 192-byte region [ffff88801deeef00, ffff88801deeefc0)

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1deee
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
page_type: f5(slab)
raw: 00fff00000000000 ffff88801a4413c0 ffffea00006fca40 dead000000000004
raw: 0000000000000000 0000000000100010 00000000f5000000 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 0x52820(GFP_ATOMIC|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP), pid 1, tgid 1 (swapper/0), ts 2954361175, free_ts 2954343552
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x240/0x2a0 mm/page_alloc.c:1704
prep_new_page mm/page_alloc.c:1712 [inline]
get_page_from_freelist+0x21e4/0x22c0 mm/page_alloc.c:3669
__alloc_frozen_pages_noprof+0x181/0x370 mm/page_alloc.c:4959
alloc_pages_mpol+0x232/0x4a0 mm/mempolicy.c:2419
alloc_slab_page mm/slub.c:2451 [inline]
allocate_slab+0x8a/0x3b0 mm/slub.c:2619
new_slab mm/slub.c:2673 [inline]
___slab_alloc+0xbfc/0x1480 mm/slub.c:3859
__slab_alloc mm/slub.c:3949 [inline]
__slab_alloc_node mm/slub.c:4024 [inline]
slab_alloc_node mm/slub.c:4185 [inline]
__do_kmalloc_node mm/slub.c:4327 [inline]
__kmalloc_node_noprof+0x2fd/0x4e0 mm/slub.c:4334
kmalloc_node_noprof include/linux/slab.h:932 [inline]
__vmalloc_area_node mm/vmalloc.c:3690 [inline]
__vmalloc_node_range_noprof+0x5a9/0x12f0 mm/vmalloc.c:3885
vmalloc_huge_node_noprof+0xb3/0xf0 mm/vmalloc.c:4001
vmalloc_huge include/linux/vmalloc.h:185 [inline]
alloc_large_system_hash+0x2b8/0x5e0 mm/mm_init.c:2515
posixtimer_init+0x140/0x270 kernel/time/posix-timers.c:1561
do_one_initcall+0x233/0x820 init/main.c:1274
do_initcall_level+0x137/0x1f0 init/main.c:1336
do_initcalls+0x69/0xd0 init/main.c:1352
kernel_init_freeable+0x3d9/0x570 init/main.c:1584
kernel_init+0x1d/0x1d0 init/main.c:1474
page last free pid 1 tgid 1 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1248 [inline]
__free_frozen_pages+0xc71/0xe70 mm/page_alloc.c:2706
__kasan_populate_vmalloc mm/kasan/shadow.c:383 [inline]
kasan_populate_vmalloc+0x18a/0x1a0 mm/kasan/shadow.c:417
alloc_vmap_area+0xd51/0x1490 mm/vmalloc.c:2084
__get_vm_area_node+0x1f8/0x300 mm/vmalloc.c:3179
__vmalloc_node_range_noprof+0x301/0x12f0 mm/vmalloc.c:3845
vmalloc_huge_node_noprof+0xb3/0xf0 mm/vmalloc.c:4001
vmalloc_huge include/linux/vmalloc.h:185 [inline]
alloc_large_system_hash+0x2b8/0x5e0 mm/mm_init.c:2515
posixtimer_init+0x140/0x270 kernel/time/posix-timers.c:1561
do_one_initcall+0x233/0x820 init/main.c:1274
do_initcall_level+0x137/0x1f0 init/main.c:1336
do_initcalls+0x69/0xd0 init/main.c:1352
kernel_init_freeable+0x3d9/0x570 init/main.c:1584
kernel_init+0x1d/0x1d0 init/main.c:1474
ret_from_fork+0x3f9/0x770 arch/x86/kernel/process.c:148
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

Memory state around the buggy address:
ffff88801deeee00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff88801deeee80: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
>ffff88801deeef00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88801deeef80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
ffff88801deef000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================


---
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 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

[Eduard Zingerman]

Accessed memory is freed at an error path in push_stack():

static struct bpf_verifier_state *push_stack(...)
{
...
err:
free_verifier_state(env->cur_state, true); // <-- KASAN points here
...
}

And is accessed after being freed here:

static int do_check(struct bpf_verifier_env *env)
{
...
err = do_check_insn(env, &do_print_state);
KASAN --> if (state->speculative && error_recoverable_with_nospec(err)) ...
...
}

[...]

Either 'state = env->cur_state' is needed after 'do_check_insn()' or
error path should not free env->cur_state (seems logical).

[Luis Gerhorst]

Eduard Zingerman <edd...@gmail.com> writes:

> Accessed memory is freed at an error path in push_stack():
>
> static struct bpf_verifier_state *push_stack(...)
> {
> ...
> err:
> free_verifier_state(env->cur_state, true); // <-- KASAN points here
> ...
> }
>
> And is accessed after being freed here:
>
> static int do_check(struct bpf_verifier_env *env)
> {
> ...
> err = do_check_insn(env, &do_print_state);
> KASAN --> if (state->speculative && error_recoverable_with_nospec(err)) ...
> ...
> }
>
> [...]
>
> Either 'state = env->cur_state' is needed after 'do_check_insn()' or
> error path should not free env->cur_state (seems logical).

Sorry, this was my error from [1]. Thanks for the pointer.

Yes, I think the former makes sense (with the respective `state &&`
added to the if).

The latter might also be possible, but I guess it would require more
significant changes.

state->speculative does not make sense if the error path of push_stack()
ran. In that case, `state->speculative &&
error_recoverable_with_nospec(err)` as a whole should already never
evaluate to true (because all cases where push_stack() fails also return
a non-recoverable error -ENOMEM/-EFAULT).

Alternatively to adding `state = env->cur_state` and `state &&`, turning
the check around would avoid the use-after-free. However, I think your
idea is better because it is more explicit compared to this:

if (error_recoverable_with_nospec(err) && state->speculative) ...

Does this make sense to you? If yes I can send the fix later today.

I will also check that all other paths calling free_verifier_state() are
sane. So far it looks good.

The later

if (state->speculative && cur_aux(env)->nospec_result) {

should already be fine, because !env->cur_state should imply that the
previous if raises the error.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=d6f1c85f22534d2d9fea9b32645da19c91ebe7d2

[Eduard Zingerman]

On Wed, 2025-06-11 at 16:03 +0200, Luis Gerhorst wrote:
> Eduard Zingerman <edd...@gmail.com> writes:
>
> > Accessed memory is freed at an error path in push_stack():
> >
> > static struct bpf_verifier_state *push_stack(...)
> > {
> > ...
> > err:
> > free_verifier_state(env->cur_state, true); // <-- KASAN points here
> > ...
> > }
> >
> > And is accessed after being freed here:
> >
> > static int do_check(struct bpf_verifier_env *env)
> > {
> > ...
> > err = do_check_insn(env, &do_print_state);
> > KASAN --> if (state->speculative && error_recoverable_with_nospec(err)) ...
> > ...
> > }
> >
> > [...]
> >
> > Either 'state = env->cur_state' is needed after 'do_check_insn()' or
> > error path should not free env->cur_state (seems logical).
>
> Sorry, this was my error from [1]. Thanks for the pointer.
>
> Yes, I think the former makes sense (with the respective `state &&`
> added to the if).
>
> The latter might also be possible, but I guess it would require more
> significant changes.

do_check_common() has the following logic:

out:
/* check for NULL is necessary, since cur_state can be freed inside
* do_check() under memory pressure.
*/
if (env->cur_state) {
free_verifier_state(state: env->cur_state, free_self: true);
env->cur_state = NULL;
}
while (!pop_stack(env, prev_insn_idx: NULL, insn_idx: NULL, pop_log: false));
if (!ret && pop_log)
bpf_vlog_reset(log: &env->log, new_pos: 0);
free_states(env);
return ret;

Same cleanup cycles are done in push_stack() and push_async_cb(),
both functions are only reachable from do_check_common() via
do_check() -> do_check_insn().

Hence, I think that cur state should not be freed in push_*()
functions and pop_stack() loop there is not needed.

> state->speculative does not make sense if the error path of push_stack()
> ran. In that case, `state->speculative &&
> error_recoverable_with_nospec(err)` as a whole should already never
> evaluate to true (because all cases where push_stack() fails also return
> a non-recoverable error -ENOMEM/-EFAULT).
>
> Alternatively to adding `state = env->cur_state` and `state &&`, turning
> the check around would avoid the use-after-free. However, I think your
> idea is better because it is more explicit compared to this:
>
> if (error_recoverable_with_nospec(err) && state->speculative) ...
>
> Does this make sense to you? If yes I can send the fix later today.

I think this flip makes perfect sense and should be done.

[...]

[Luis Gerhorst]

Eduard Zingerman <edd...@gmail.com> writes:

> On Wed, 2025-06-11 at 16:03 +0200, Luis Gerhorst wrote:
>> Eduard Zingerman <edd...@gmail.com> writes:
>>
>> > Either 'state = env->cur_state' is needed after 'do_check_insn()' or
>> > error path should not free env->cur_state (seems logical).

[...]

>> The latter might also be possible, but I guess it would require more
>> significant changes.
>
> do_check_common() has the following logic:
>
> out:
> /* check for NULL is necessary, since cur_state can be freed inside
> * do_check() under memory pressure.
> */
> if (env->cur_state) {
> free_verifier_state(state: env->cur_state, free_self: true);
> env->cur_state = NULL;
> }
> while (!pop_stack(env, prev_insn_idx: NULL, insn_idx: NULL, pop_log: false));
> if (!ret && pop_log)
> bpf_vlog_reset(log: &env->log, new_pos: 0);
> free_states(env);
> return ret;
>
> Same cleanup cycles are done in push_stack() and push_async_cb(),
> both functions are only reachable from do_check_common() via
> do_check() -> do_check_insn().
>
> Hence, I think that cur state should not be freed in push_*()
> functions and pop_stack() loop there is not needed.

Ah, yes I agree. I sent a patch separate from the fix [2].

>> state->speculative does not make sense if the error path of push_stack()
>> ran. In that case, `state->speculative &&
>> error_recoverable_with_nospec(err)` as a whole should already never
>> evaluate to true (because all cases where push_stack() fails also return
>> a non-recoverable error -ENOMEM/-EFAULT).

I noticed the was not really true yet, I had to fix the call for
sanitize_ptr_alu() to return -ENOMEM while [3] is not landed yet.

>> Alternatively to adding `state = env->cur_state` and `state &&`, turning
>> the check around would avoid the use-after-free. However, I think your
>> idea is better because it is more explicit compared to this:
>>
>> if (error_recoverable_with_nospec(err) && state->speculative) ...
>>
>> Does this make sense to you? If yes I can send the fix later today.
>
> I think this flip makes perfect sense and should be done.

I sent the fix [1], let me know if it is as desired.

[1] https://lore.kernel.org/all/20250611210728.266...@fau.de/
[2] https://lore.kernel.org/all/20250611211431.275...@fau.de/
[3] https://lore.kernel.org/all/20250603213232.339...@fau.de/

[Eduard Zingerman]

On Wed, 2025-06-11 at 05:36 -0700, syzbot wrote:

#syz test: g...@github.com:kernel-patches/bpf.git 974c296e39c3b2462bbf1f926d5a5db64399359f

[Eduard Zingerman]

On Wed, 2025-06-11 at 23:32 +0200, Luis Gerhorst wrote:

[...]

> I sent the fix [1], let me know if it is as desired.

Thank you for the patches, I'll take a look shortly.
For now I asked syzbot to test "bpf: Fix state use-after-free on push_stack() err"
(hope I did it correctly).

[...]

[syzbot]

Hello,

syzbot tried to test the proposed patch but the build/boot failed:

failed to checkout kernel repo g...@github.com:kernel-patches/bpf.git on commit 974c296e39c3b2462bbf1f926d5a5db64399359f: failed to run ["git" "fetch" "--force" "--tags" "73aba3dff4d9ee1b85deaa6efdc44b9d57c62235" "974c296e39c3b2462bbf1f926d5a5db64399359f"]: exit status 128
Host key verification failed.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.



Tested on:

commit: [unknown
git tree: g...@github.com:kernel-patches/bpf.git 974c296e39c3b2462bbf1f926d5a5db64399359f

kernel config: https://syzkaller.appspot.com/x/.config?x=76ed3656d7159e27
dashboard link: https://syzkaller.appspot.com/bug?extid=b5eb72a560b8149a1885
compiler:

Note: no patches were applied.