Re: [PATCH v4 2/7] mm/slab: Give bucket caches the alignment of the caches they mirror

[Kees Cook]

On Mon, Sep 21, 2026 at 02:17:21PM +0100, Harry Yoo wrote:
> On Mon, Sep 21, 2026 at 12:58:13AM -0700, Kees Cook wrote:
> > A bucket set is created with kmem_cache_create_usercopy(..., align = 0),
> > so calculate_alignment() falls back to arch_slab_minalign(), typically 8
> > bytes. The general kmalloc caches it stands in for are created through
> > create_boot_cache(), which starts from ARCH_KMALLOC_MINALIGN and raises
> > it to the largest power-of-two divisor of the size:
> > 
> > 	if (flags & SLAB_KMALLOC)
> > 		align = max(align, 1U << (ffs(size) - 1));
> > 
> > This is only a problem when slab metadata is enabled with
> > CONFIG_KASAN=y, CONFIG_SLUB_DEBUG_ON=y, or "slab_debug=...", because
> > metadata changes the stride size off a power of two, for example:
> >
> > 	size 128:  bucket align=8 size=224  | kmalloc align=128  size=384
> > 	size 512:  bucket align=8 size=608  | kmalloc align=512  size=1536
> > 	size 2048: bucket align=8 size=2144 | kmalloc align=2048 size=6144
> 
> Hmm... I think what adds confusion here is that in new_kmalloc_cache()
> we adjust the size based on alignment, but in create_boot_cache() we
> don't do that. Perhaps let's make it consistent and move it to
> new_kmalloc_cache()?

Yeah, I really couldn't figure out what was "correct" here.

> > So bucket allocations will fail the IS_ALIGNED(p, ARCH_DMA_MINALIGN)
> > check, potentially creating problems for non-coherent DMA situation.
> 
> I was wondering "Why should they respect kmalloc alignment..." but yeah,
> It makes sense if the users were using kmalloc and depended on its
> alignment.

Right, it was a "visible" change between standard kmalloc and bucketed
kmalloc, so I figured the right action was to be (bug?) identical.

> Well, but that's already done in new_kmalloc_cache() and
> kmem_buckets_create() should already honor ARCH_KMALLOC_MINALIGN? 
> 
> The largest-power-of-two-divisor-alignment guarantee was introduced by
> commit ad59baa31695 ("slab, rust: extend kmalloc() alignment guarantees
> to remove Rust padding")
> 
> ...which makes me wonder what you're trying to fix here?

What Sashiko noticed was that alignment might not match under certain
configs, and then I verified it at runtime, and figured I'd best fix it
just on the basis that it was a difference from what a user might expect,
and it might be especially important for skb data.

> >  			if (WARN_ON(!cache_name))
> >  				goto fail;
> >  			(*b)[aligned_idx] = kmem_cache_create_usercopy(cache_name, size,
> > -					0, flags, cache_useroffset,
> > +					kmalloc_caches[KMALLOC_NORMAL][idx]->align,
> > +					flags, cache_useroffset,
> >  					cache_usersize, ctor);
> >  			kfree(cache_name);
> >  			if (WARN_ON(!(*b)[aligned_idx]))

It looks "obviously correct", but I probably failed to correctly
describe it. I'm happy to do whatever here.

-- 
Kees Cook