linux-kernel - Re: [PATCH v2] cgroup: Reorganize css_set

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]

Date:   Wed, 5 Oct 2022 06:47:31 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Michal Koutný <mkoutny@...e.com>
Cc:     cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH v2] cgroup: Reorganize css_set_lock and kernfs path
 processing

Hello,

On Wed, Sep 28, 2022 at 01:33:16PM +0200, Michal Koutný wrote:
...
> I realized the pinning with reference taking won't really work
> generally. The code would get the reference within RCU read section, so
> it'd have to be cgroup_get_live() and if that fails there's not much to
> do.

Hmm... isn't current's root cgrp guaranteed to be alive? How would
cgroup_get_live() fail? Also, shouldn't cgroup_get() enough for path
walking?

> @@ -6673,8 +6678,8 @@ struct cgroup *cgroup_get_from_path(const char *path)
>  
>  	spin_lock_irq(&css_set_lock);
>  	root_cgrp = current_cgns_cgroup_from_root(&cgrp_dfl_root);
> -	kn = kernfs_walk_and_get(root_cgrp->kn, path);
>  	spin_unlock_irq(&css_set_lock);
> +	kn = kernfs_walk_and_get(root_cgrp->kn, path);

If you really wanna do it this way, can you please add a detailed comment
here why this is safe? But I'd prefer just doing a strightforward ref
inc/dec around it.

Thanks.

-- 
tejun