Re: [PATCH] slub: Don't call lockdep_unregister_key() for immature kmem_cache.

[Harry Yoo <harry.yoo@oracle.com>]

On Tue, Oct 07, 2025 at 08:07:54PM +0200, Vlastimil Babka wrote:
> On 10/7/25 18:11, Alexei Starovoitov wrote:
> > On Tue, Oct 7, 2025 at 12:46 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >>
> >> On 10/7/25 07:25, Kuniyuki Iwashima wrote:
> >> > syzbot reported the lockdep splat below in __kmem_cache_release(). [0]
> >> >
> >> > The problem is that __kmem_cache_release() could be called from
> >> > do_kmem_cache_create() before init_kmem_cache_cpus() registers
> >> > the lockdep key.
> > 
> > Hmm. If that's the case then alloc_kmem_cache_cpus() wasn't called yet
> > and s->cpu_slab == NULL, so any kmalloc/kmem_cache_alloc from
> > that slub will crash. The above sounds like it's a race condition,
> > but it's not. It's a weird error path in do_kmem_cache_create()
> 
> Indeed.
> 
> > due to syzbot pressure.
> > So the fix isn't quite right.
> > There is no need to sprinkle #ifdef all over the code.
> > 
> >> >
> >> > Let's move lockdep_unregister_key() from __kmem_cache_release()
> >> > to slab_kmem_cache_release() and do_kmem_cache_create().
> > 
> > I wouldn't.
> > 
> >> Thanks, added to slab/for-next-fixes
> > 
> > The following is imo much cleaner fix:
> 
> Yes much better, I'm changing it. Thanks!
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 584a5ff1828b..0a1fbddb77f8 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -7693,7 +7693,8 @@ void __kmem_cache_release(struct kmem_cache *s)
> >                 pcs_destroy(s);
> >  #ifndef CONFIG_SLUB_TINY
> >  #ifdef CONFIG_PREEMPT_RT
> > -       lockdep_unregister_key(&s->lock_key);
> > +       if (s->cpu_slab)
> > +               lockdep_unregister_key(&s->lock_key);
> >  #endif
> >         free_percpu(s->cpu_slab);
> >  #endif
> > 
> > compile tested only...

Looks good to me,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

-- 
Cheers,
Harry / Hyeonggon