Re: [PATCH v2] mm/slab: fix probable issue of dentries registration under /sys/kernel/slab

[Harry Yoo <harry@kernel.org>]

On 5/21/26 8:15 PM, Vladimir Zapolskiy wrote:
> L2TP/IP and L2TP/IPv6 protocol names contain a slash symbol, however these
> names are blindly used as symlinks to slab cache objects registered under
> /sys/kernel/slab. This kind of symlink creation is successful, but its
> dentry is obviously broken, as well it breaks the access to the list of
> /sys/kernel/slab dentries.
> 
> Likely L2TP protocol renames cannot be done, since the defined protocol
> names are exposed over /proc/net/protocols for years, but the symlink
> names can be renamed, because they are yet to be properly created, and
> this should be eventually done by this change.
> 
> The problem manifests itself, if CONFIG_L2TP_IP build symbol is selected.
> 
> Fixes: 81819f0fc8285 ("SLUB core")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---

Ideally we should reject '/' and let users avoid having it in the slab 
cache name instead of silently replacing it with another character (you 
don't have to rename protocol to rename the slab cache name!).

...but actually, I see kobject already replaces '/' with '!':

/**
  * kobject_set_name_vargs() - Set the name of a kobject.
  * @kobj: struct kobject to set the name of
  * @fmt: format string used to build the name
  * @vargs: vargs to format the string.
  */
int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
                                   va_list vargs)
{
         const char *s;

         if (kobj->name && !fmt)
                 return 0;

         s = kvasprintf_const(GFP_KERNEL, fmt, vargs);
         if (!s)
                 return -ENOMEM;

         /*
          * ewww... some of these buggers have '/' in the name ... If
          * that's the case, we need to make sure we have an actual 

* allocated copy to modify, since kvasprintf_const may have
          * returned something from .rodata.
          */

(Sorry for broken formatting my email setup is bit messed now)

But sysfs symlink doesn't handle this.

Any thoughts?

[+Cc DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS folks]

> Changes from v2 to v1:
> * added slash symbol replacement to __kmem_cache_create_args() as well,
>    many thanks to Hao and Harry for review.
> 
> Link to v1:
> * https://lore.kernel.org/linux-mm/8a80496b-1568-4a0b-a878-836c27d19ad5@kernel.org/
> 
>   mm/slab_common.c | 23 +++++++++++++++++++----
>   mm/slub.c        | 23 ++++++++++++++++++++++-
>   2 files changed, 41 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index d5a70a831a2a..b9930521496d 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -366,16 +366,31 @@ struct kmem_cache *__kmem_cache_create_args(const char *name,
>   		    object_size - args->usersize < args->useroffset))
>   		args->usersize = args->useroffset = 0;
>   
> -	s = __kmem_cache_alias(name, object_size, flags, args);
> -	if (s)
> -		goto out_unlock;
> -
>   	cache_name = kstrdup_const(name, GFP_KERNEL);
>   	if (!cache_name) {
>   		err = -ENOMEM;
>   		goto out_unlock;
>   	}
>   
> +	if (strchr(cache_name, '/')) {
> +		char *n;
> +
> +		n = kstrdup(cache_name, GFP_KERNEL);
> +		kfree_const(cache_name);
> +		if (!n) {
> +			err = -ENOMEM;
> +			goto out_unlock;
> +		}
> +
> +		cache_name = strreplace(n, '/', '_');
> +	}
> +
> +	s = __kmem_cache_alias(cache_name, object_size, flags, args);
> +	if (s) {
> +		kfree_const(cache_name);
> +		goto out_unlock;
> +	}
> +
>   	args->align = calculate_alignment(flags, args->align, object_size);
>   	s = create_cache(cache_name, object_size, args, flags);
>   	if (IS_ERR(s)) {
> diff --git a/mm/slub.c b/mm/slub.c
> index 0baa906f39ab..843bed864a7f 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -9634,6 +9634,7 @@ static struct saved_alias *alias_list;
>   int sysfs_slab_alias(struct kmem_cache *s, const char *name)
>   {
>   	struct saved_alias *al;
> +	const char *al_name;
>   
>   	if (slab_state == FULL) {
>   		/*
> @@ -9652,8 +9653,27 @@ int sysfs_slab_alias(struct kmem_cache *s, const char *name)
>   	if (!al)
>   		return -ENOMEM;
>   
> +	al_name = kstrdup_const(name, GFP_KERNEL);
> +	if (!al_name) {
> +		kfree(al);
> +		return -ENOMEM;
> +	}
> +
> +	if (strchr(al_name, '/')) {
> +		char *n;
> +
> +		n = kstrdup(al_name, GFP_KERNEL);
> +		kfree_const(al_name);
> +		if (!n) {
> +			kfree(al);
> +			return -ENOMEM;
> +		}
> +
> +		al_name = strreplace(n, '/', '_');
> +	}
> +
>   	al->s = s;
> -	al->name = name;
> +	al->name = al_name;
>   	al->next = alias_list;
>   	alias_list = al;
>   	kmsan_unpoison_memory(al, sizeof(*al));
> @@ -9691,6 +9711,7 @@ static int __init slab_sysfs_init(void)
>   		if (err)
>   			pr_err("SLUB: Unable to add boot slab alias %s to sysfs\n",
>   			       al->name);
> +		kfree_const(al->name);
>   		kfree(al);
>   	}
>   

-- 
Cheers,
Harry / Hyeonggon