Re: [PATCH v1] slab: support for compiler-assisted type-based slab cache partitioning

[Harry Yoo (Oracle)]

Now somewhat out-of-scope (or at least pre-existing) review comments
from Sashiko that I think are still worth mentioning...

> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -662,9 +662,20 @@ extern kmem_buckets kmalloc_caches[NR_KMALLOC_TYPES];
> -static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags, unsigned long caller)
> +static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags, kmalloc_token_t token)
>  {
>  	/*
>  	 * The most common case is KMALLOC_NORMAL, so test for it
> @@ -672,9 +683,11 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags, unsigne
>  	 */
>  	if (likely((flags & KMALLOC_NOT_NORMAL_BITS) == 0))

Sashiko pointed out KMALLOC_CGROUP caches are not partitioned [1]:
| Do allocations with the __GFP_ACCOUNT flag completely bypass typed
| and random partitioning? KMALLOC_NOT_NORMAL_BITS includes __GFP_ACCOUNT.

Right.

| If this bit is set, the code bypasses the partitioning logic and routes
| the allocation to the KMALLOC_CGROUP cache.

Right.

| Since user-controllable objects
| like msg_msg, file descriptors, and pipes are allocated with __GFP_ACCOUNT,

Right.

| they will all be clustered in the exact same unpartitioned cache.

Right.

>From security perspective do you think it'd be worthwhile to partition
KMALLOC_CGROUP caches? (I see at least few hundreds of users, unlike
KMALLOC_RECLAIM where there are only few users).

Another valid concern from Sashiko [1]:
| Does this leave reallocation functions like krealloc() and kvrealloc()
| without allocation token propagation?
|
| When an object is reallocated and requires memory expansion, the underlying
| generic SLUB code allocates a new buffer. Because the token macro is not
| applied to these realloc paths, __builtin_infer_alloc_token() evaluates
| locally on a generic size_t variable rather than the original type.

I think this is a valid point and worth addressing.

| This causes it to return the fallback token (0), which silently migrates the
| object from its isolated typed cache to the shared fallback cache
| (kmalloc-part-00) when resized.

[1] https://sashiko.dev/#/patchset/20260331111240.153913-1-elver%40google.com

-- 
Cheers,
Harry / Hyeonggon