linux-kernel - Re: [PATCH v2] cgroup: Kill the parent controller when its last child is killed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 21 Apr 2022 12:37:46 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Bui Quang Minh <minhquangbui99@...il.com>
Cc:     Michal Koutný <mkoutny@...e.com>,
        cgroups@...r.kernel.org, kernel test robot <lkp@...el.com>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH v2] cgroup: Kill the parent controller when its last
 child is killed

On Tue, Apr 05, 2022 at 09:58:01PM +0700, Bui Quang Minh wrote:
> @@ -5152,12 +5153,28 @@ static void css_release_work_fn(struct work_struct
> *work)
>                 container_of(work, struct cgroup_subsys_state,
> destroy_work);
>         struct cgroup_subsys *ss = css->ss;
>         struct cgroup *cgrp = css->cgroup;
> +       struct cgroup *parent = cgroup_parent(cgrp);
> 
>         mutex_lock(&cgroup_mutex);
> 
>         css->flags |= CSS_RELEASED;
>         list_del_rcu(&css->sibling);
> 
> +       /*
> +        * If parent doesn't have any children, start killing it.
> +        * And don't kill the default root.
> +        */
> +       if (parent && list_empty(&parent->self.children) &&
> +           parent->flags & CGRP_UMOUNT &&
> +           parent != &cgrp_dfl_root.cgrp &&
> +           !percpu_ref_is_dying(&parent->self.refcnt)) {
> +#ifdef CONFIG_CGROUP_BPF
> +               if (!percpu_ref_is_dying(&cgrp->bpf.refcnt))
> +                       cgroup_bpf_offline(parent);
> +#endif
> +               percpu_ref_kill(&parent->self.refcnt);
> +       }
> +
>         if (ss) {
>                 /* css release path */
>                 if (!list_empty(&css->rstat_css_node)) {
> 
> The idea is to set a flag in the umount path, in the rmdir it will destroy
> the css in case its direct parent is umounted, no recursive here. This is
> just an incomplete example, we may need to reset that flag when remounting.

I'm generally against adding complexities for this given that it's never
gonna be actually reliable. If adding one liner flush_workqueue makes life
easier in some cases, why not? But the root cause is something which can't
be solved from messing with release / umount paths and something we decided
against supporting.

Thanks.

-- 
tejun