Re: [PATCH vfs/for-next v6] cgroup: fix top cgroup refcnt leak

[Andrei Vagin <avagin@gmail.com>]

On Thu, Jan 03, 2019 at 08:32:29AM +0000, Al Viro wrote:
> On Wed, Jan 02, 2019 at 07:54:26PM -0800, Andrei Vagin wrote:
> 
> [I'm thoroughly sick of refcounting in that thing, TBH ;-/]

feel your pain...

> 
> > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> > index a19f0fec9d82..fe67b5e81f9a 100644
> > --- a/kernel/cgroup/cgroup.c
> > +++ b/kernel/cgroup/cgroup.c
> > @@ -2019,7 +2019,7 @@ int cgroup_do_get_tree(struct fs_context *fc)
> >  
> >  	ret = kernfs_get_tree(fc);
> >  	if (ret < 0)
> > -		goto out_cgrp;
> > +		return ret;
> 
> Why does that case avoid needing cgroup_put()?  Note, BTW, that we
> also have this:

The origin patch which added this cgroup_put says that it is needed
because mount() and kill_sb() is not a one-to-one match. sb is created
in kernfs_get_tree(), so I decided that this cgroup_put() is needed only
after kernfs_get_tree().

commit c6b3d5bcd67c75961a1e8b9564d1475c0f194a84
Author: Li Zefan <lizefan@huawei.com>
Date:   Fri Apr 4 17:14:41 2014 +0800

cgroup: fix top cgroup refcnt leak

As mount() and kill_sb() is not a one-to-one match, If we mount the same
cgroupfs in serveral mount points, and then umount all of them, kill_sb()
will be called only once.


> /*
>  * Destroy a cgroup filesystem context.
>  */
> static void cgroup_fs_context_free(struct fs_context *fc)
> {
>         struct cgroup_fs_context *ctx = cgroup_fc2context(fc);
> 
>         kfree(ctx->name);
>         kfree(ctx->release_agent);
>         if (ctx->root)
>                 cgroup_put(&ctx->root->cgrp);
>         put_cgroup_ns(ctx->ns);
>         kernfs_free_fs_context(fc);
>         kfree(ctx);
> }
> 
> which also needs to be taken into account.

we have unconditional cgroup_get in cgroup1_get_tree() to match this
put, but we need to check error paths.