Re: [PATCH slab/for-next 14/14] mm/slab: do not set SLAB_MAY_ACCOUNT for all caches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/slab: missing SLAB_MAY_ACCOUNT in create_unique_id() causes sysfs collision
--

commit 51cadd40978b52beb867d85f64efe89a03c7cbf8
Author: Harry Yoo (Meta) <harry@kernel.org>

mm/slab: do not set SLAB_MAY_ACCOUNT for all caches

This commit stops setting SLAB_MAY_ACCOUNT unconditionally on all slab caches
when memcg kmem accounting is enabled, instead only setting it when
SLAB_ACCOUNT is specified or when the caller explicitly passes
SLAB_MAY_ACCOUNT.

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index e66c0778ee984..c69aac8bb0e41 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -359,12 +359,10 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>  		goto out_unlock;
>  	}
>  
> -	/*
> -	 * For now we assume any cache can be used with __GFP_ACCOUNT and thus
> -	 * may need to store objcg pointers for objects
> -	 */
> -	if (!mem_cgroup_kmem_disabled())
> +	if (flags & SLAB_ACCOUNT)
>  		flags |= SLAB_MAY_ACCOUNT;
> +	if (mem_cgroup_kmem_disabled())
> +		flags &= ~SLAB_MAY_ACCOUNT;

[Severity: High]
Does stopping __kmem_cache_create_args() in mm/slab_common.c from setting
SLAB_MAY_ACCOUNT on all caches cause a sysfs filename collision in
sysfs_slab_add() -> create_unique_id() for mergeable caches that differ only
by SLAB_MAY_ACCOUNT?

In mm/slab_common.c, SLAB_MERGE_SAME includes SLAB_MAY_ACCOUNT:

#define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
			 SLAB_CACHE_DMA32 | SLAB_ACCOUNT | SLAB_MAY_ACCOUNT)

Previously, all mergeable caches had SLAB_MAY_ACCOUNT set whenever
!mem_cgroup_kmem_disabled(). With this change, find_mergeable() in
mm/slab_common.c will keep a mergeable cache created with SLAB_MAY_ACCOUNT
(and without SLAB_ACCOUNT, such as fanotify_mnt_event, fanotify_path_event,
fanotify_perm_event, or buffer_head) separate from a same-sized mergeable
cache created without SLAB_MAY_ACCOUNT.

However, when sysfs_slab_add() in mm/slub.c registers mergeable caches in
sysfs, create_unique_id() does not encode SLAB_MAY_ACCOUNT in the generated
name:

mm/slub.c:create_unique_id() {
	...
	if (s->flags & SLAB_CACHE_DMA)
		*p++ = 'd';
	if (s->flags & SLAB_CACHE_DMA32)
		*p++ = 'D';
	if (s->flags & SLAB_RECLAIM_ACCOUNT)
		*p++ = 'a';
	if (s->flags & SLAB_CONSISTENCY_CHECKS)
		*p++ = 'F';
	if (s->flags & SLAB_ACCOUNT)
		*p++ = 'A';
	if (p != name + 1)
		*p++ = '-';
	p += snprintf(p, ID_STR_LENGTH - (p - name), "%07u", s->size);
	...
}

When CONFIG_MEMCG=y and cgroup kmem accounting is enabled, won't two
mergeable caches of the same size that differ only by SLAB_MAY_ACCOUNT
receive the exact same sysfs ID (for example, :0000056, :0000064, or
:a-0000104), causing kobject_init_and_add() in sysfs_slab_add() to fail with
-EEXIST and trigger a duplicate sysfs filename warning during boot?

>  
>  	/* Fail closed on bad usersize of useroffset values. */
>  	if (!IS_ENABLED(CONFIG_HARDENED_USERCOPY) ||

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021-memalloc-prof-dont-waste-slabobj-ext-metadata-v1-0-096800aa3330@kernel.org?part=14