From nobody Sat May 2 09:14:37 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D9E9C43334 for ; Fri, 3 Jun 2022 17:35:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244909AbiFCRfX (ORCPT ); Fri, 3 Jun 2022 13:35:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41142 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344523AbiFCRfT (ORCPT ); Fri, 3 Jun 2022 13:35:19 -0400 Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69C3652E67 for ; Fri, 3 Jun 2022 10:35:16 -0700 (PDT) Received: by mail-pj1-x102b.google.com with SMTP id q12-20020a17090a304c00b001e2d4fb0eb4so12588447pjl.4 for ; Fri, 03 Jun 2022 10:35:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=wsGkporiWQeNHg4v1skX8MmfnPHmZ0ni6U0Bq1ri7HA=; b=a1sktZ6PwTtJdZaiQGYWK4MxHPhEr3vs5B0FyLaZOmVggg3/BzW2lFLLjb4vdmZ2CM 5tdYsRV4Tl1/efkLpIahk9F3SmrQ5a9wS/pdsPXgydC6nU6Aq+3QE1TAZjfLTiGszZJP wWHnm8d+dE53knPFb08NqEB0r1WGlH+WTqimSxLxIUH9Co088ymYV3G03+rGClHdUdLD 1VUHbZUkzEzNAdMDaY7zwRX0M1bCniqdHONnnQn3TCRwa/eTtQrTEPFkfZZ3Vb+9u42t Jnt6wzCrwFif404LEePDQmE5XXMq3Ly9WbIA1zCToHxYzdXKwCXsPqlzpQPM+YkmOoWQ fAEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=wsGkporiWQeNHg4v1skX8MmfnPHmZ0ni6U0Bq1ri7HA=; b=0iy3+4hokxk/NuwUesH3amiONdR0L9akRLScFIvmjbxAhzbJh4zn5U5WqtPF1t1g8s LDnVjwR7ygATUxgRTPotJGd6w8pm4ei11JFrKKHDvCBiYADpgr569eIqPovunrDo3NWj kJ/2kBXgY1GaQhztkz56waJU11nHwFGS5j8yQ67vh3l1buTuqNHoAOvCMiT2t8CUP1Tl nhynsdkiz1xZY2BLFE+j0F0Cby40kMUA8IE6bNLuva/8enxrbeyf0CJBNID+yl5ad8ZO vluiVjRduj59fo7JEeG/tPin6860wFKekWgx/J2RXUMeO2rKcyRayDLX8UFIA2uIMkJ9 PocQ== X-Gm-Message-State: AOAM5337j+cWZFtbk2Jif27/7RQuMHbcjnIKShNTYuqJSxl+F4lcFea2 01sGZC4FHLsTHNd2+zaMCBbhAw== X-Google-Smtp-Source: ABdhPJx88jfqJGvlh85IKtdIkpIo1pu4gJb5NXPiJ/0S9jrCIk/XxyxIGjdKNFq7q+cF1hUVJ66yCQ== X-Received: by 2002:a17:902:d2d1:b0:167:4c33:d5d3 with SMTP id n17-20020a170902d2d100b001674c33d5d3mr3799752plc.81.1654277715866; Fri, 03 Jun 2022 10:35:15 -0700 (PDT) Received: from localhost.localdomain ([50.39.160.154]) by smtp.gmail.com with ESMTPSA id x17-20020a056a000bd100b0051be1b4cfb5sm1730265pfu.5.2022.06.03.10.35.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Jun 2022 10:35:15 -0700 (PDT) From: Tadeusz Struk To: Tejun Heo Cc: Tadeusz Struk , Michal Koutny , Zefan Li , Johannes Weiner , Christian Brauner , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , cgroups@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org, stable@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+e42ae441c3b10acf9e9d@syzkaller.appspotmail.com Subject: [PATCH] cgroup: serialize css kill and release paths Date: Fri, 3 Jun 2022 10:34:55 -0700 Message-Id: <20220603173455.441537-1-tadeusz.struk@linaro.org> X-Mailer: git-send-email 2.36.1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Syzbot found a corrupted list bug scenario that can be triggered from cgroup_subtree_control_write(cgrp). The reproduces writes to cgroup.subtree_control file, which invokes: cgroup_apply_control_enable()->css_create()->css_populate_dir(), which then fails with a fault injected -ENOMEM. In such scenario the css_killed_work_fn will be en-queued via cgroup_apply_control_disable(cgrp)->kill_css(css), and bail out to cgroup_kn_unlock(). Then cgroup_kn_unlock() will call: cgroup_put(cgrp)->css_put(&cgrp->self), which will try to enqueue css_release_work_fn for the same css instance, causing a list_add corruption bug, as can be seen in the syzkaller report [1]. Fix this by synchronizing the css ref_kill and css_release jobs. css_release() function will check if the css_killed_work_fn() has been scheduled for the css and only en-queue the css_release_work_fn() if css_killed_work_fn wasn't already en-queued. Otherwise css_release() will set the CSS_REL_LATER flag for that css. This will cause the css_release_wo= rk_fn() work to be executed after css_killed_work_fn() is finished. Two scc flags have been introduced to implement this serialization mechaniz= m: * CSS_KILL_ENQED, which will be set when css_killed_work_fn() is en-queued= , and * CSS_REL_LATER, which, if set, will cause the css_release_work_fn() to be scheduled after the css_killed_work_fn is finished. There is also a new lock, which will protect the integrity of the css flags. [1] https://syzkaller.appspot.com/bug?id=3De26e54d6eac9d9fb50b221ec3e4627b3= 27465dbd Cc: Tejun Heo Cc: Michal Koutny Cc: Zefan Li Cc: Johannes Weiner Cc: Christian Brauner Cc: Alexei Starovoitov Cc: Daniel Borkmann Cc: Andrii Nakryiko Cc: Martin KaFai Lau Cc: Song Liu Cc: Yonghong Song Cc: John Fastabend Cc: KP Singh Cc: Cc: Cc: Cc: Cc: Reported-and-tested-by: syzbot+e42ae441c3b10acf9e9d@syzkaller.appspotmail.c= om Fixes: 8f36aaec9c92 ("cgroup: Use rcu_work instead of explicit rcu and work= item") Signed-off-by: Tadeusz Struk --- include/linux/cgroup-defs.h | 4 ++++ kernel/cgroup/cgroup.c | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h index 1bfcfb1af352..8dc8b4edb242 100644 --- a/include/linux/cgroup-defs.h +++ b/include/linux/cgroup-defs.h @@ -53,6 +53,8 @@ enum { CSS_RELEASED =3D (1 << 2), /* refcnt reached zero, released */ CSS_VISIBLE =3D (1 << 3), /* css is visible to userland */ CSS_DYING =3D (1 << 4), /* css is dying */ + CSS_KILL_ENQED =3D (1 << 5), /* kill work enqueued for the css */ + CSS_REL_LATER =3D (1 << 6), /* release needs to be done after kill */ }; =20 /* bits in struct cgroup flags field */ @@ -162,6 +164,8 @@ struct cgroup_subsys_state { */ int id; =20 + /* lock to protect flags */ + spinlock_t lock; unsigned int flags; =20 /* diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index 1779ccddb734..a0ceead4b390 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -5210,8 +5210,23 @@ static void css_release(struct percpu_ref *ref) struct cgroup_subsys_state *css =3D container_of(ref, struct cgroup_subsys_state, refcnt); =20 - INIT_WORK(&css->destroy_work, css_release_work_fn); - queue_work(cgroup_destroy_wq, &css->destroy_work); + spin_lock_bh(&css->lock); + + /* + * Check if the css_killed_work_fn work has been scheduled for this + * css and enqueue css_release_work_fn only if it wasn't. + * Otherwise set the CSS_REL_LATER flag, which will cause + * release to be enqueued after css_killed_work_fn is finished. + * This is to prevent list corruption by en-queuing two instance + * of the same work struct on the same WQ, namely cgroup_destroy_wq. + */ + if (!(css->flags & CSS_KILL_ENQED)) { + INIT_WORK(&css->destroy_work, css_release_work_fn); + queue_work(cgroup_destroy_wq, &css->destroy_work); + } else { + css->flags |=3D CSS_REL_LATER; + } + spin_unlock_bh(&css->lock); } =20 static void init_and_link_css(struct cgroup_subsys_state *css, @@ -5230,6 +5245,7 @@ static void init_and_link_css(struct cgroup_subsys_st= ate *css, INIT_LIST_HEAD(&css->rstat_css_node); css->serial_nr =3D css_serial_nr_next++; atomic_set(&css->online_cnt, 0); + spin_lock_init(&css->lock); =20 if (cgroup_parent(cgrp)) { css->parent =3D cgroup_css(cgroup_parent(cgrp), ss); @@ -5545,10 +5561,12 @@ int cgroup_mkdir(struct kernfs_node *parent_kn, con= st char *name, umode_t mode) */ static void css_killed_work_fn(struct work_struct *work) { - struct cgroup_subsys_state *css =3D + struct cgroup_subsys_state *css_killed, *css =3D container_of(work, struct cgroup_subsys_state, destroy_work); =20 mutex_lock(&cgroup_mutex); + css_killed =3D css; + css_killed->flags &=3D ~CSS_KILL_ENQED; =20 do { offline_css(css); @@ -5557,6 +5575,14 @@ static void css_killed_work_fn(struct work_struct *w= ork) css =3D css->parent; } while (css && atomic_dec_and_test(&css->online_cnt)); =20 + spin_lock_bh(&css->lock); + if (css_killed->flags & CSS_REL_LATER) { + /* If css_release work was delayed for the css enqueue it now. */ + INIT_WORK(&css_killed->destroy_work, css_release_work_fn); + queue_work(cgroup_destroy_wq, &css_killed->destroy_work); + css_killed->flags &=3D ~CSS_REL_LATER; + } + spin_unlock_bh(&css->lock); mutex_unlock(&cgroup_mutex); } =20 @@ -5566,10 +5592,13 @@ static void css_killed_ref_fn(struct percpu_ref *re= f) struct cgroup_subsys_state *css =3D container_of(ref, struct cgroup_subsys_state, refcnt); =20 + spin_lock_bh(&css->lock); if (atomic_dec_and_test(&css->online_cnt)) { + css->flags |=3D CSS_KILL_ENQED; INIT_WORK(&css->destroy_work, css_killed_work_fn); queue_work(cgroup_destroy_wq, &css->destroy_work); } + spin_unlock_bh(&css->lock); } =20 /** --=20 2.36.1