linux-kernel - Re: [RFC PATCH bpf-next 6/9] cgroup: add v1 support to cgroup_get_from

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <YnqwFuhncWiR3rjq@slm.duckdns.org>
Date:   Tue, 10 May 2022 08:33:58 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Yosry Ahmed <yosryahmed@...gle.com>
Cc:     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>, Hao Luo <haoluo@...gle.com>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Shuah Khan <shuah@...nel.org>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Michal Hocko <mhocko@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Greg Thelen <gthelen@...gle.com>,
        Shakeel Butt <shakeelb@...gle.com>,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org, cgroups@...r.kernel.org
Subject: Re: [RFC PATCH bpf-next 6/9] cgroup: add v1 support to
 cgroup_get_from_id()

On Tue, May 10, 2022 at 12:18:04AM +0000, Yosry Ahmed wrote:
> The current implementation of cgroup_get_from_id() only searches the
> default hierarchy for the given id. Make it compatible with cgroup v1 by
> looking through all the roots instead.
> 
> cgrp_dfl_root should be the first element in the list so there shouldn't
> be a performance impact for cgroup v2 users (in the case of a valid id).
> 
> Signed-off-by: Yosry Ahmed <yosryahmed@...gle.com>
> ---
>  kernel/cgroup/cgroup.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index af703cfcb9d2..12700cd21973 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -5970,10 +5970,16 @@ void cgroup_path_from_kernfs_id(u64 id, char *buf, size_t buflen)
>   */
>  struct cgroup *cgroup_get_from_id(u64 id)
>  {
> -	struct kernfs_node *kn;
> +	struct kernfs_node *kn = NULL;
>  	struct cgroup *cgrp = NULL;
> +	struct cgroup_root *root;
> +
> +	for_each_root(root) {
> +		kn = kernfs_find_and_get_node_by_id(root->kf_root, id);
> +		if (kn)
> +			break;
> +	}

I can't see how this can work. You're smashing together separate namespaces
and the same IDs can exist across multiple of these hierarchies. You'd need
a bigger surgery to make this work for cgroup1 which would prolly involve
complications around 32bit ino's and file handle support too, which I'm not
likely to ack, so please give it up on adding these things to cgroup1.

Nacked-by: Tejun Heo <tj@...nel.org>

Thanks.

-- 
tejun