Re: [syzbot] [bpf?] WARNING in push_jmp_history

[Eduard Zingerman]

On Tue, 2024-10-08 at 01:43 -0700, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit d0a38fad51cc70ab3dd3c59b54d8079ac19220b9
> Author: Feng Tang <feng.tang@xxxxxxxxx>
> Date:   Wed Sep 11 06:45:34 2024 +0000
> 
>     mm/slub: Improve redzone check and zeroing for krealloc()
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11ddbb80580000
> start commit:   c02d24a5af66 Add linux-next specific files for 20241003
> git tree:       linux-next
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=13ddbb80580000
> console output: https://syzkaller.appspot.com/x/log.txt?x=15ddbb80580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=94f9caf16c0af42d
> dashboard link: https://syzkaller.appspot.com/bug?extid=7e46cdef14bf496a3ab4
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=10b82707980000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16f4c327980000
> 
> Reported-by: syzbot+7e46cdef14bf496a3ab4@xxxxxxxxxxxxxxxxxxxxxxxxx
> Fixes: d0a38fad51cc ("mm/slub: Improve redzone check and zeroing for krealloc()")
> 
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

There are two issues demonstrated by this repro:
- one is mm/slub related;
- another one is verification taking forever.

About the mm/slub related. Applying the following patch with
additional logging on top of commit d0a38fad51cc identified by syzbot:

------- 8< ------------------------------------------------------------
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9a7ed527e47e..c1582a6d1d33 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3494,7 +3494,9 @@ static int push_jmp_history(struct bpf_verifier_env *env, struct bpf_verifier_st
 
        cnt++;
        alloc_size = kmalloc_size_roundup(size_mul(cnt, sizeof(*p)));
+       printk("push_jmp_history: #1 cur->jmp_history=%p\n", cur->jmp_history);
        p = krealloc(cur->jmp_history, alloc_size, GFP_USER);
+       printk("push_jmp_history: #2 cur->jmp_history=%p\n", p);
        if (!p)
                return -ENOMEM;
        cur->jmp_history = p;
diff --git a/mm/slub.c b/mm/slub.c
index e0fb0a26c796..3f5b080ac1f5 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4627,7 +4627,7 @@ static inline struct kmem_cache *virt_to_cache(const void *obj)
        struct slab *slab;
 
        slab = virt_to_slab(obj);
-       if (WARN_ONCE(!slab, "%s: Object is not a Slab page!\n", __func__))
+       if (WARN_ONCE(!slab, "%s: Object %p is not a Slab page!\n", __func__, obj))
                return NULL;
        return slab->slab_cache;
 }
------------------------------------------------------------ >8 -------

Produces the following log:

 l1: [    2.942120] push_jmp_history: #2 cur->jmp_history=00000000a0f6f503
 l2: [    2.944445] push_jmp_history: #1 cur->jmp_history=00000000a0f6f503
 l3: [    2.944560] ------------[ cut here ]------------
 l4: [    2.944647] virt_to_cache: Object 00000000a0f6f503 is not a Slab page!
 l5: [    2.944765] WARNING: CPU: 0 PID: 145 at mm/slub.c:4630 krealloc_noprof (mm/slub.c:4630 mm/slub.c:4728 mm/slub.c:4813) 
 l6: [    2.944906] Modules linked in:
 l7: [    2.945134] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-2.fc40 04/01/2014
 l8: [    2.945285] RIP: 0010:krealloc_noprof (mm/slub.c:4630 mm/slub.c:4728 mm/slub.c:4813) 
               ...
l9:  [    2.952088] BUG: kernel NULL pointer dereference, address: 000000000000001c
l10: [    2.952171] #PF: supervisor read access in kernel mode
l11: [    2.952240] #PF: error_code(0x0000) - not-present page
l12: [    2.952309] PGD 105d51067 P4D 105d51067 PUD 1013d0067 PMD 0 
l13: [    2.952402] Oops: Oops: 0000 [#1] PREEMPT SMP KASAN NOPTI
l14: [    2.952611] Tainted: [W]=WARN
l15: [    2.952664] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-2.fc40 04/01/2014
l16: [    2.952794] RIP: 0010:krealloc_noprof (mm/slub.c:0 mm/slub.c:4729 mm/slub.c:4813) 

Lines l{1,2,4} show that address 0xa0f6f503 was first allocated by
krealloc and then krealloc failed to recognize it as such.

The warning at l3 is reported by virt_to_cache() called from
__do_krealloc():


   4715 static __always_inline __realloc_size(2) void *
   4716 __do_krealloc(const void *p, size_t new_size, gfp_t flags)
   4717 {
   4718         void *ret;
   4719         size_t ks;
   4720         int orig_size = 0;
   4721         struct kmem_cache *s;
   4722 
   4723         /* Check for double-free. */
   4724         if (likely(!ZERO_OR_NULL_PTR(p))) {
   4725                 if (!kasan_check_byte(p))
   4726                         return NULL;
   4727 
   4728                 s = virt_to_cache(p);
   4729                 orig_size = get_orig_size(s, (void *)p);
   4730                 ks = s->object_size;

When virt_to_cache() reports the warning it returns NULL.
Hence variable 's' at line 4728 is NULL and this causes null pointer
dereference at line 4730, reported at l9.

Lines 4725-4730 were changed by commit d0a38fad51cc identified by syzbot,
previously 'ks' was identified using other means.

Feng, this issue seem unrelated to BPF verifier, could you please take a look?

Best regards,
Eduard