Skip to content

Commit 8b6850f

Browse files
committed
cgroup: Reorganize css_set_lock and kernfs path processing
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2077665 commit 46307fd Author: Michal Koutný <mkoutny@suse.com> Date: Mon, 10 Oct 2022 10:29:18 +0200 cgroup: Reorganize css_set_lock and kernfs path processing The commit 74e4b95 incorrectly wrapped kernfs_walk_and_get (might_sleep) under css_set_lock (spinlock). css_set_lock is needed by __cset_cgroup_from_root to ensure stable cset->cgrp_links but not for kernfs_walk_and_get. We only need to make sure that the returned root_cgrp won't be freed under us. This is given in the case of global root because it is static (cgrp_dfl_root.cgrp). When the root_cgrp is lower in the hierarchy, it is pinned by cgroup_ns->root_cset (and `current` task cannot switch namespace asynchronously so ns_proxy pins cgroup_ns). Note this reasoning won't hold for root cgroups in v1 hierarchies, therefore create a special-cased helper function just for the default hierarchy. Fixes: 74e4b95 ("cgroup: Honor caller's cgroup NS when resolving path") Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Michal Koutný <mkoutny@suse.com> Signed-off-by: Tejun Heo <tj@kernel.org> Signed-off-by: Waiman Long <longman@redhat.com>
1 parent eee9f1d commit 8b6850f

File tree

1 file changed

+27
-13
lines changed

1 file changed

+27
-13
lines changed

kernel/cgroup/cgroup.c

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,9 @@ static void cgroup_destroy_root(struct cgroup_root *root)
13781378
cgroup_free_root(root);
13791379
}
13801380

1381+
/*
1382+
* Returned cgroup is without refcount but it's valid as long as cset pins it.
1383+
*/
13811384
static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
13821385
struct cgroup_root *root)
13831386
{
@@ -1389,6 +1392,7 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
13891392
res_cgroup = cset->dfl_cgrp;
13901393
} else {
13911394
struct cgrp_cset_link *link;
1395+
lockdep_assert_held(&css_set_lock);
13921396

13931397
list_for_each_entry(link, &cset->cgrp_links, cgrp_link) {
13941398
struct cgroup *c = link->cgrp;
@@ -1400,6 +1404,7 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset,
14001404
}
14011405
}
14021406

1407+
BUG_ON(!res_cgroup);
14031408
return res_cgroup;
14041409
}
14051410

@@ -1422,23 +1427,36 @@ current_cgns_cgroup_from_root(struct cgroup_root *root)
14221427

14231428
rcu_read_unlock();
14241429

1425-
BUG_ON(!res);
14261430
return res;
14271431
}
14281432

1433+
/*
1434+
* Look up cgroup associated with current task's cgroup namespace on the default
1435+
* hierarchy.
1436+
*
1437+
* Unlike current_cgns_cgroup_from_root(), this doesn't need locks:
1438+
* - Internal rcu_read_lock is unnecessary because we don't dereference any rcu
1439+
* pointers.
1440+
* - css_set_lock is not needed because we just read cset->dfl_cgrp.
1441+
* - As a bonus returned cgrp is pinned with the current because it cannot
1442+
* switch cgroup_ns asynchronously.
1443+
*/
1444+
static struct cgroup *current_cgns_cgroup_dfl(void)
1445+
{
1446+
struct css_set *cset;
1447+
1448+
cset = current->nsproxy->cgroup_ns->root_cset;
1449+
return __cset_cgroup_from_root(cset, &cgrp_dfl_root);
1450+
}
1451+
14291452
/* look up cgroup associated with given css_set on the specified hierarchy */
14301453
static struct cgroup *cset_cgroup_from_root(struct css_set *cset,
14311454
struct cgroup_root *root)
14321455
{
1433-
struct cgroup *res = NULL;
1434-
14351456
lockdep_assert_held(&cgroup_mutex);
14361457
lockdep_assert_held(&css_set_lock);
14371458

1438-
res = __cset_cgroup_from_root(cset, root);
1439-
1440-
BUG_ON(!res);
1441-
return res;
1459+
return __cset_cgroup_from_root(cset, root);
14421460
}
14431461

14441462
/*
@@ -6049,9 +6067,7 @@ struct cgroup *cgroup_get_from_id(u64 id)
60496067
if (!cgrp)
60506068
return ERR_PTR(-ENOENT);
60516069

6052-
spin_lock_irq(&css_set_lock);
6053-
root_cgrp = current_cgns_cgroup_from_root(&cgrp_dfl_root);
6054-
spin_unlock_irq(&css_set_lock);
6070+
root_cgrp = current_cgns_cgroup_dfl();
60556071
if (!cgroup_is_descendant(cgrp, root_cgrp)) {
60566072
cgroup_put(cgrp);
60576073
return ERR_PTR(-ENOENT);
@@ -6619,10 +6635,8 @@ struct cgroup *cgroup_get_from_path(const char *path)
66196635
struct cgroup *cgrp = ERR_PTR(-ENOENT);
66206636
struct cgroup *root_cgrp;
66216637

6622-
spin_lock_irq(&css_set_lock);
6623-
root_cgrp = current_cgns_cgroup_from_root(&cgrp_dfl_root);
6638+
root_cgrp = current_cgns_cgroup_dfl();
66246639
kn = kernfs_walk_and_get(root_cgrp->kn, path);
6625-
spin_unlock_irq(&css_set_lock);
66266640
if (!kn)
66276641
goto out;
66286642

0 commit comments

Comments
 (0)