| 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
| ||
|
Message-Id: <20220523212724.233314-1-tadeusz.struk@linaro.org>
Date: Mon, 23 May 2022 14:27:24 -0700
From: Tadeusz Struk <tadeusz.struk@...aro.org>
To: Tejun Heo <tj@...nel.org>
Cc: Tadeusz Struk <tadeusz.struk@...aro.org>,
Zefan Li <lizefan.x@...edance.com>,
Johannes Weiner <hannes@...xchg.org>,
Christian Brauner <brauner@...nel.org>,
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>, cgroups@...r.kernel.org,
netdev@...r.kernel.org, bpf@...r.kernel.org,
stable@...r.kernel.org, linux-kernel@...r.kernel.org,
syzbot+e42ae441c3b10acf9e9d@...kaller.appspotmail.com
Subject: [PATCH v2] cgroups: separate destroy_work into two separate wq
Syzbot found a corrupted list bug scenario that can be triggered from
cgroup css_create(). The reproduces writes to cgroup.subtree_control
file, which invokes cgroup_apply_control_enable(), css_create(), and
css_populate_dir(), which then randomly fails with a fault injected -ENOMEM.
In such scenario the css_create() error path rcu enqueues css_free_rwork_fn
work for an css->refcnt initialized with css_release() destructor,
and there is a chance that the css_release() function will be invoked
for a cgroup_subsys_state, for which a destroy_work has already been
queued via css_create() error path. This causes a list_add corruption
as can be seen in the syzkaller report [1].
This can be fixed by separating the css_release and ref_kill paths
to work with two separate work_structs.
[1] https://syzkaller.appspot.com/bug?id=e26e54d6eac9d9fb50b221ec3e4627b327465dbd
Cc: Tejun Heo <tj@...nel.org>
Cc: Zefan Li <lizefan.x@...edance.com>
Cc: Johannes Weiner <hannes@...xchg.org>
Cc: Christian Brauner <brauner@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>
Cc: Daniel Borkmann <daniel@...earbox.net>
Cc: Andrii Nakryiko <andrii@...nel.org>
Cc: Martin KaFai Lau <kafai@...com>
Cc: Song Liu <songliubraving@...com>
Cc: Yonghong Song <yhs@...com>
Cc: John Fastabend <john.fastabend@...il.com>
Cc: KP Singh <kpsingh@...nel.org>
Cc: <cgroups@...r.kernel.org>
Cc: <netdev@...r.kernel.org>
Cc: <bpf@...r.kernel.org>
Cc: <stable@...r.kernel.org>
Cc: <linux-kernel@...r.kernel.org>
Reported-and-tested-by: syzbot+e42ae441c3b10acf9e9d@...kaller.appspotmail.com
Fixes: 8f36aaec9c92 ("cgroup: Use rcu_work instead of explicit rcu and work item")
Signed-off-by: Tadeusz Struk <tadeusz.struk@...aro.org>
---
v2: Add a separate work_struct for the css_ref_kill path instead of
checking if a work has already been enqueued.
---
include/linux/cgroup-defs.h | 5 +++--
kernel/cgroup/cgroup.c | 14 +++++++-------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 1bfcfb1af352..92b0c5e8c472 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -178,8 +178,9 @@ struct cgroup_subsys_state {
*/
atomic_t online_cnt;
- /* percpu_ref killing and RCU release */
- struct work_struct destroy_work;
+ /* percpu_ref killing, css release, and RCU release work structs */
+ struct work_struct release_work;
+ struct work_struct killed_ref_work;
struct rcu_work destroy_rwork;
/*
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index adb820e98f24..3e00a793e15d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5099,7 +5099,7 @@ static struct cftype cgroup_base_files[] = {
* css_free_work_fn().
*
* It is actually hairier because both step 2 and 4 require process context
- * and thus involve punting to css->destroy_work adding two additional
+ * and thus involve punting to css->release_work adding two additional
* steps to the already complex sequence.
*/
static void css_free_rwork_fn(struct work_struct *work)
@@ -5154,7 +5154,7 @@ static void css_free_rwork_fn(struct work_struct *work)
static void css_release_work_fn(struct work_struct *work)
{
struct cgroup_subsys_state *css =
- container_of(work, struct cgroup_subsys_state, destroy_work);
+ container_of(work, struct cgroup_subsys_state, release_work);
struct cgroup_subsys *ss = css->ss;
struct cgroup *cgrp = css->cgroup;
@@ -5210,8 +5210,8 @@ static void css_release(struct percpu_ref *ref)
struct cgroup_subsys_state *css =
container_of(ref, struct cgroup_subsys_state, refcnt);
- INIT_WORK(&css->destroy_work, css_release_work_fn);
- queue_work(cgroup_destroy_wq, &css->destroy_work);
+ INIT_WORK(&css->release_work, css_release_work_fn);
+ queue_work(cgroup_destroy_wq, &css->release_work);
}
static void init_and_link_css(struct cgroup_subsys_state *css,
@@ -5546,7 +5546,7 @@ int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, umode_t mode)
static void css_killed_work_fn(struct work_struct *work)
{
struct cgroup_subsys_state *css =
- container_of(work, struct cgroup_subsys_state, destroy_work);
+ container_of(work, struct cgroup_subsys_state, killed_ref_work);
mutex_lock(&cgroup_mutex);
@@ -5567,8 +5567,8 @@ static void css_killed_ref_fn(struct percpu_ref *ref)
container_of(ref, struct cgroup_subsys_state, refcnt);
if (atomic_dec_and_test(&css->online_cnt)) {
- INIT_WORK(&css->destroy_work, css_killed_work_fn);
- queue_work(cgroup_destroy_wq, &css->destroy_work);
+ INIT_WORK(&css->killed_ref_work, css_killed_work_fn);
+ queue_work(cgroup_destroy_wq, &css->killed_ref_work);
}
}
--
2.36.1
Powered by blists - more mailing lists