lkml.org 
[lkml]   [2022]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH V2 1/1] mm/slub: release kobject if kobject_init_and_add failed in sysfs_slab_add
On Thu, Aug 11, 2022 at 03:18:44PM +0800, Xin Hao wrote:
> In kobject_init_and_add() function, the refcount is setted by calling
> kobject_init() function, regardless of whether the return value is zero
> or not, therefore, we must call kobject_del(&s->kobj) to prevent memory
> of s->kobj is leaked.

TL;DR: IIUC current code works just fine

After thinking more, I don't think the memory leak you said exist.
The space for s->kobj is freed in create_cache() when __kmem_cache_create() failed.

The situation here is:

create_cache() {
s = kmem_cache_alloc(kmem_cache, GFP_KERNEL)
err = __kmem_cache_create()
if (err)
goto out_free_cache;

out_free_cache:
kmem_cache_free(s) // s is freed here (including its kobject)
[...]
}

__kmem_cache_create() {
[...]

err = sysfs_slab_add();
if (err) {
__kmem_cache_release(s);
return err;
}
}

The primary goal of kobject_put() is to call release() function
of kobj_type (when reference becomes zero), which is kmem_cache_release().

kmem_cache_release() {
__kmem_cache_release(s)
kfree_const(s->name)
kmem_cache_free(s)
}

But when slab_sysfs_add() failed, __kmem_cache_release() and
create_cache() releases resources related to the cache.
(Also its name is freed in kmem_cache_create_usercopy().)

So IIUC current code works just fine!

>
> Signed-off-by: Xin Hao <xhao@linux.alibaba.com>
> ---
> mm/slub.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index b1281b8654bd..940a3f52e07c 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5981,19 +5981,18 @@ static int sysfs_slab_add(struct kmem_cache *s)
>
> err = sysfs_create_group(&s->kobj, &slab_attr_group);
> if (err)
> - goto out_del_kobj;
> + goto out;
>
> if (!unmergeable) {
> /* Setup first alias */
> sysfs_slab_alias(s, s->name);
> }
> + return err;
> out:
> if (!unmergeable)
> kfree(name);
> + kobject_put(&s->kobj);
> return err;
> -out_del_kobj:
> - kobject_del(&s->kobj);

So related resources are released in create_cache(), instead of by
calling kobject_put().

But kobject_del() is still needed because it should unlink kobject
hierarchy when kobject_add() succeeded but sysfs_create_group() failed!

> - goto out;
> }
>
> void sysfs_slab_unlink(struct kmem_cache *s)
> --
> 2.31.0
>

--
Thanks,
Hyeonggon

\
 
 \ /
  Last update: 2022-08-14 10:06    [W:0.058 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site