[adding Andrew to Cc]
On 03/07/2014 01:11 AM, Dave Jones wrote:
> The failure paths of sysfs_slab_add don't release the allocation of 'name'
> made by create_unique_id() a few lines above the context of the diff below.
> Create a common exit path to make it more obvious what needs freeing.
>
> Signed-off-by: Dave Jones <[email protected]>
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 25f14ad8f817..b2181d2682ac 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5197,17 +5197,13 @@ static int sysfs_slab_add(struct kmem_cache *s)
>
> s->kobj.kset = slab_kset;
> err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, "%s", name);
> - if (err) {
> - kobject_put(&s->kobj);
> - return err;
> - }
> + if (err)
> + goto err_out;
>
> err = sysfs_create_group(&s->kobj, &slab_attr_group);
> - if (err) {
> - kobject_del(&s->kobj);
> - kobject_put(&s->kobj);
> - return err;
> - }
> + if (err)
> + goto err_sysfs;
> +
> kobject_uevent(&s->kobj, KOBJ_ADD);
> if (!unmergeable) {
> /* Setup first alias */
> @@ -5215,6 +5211,13 @@ static int sysfs_slab_add(struct kmem_cache *s)
> kfree(name);
> }
> return 0;
> +
> +err_sysfs:
> + kobject_del(&s->kobj);
> +err_out:
> + kobject_put(&s->kobj);
> + kfree(name);
> + return err;
> }
We should free the name only if !unmergeable, because:
sysfs_slab_add():
if (unmergeable) {
/*
* Slabcache can never be merged so we can use the name proper.
* This is typically the case for debug situations. In that
* case we can catch duplicate names easily.
*/
sysfs_remove_link(&slab_kset->kobj, s->name);
name = s->name;
} else {
/*
* Create a unique name for the slab as a target
* for the symlinks.
*/
name = create_unique_id(s);
}
Since this function was modified in the mmotm tree, I would propose
something like this on top of mmotm to avoid further merge conflicts:
diff --git a/mm/slub.c b/mm/slub.c
index c6eb29d65847..f4ca525c05b0 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5214,25 +5214,19 @@ static int sysfs_slab_add(struct kmem_cache *s)
s->kobj.kset = cache_kset(s);
err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, "%s", name);
- if (err) {
- kobject_put(&s->kobj);
- return err;
- }
+ if (err)
+ goto out_put_kobj;
err = sysfs_create_group(&s->kobj, &slab_attr_group);
- if (err) {
- kobject_del(&s->kobj);
- kobject_put(&s->kobj);
- return err;
- }
+ if (err)
+ goto out_del_kobj;
#ifdef CONFIG_MEMCG_KMEM
if (is_root_cache(s)) {
s->memcg_kset = kset_create_and_add("cgroup", NULL, &s->kobj);
if (!s->memcg_kset) {
- kobject_del(&s->kobj);
- kobject_put(&s->kobj);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto out_del_kobj;
}
}
#endif
@@ -5241,9 +5235,16 @@ static int sysfs_slab_add(struct kmem_cache *s)
if (!unmergeable) {
/* Setup first alias */
sysfs_slab_alias(s, s->name);
- kfree(name);
}
- return 0;
+out:
+ if (!unmergeable)
+ kfree(name);
+ return err;
+out_del_kobj:
+ kobject_del(&s->kobj);
+out_put_kobj:
+ kobject_put(&s->kobj);
+ goto out;
}
static void sysfs_slab_remove(struct kmem_cache *s)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
