From 8c8a5502183c724854afd2f143a72f7eb71b6fea Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Fri, 17 Jun 2016 12:23:59 -0400 Subject: cgroup: fix idr leak for the first cgroup root The valid cgroup hierarchy ID range includes 0, so we can't filter for positive numbers when freeing it, or it'll leak the first ID. No big deal, just disruptive when reading the code. The ID is freed during error handling and when the reference count hits zero, so the double-free test is not necessary; remove it. Signed-off-by: Johannes Weiner Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 86cb5c6..36fc0ff 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -1158,18 +1158,12 @@ static void cgroup_exit_root_id(struct cgroup_root *root) { lockdep_assert_held(&cgroup_mutex); - if (root->hierarchy_id) { - idr_remove(&cgroup_hierarchy_idr, root->hierarchy_id); - root->hierarchy_id = 0; - } + idr_remove(&cgroup_hierarchy_idr, root->hierarchy_id); } static void cgroup_free_root(struct cgroup_root *root) { if (root) { - /* hierarchy ID should already have been released */ - WARN_ON_ONCE(root->hierarchy_id); - idr_destroy(&root->cgroup_idr); kfree(root); } -- cgit v0.10.2 From d6ccc55e66ccdbc8ad0eeda14419f8eaccbc246b Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Fri, 17 Jun 2016 12:24:27 -0400 Subject: cgroup: remove unnecessary 0 check from css_from_id() css_idr allocation starts at 1, so index 0 will never point to an item. css_from_id() currently filters that before asking idr_find(), but idr_find() would also just return NULL, so this is not needed. Signed-off-by: Johannes Weiner Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 36fc0ff..78f6d18 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -6162,7 +6162,7 @@ struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry, struct cgroup_subsys_state *css_from_id(int id, struct cgroup_subsys *ss) { WARN_ON_ONCE(!rcu_read_lock_held()); - return id > 0 ? idr_find(&ss->css_idr, id) : NULL; + return idr_find(&ss->css_idr, id); } /** -- cgit v0.10.2 From e7e15b87f86d4a48c270b81cf027eafd801e5b89 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Tue, 21 Jun 2016 13:06:24 -0400 Subject: cgroup: allow NULL return from ss->css_alloc() cgroup core expected css_alloc to return an ERR_PTR value on failure and caused NULL deref if it returned NULL. It's an easy mistake to make from an alloc function and there's no ambiguity in what's being indicated. Update css_create() so that it interprets NULL return from css_alloc as -ENOMEM. Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 78f6d18..dd26e1b 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -5133,6 +5133,8 @@ static struct cgroup_subsys_state *css_create(struct cgroup *cgrp, lockdep_assert_held(&cgroup_mutex); css = ss->css_alloc(parent_css); + if (!css) + css = ERR_PTR(-ENOMEM); if (IS_ERR(css)) return css; -- cgit v0.10.2 From 135b8b37bd91cc82f83e98fca109b80375f5317e Mon Sep 17 00:00:00 2001 From: Kenny Yu Date: Tue, 21 Jun 2016 14:04:36 -0400 Subject: cgroup: Add pids controller event when fork fails because of pid limit This patch adds more visibility into the pids controller when the controller rejects a fork request. Whenever fork fails because the limit on the number of pids in the cgroup is reached, the controller will log this and also notify the newly added cgroups events file. The `max` key in the events file represents the number of times fork failed because of the pids controller. This change also logs only the first time the `max` event counter is incremented. This is to provide a hint to the user to understand why fork failed, as users are not yet used to seeing fork failures because of the pids controller. Signed-off-by: Kenny Yu Acked-by: Johannes Weiner cmpxchg.org> Signed-off-by: Tejun Heo diff --git a/kernel/cgroup_pids.c b/kernel/cgroup_pids.c index 303097b..9740ea6 100644 --- a/kernel/cgroup_pids.c +++ b/kernel/cgroup_pids.c @@ -49,6 +49,12 @@ struct pids_cgroup { */ atomic64_t counter; int64_t limit; + + /* Handle for "pids.events" */ + struct cgroup_file events_file; + + /* Number of times fork failed because limit was hit. */ + atomic64_t events_limit; }; static struct pids_cgroup *css_pids(struct cgroup_subsys_state *css) @@ -72,6 +78,7 @@ pids_css_alloc(struct cgroup_subsys_state *parent) pids->limit = PIDS_MAX; atomic64_set(&pids->counter, 0); + atomic64_set(&pids->events_limit, 0); return &pids->css; } @@ -213,10 +220,21 @@ static int pids_can_fork(struct task_struct *task) { struct cgroup_subsys_state *css; struct pids_cgroup *pids; + int err; css = task_css_check(current, pids_cgrp_id, true); pids = css_pids(css); - return pids_try_charge(pids, 1); + err = pids_try_charge(pids, 1); + if (err) { + /* Only log the first time events_limit is incremented. */ + if (atomic64_inc_return(&pids->events_limit) == 1) { + pr_info("cgroup: fork rejected by pids controller in "); + pr_cont_cgroup_path(task_cgroup(current, pids_cgrp_id)); + pr_cont("\n"); + } + cgroup_file_notify(&pids->events_file); + } + return err; } static void pids_cancel_fork(struct task_struct *task) @@ -288,6 +306,14 @@ static s64 pids_current_read(struct cgroup_subsys_state *css, return atomic64_read(&pids->counter); } +static int pids_events_show(struct seq_file *sf, void *v) +{ + struct pids_cgroup *pids = css_pids(seq_css(sf)); + + seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit)); + return 0; +} + static struct cftype pids_files[] = { { .name = "max", @@ -300,6 +326,12 @@ static struct cftype pids_files[] = { .read_s64 = pids_current_read, .flags = CFTYPE_NOT_ON_ROOT, }, + { + .name = "events", + .seq_show = pids_events_show, + .file_offset = offsetof(struct pids_cgroup, events_file), + .flags = CFTYPE_NOT_ON_ROOT, + }, { } /* terminate */ }; -- cgit v0.10.2 From 9f6870dd9790dd87da1d0cf9e43e60113f3a278d Mon Sep 17 00:00:00 2001 From: Kenny Yu Date: Tue, 21 Jun 2016 11:55:35 -0700 Subject: cgroup: Use lld instead of ld when printing pids controller events_limit The `events_limit` variable needs to be formatted with %lld and not %ld. This fixes the following warning discovered by kbuild test robot: kernel/cgroup_pids.c: In function 'pids_events_show': kernel/cgroup_pids.c:313:24: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=] seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit)); ^ tj: Added explicit (s64) cast as atomic64 switches between long long and long depending on 32 or 64. Signed-off-by: Kenny Yu Signed-off-by: Tejun Heo diff --git a/kernel/cgroup_pids.c b/kernel/cgroup_pids.c index 9740ea6..2bd6737 100644 --- a/kernel/cgroup_pids.c +++ b/kernel/cgroup_pids.c @@ -310,7 +310,7 @@ static int pids_events_show(struct seq_file *sf, void *v) { struct pids_cgroup *pids = css_pids(seq_css(sf)); - seq_printf(sf, "max %ld\n", atomic64_read(&pids->events_limit)); + seq_printf(sf, "max %lld\n", (s64)atomic64_read(&pids->events_limit)); return 0; } -- cgit v0.10.2 From 55094f57535831883b60776de5eb78c6bfb3c16d Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Tue, 19 Jul 2016 12:02:39 +0000 Subject: cgroup: remove duplicated include from cgroup.c Remove duplicated include. Signed-off-by: Wei Yongjun Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index dd26e1b..861995c 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -61,7 +61,6 @@ #include #include #include -#include #include /* -- cgit v0.10.2