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

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:

quoted

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.

quoted

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.

quoted

 			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