linux-kernel - Re: [RFC PATCH 3/3] cgroup: Do not take css_set_lock in cgroup_show

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:   Fri, 5 May 2023 19:32:40 +0200
From:   Michal Koutný <mkoutny@...e.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        cgroups@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Dave Chinner <dchinner@...hat.com>,
        Rik van Riel <riel@...riel.com>,
        Jiri Wiesner <jwiesner@...e.de>
Subject: Re: [RFC PATCH 3/3] cgroup: Do not take css_set_lock in
 cgroup_show_path

On Fri, May 05, 2023 at 05:45:58AM -1000, Tejun Heo <tj@...nel.org> wrote:
> > There are three relevant nodes for each cgroupfs entry:
> > 
> >         R ... cgroup hierarchy root
> >         M ... mount root
> >         C ... reader's cgroup NS root
> > 
> > mountinfo is supposed to show path from C to M.
> 
> At least for cgroup2, the path from C to M isn't gonna change once NS is
> established, right?

Right. Although, the argument about M (when C above M or when C and M in
different subtrees) implicitly relies on the namespace_sem.

> Nothing can be moved or renamed while the NS root is there. If so,
> can't we just cache the formatted path and return the same thing
> without any locking?

Here I find the caching complexity not worth it for v2 only (also C is
in the eye of the beholder) -- because then css_set_lock can be dropped
from cgroup_show_path with simpler reasoning.

(Sadly, the bigger benefit would be on hybrid setups multiplied by the
number of v1 hierarchies listed.)

(OTOH, the caching argument and weights for it may be different for
/proc/$pid/cgroup.)

> The proposed changes seem a bit too brittle to me.

OK, I'll look into separate cgroup_show_path and cgroup1_show_path
approach.

Thanks,
Michal

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)