[PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state

BPF List
 help / color / mirror / Atom feed

* [PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state
@ 2026-11-12 17:27 Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 1/3] bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY Shakeel Butt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Shakeel Butt @ 2026-11-12 17:27 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Song Liu, Yonghong Song, John Fastabend, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Roman Gushchin, Amery Hung,
	Tejun Heo, Johannes Weiner, Michal Koutný, Meta kernel team,
	cgroups, linux-kernel

In the Meta fleet, BPF_PROG_QUERY callers, link-info readers and the
cgroup bpf release work often wait minutes for cgroup_mutex. The holder is
almost always a cgroup mkdir or rmdir stuck on kernfs_rwsem. In one day,
hung-task reports showed this on 1,075 hosts (query), 94 hosts (link info)
and 458 hosts (release work).

Patch 3 gives the cgroup bpf state its own mutex, so these paths no longer
take cgroup_mutex. Attach, detach and replace still take cgroup_mutex
first, because they walk the descendants.

Patches 1 and 2 fix two old races. Attach and query get the cgroup from an
fd and take the lock later, without checking whether the cgroup's bpf
state was released in between. An effective query then reads freed memory
(patch 1). Attach leaks the program, or with cgroup storage writes to a
freed cgroup later (patch 2). Patch 3 makes the attach race easy to hit,
so patch 2 must come before it. Both fixes have Fixes tags.

Testing, in an x86-64 VM with lockdep, PROVE_RCU and KASAN:

- The 41 cgroup bpf selftests pass. A 60-second stress of mkdir/rmdir,
  attach/detach, links, queries, fdinfo, storage map free and sockets in
  dying cgroups shows no lockdep or KASAN reports.
- Debug-only delays reproduce both races. Without the fixes, KASAN reports
  use-after-free in cgroup_bpf_prog_query() and, with patch 3, in
  cgroup_storage_map_free(), and BPF_PROG_ATTACH leaks the program. With
  the fixes, attach and query return -ENOENT.
- While a mkdir holds cgroup_mutex for 5 seconds, query and link info
  return in under a millisecond instead of 4.6 seconds. Attach and detach
  still wait.
- Builds with CGROUP_BPF=n and with BPF_LSM=n.

Shakeel Butt (3):
  bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY
  bpf, cgroup: Don't attach to a removed cgroup
  bpf, cgroup: Protect cgroup_bpf state with its own mutex

 include/linux/bpf-cgroup-defs.h |   1 +
 include/linux/bpf-cgroup.h      |   3 +
 kernel/bpf/cgroup.c             | 158 +++++++++++++++++++++++---------
 kernel/bpf/local_storage.c      |   8 +-
 4 files changed, 123 insertions(+), 47 deletions(-)

base-commit: 0fb18ce8a2e9de830433923cf65838933d615359
-- 
2.53.0-Meta

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 1/3] bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY
  2026-11-12 17:27 [PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state Shakeel Butt
@ 2026-11-12 17:27 ` Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 2/3] bpf, cgroup: Don't attach to a removed cgroup Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex Shakeel Butt
  2 siblings, 0 replies; 6+ messages in thread
From: Shakeel Butt @ 2026-11-12 17:27 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Song Liu, Yonghong Song, John Fastabend, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Roman Gushchin, Amery Hung,
	Tejun Heo, Johannes Weiner, Michal Koutný, Meta kernel team,
	cgroups, linux-kernel

BPF_PROG_QUERY gets the cgroup from an fd before it takes cgroup_mutex.
If the cgroup is removed in between and its bpf refcount drops to zero,
cgroup_bpf_release() can run first.  It frees the effective arrays but
leaves the pointers, so a BPF_F_QUERY_EFFECTIVE query then reads freed
memory.

The window is small because the release work also takes cgroup_mutex.

Check under the lock that the bpf refcount isn't zero, and return
-ENOENT if it is, the error the fd lookup gives for a removed cgroup.
The release work only runs after the count hits zero, and it takes the
same lock.

Reproduced in a VM with a debug-only delay between the fd lookup and the
lock: KASAN reports a use-after-free in cgroup_bpf_prog_query().  With
this patch, the query returns -ENOENT.

Fixes: 4bfc0bb2c60e ("bpf: decouple the lifetime of cgroup_bpf from cgroup itself")
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 kernel/bpf/cgroup.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 57a785241d42..9258c2669fa8 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1446,6 +1446,14 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
 	if (effective_query && prog_attach_flags)
 		return -EINVAL;
 
+	/*
+	 * cgroup_bpf_release() frees the effective arrays once the bpf
+	 * refcount is zero.  It takes this lock too, so a nonzero count means
+	 * the arrays are still valid.
+	 */
+	if (percpu_ref_is_zero(&cgrp->bpf.refcnt))
+		return -ENOENT;
+
 	if (type == BPF_STRUCT_OPS) {
 		u32 type_id = attr->query.type_id;
 
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 2/3] bpf, cgroup: Don't attach to a removed cgroup
  2026-11-12 17:27 [PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 1/3] bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY Shakeel Butt
@ 2026-11-12 17:27 ` Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex Shakeel Butt
  2 siblings, 0 replies; 6+ messages in thread
From: Shakeel Butt @ 2026-11-12 17:27 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Song Liu, Yonghong Song, John Fastabend, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Roman Gushchin, Amery Hung,
	Tejun Heo, Johannes Weiner, Michal Koutný, Meta kernel team,
	cgroups, linux-kernel

Attach gets the cgroup from an fd before it takes cgroup_mutex.  If the
cgroup is removed in between and cgroup_bpf_release() runs first, attach
adds a program that never runs:

- With BPF_PROG_ATTACH the program leaks, since nothing can detach it.
- With a link that uses cgroup storage, the storage stays on the freed
  cgroup's list, and freeing the map later writes to the freed cgroup.
- A cgroup struct_ops link is attached but never called.

The window is small today because the release work also waits for
cgroup_mutex.  The next patch stops that, which makes the race easy to
hit.

Check cgroup_is_dead() under the lock and return -ENOENT, the error the
fd lookup gives for a removed cgroup.  rmdir marks the cgroup dead under
cgroup_mutex before it starts the release, so the check is exact.

Reproduced in a VM with debug-only delays.  With the next patch applied,
a link attach queued behind a slow mkdir lands on the released cgroup,
and freeing the map then hits a KASAN use-after-free in
cgroup_storage_map_free().  BPF_PROG_ATTACH leaks the program.  With
this patch, attach returns -ENOENT.

Fixes: 4bfc0bb2c60e ("bpf: decouple the lifetime of cgroup_bpf from cgroup itself")
Fixes: 369d9dcd8fb8 ("bpf: Add infrastructure to support attaching struct_ops to cgroups")
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 kernel/bpf/cgroup.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 9258c2669fa8..334237096759 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -988,6 +988,13 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 		/* replace_prog implies BPF_F_REPLACE, and vice versa */
 		return -EINVAL;
 
+	/*
+	 * The cgroup may have been removed since the caller looked it up.
+	 * Then its bpf state may be freed, and nothing would detach this.
+	 */
+	if (cgroup_is_dead(cgrp))
+		return -ENOENT;
+
 	atype = bpf_cgroup_atype_find(type, new_prog->aux->attach_btf_id);
 	if (atype < 0)
 		return -EINVAL;
@@ -3170,6 +3177,12 @@ int cgroup_bpf_struct_ops_attach(struct bpf_map *map, const union bpf_attr *attr
 
 	cgroup_lock();
 
+	/* See the cgroup_is_dead() check in __cgroup_bpf_attach() */
+	if (cgroup_is_dead(cgrp)) {
+		err = -ENOENT;
+		goto unlock;
+	}
+
 	if (attr->link_create.cgroup.expected_revision &&
 	    attr->link_create.cgroup.expected_revision != cgrp->bpf.revisions[atype]) {
 		err = -ESTALE;
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex
  2026-11-12 17:27 [PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 1/3] bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY Shakeel Butt
  2026-11-12 17:27 ` [PATCH bpf-next 2/3] bpf, cgroup: Don't attach to a removed cgroup Shakeel Butt
@ 2026-11-12 17:27 ` Shakeel Butt
  2026-11-12 17:41   ` sashiko-bot
  2 siblings, 1 reply; 6+ messages in thread
From: Shakeel Butt @ 2026-11-12 17:27 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Song Liu, Yonghong Song, John Fastabend, Jiri Olsa,
	Emil Tsalapatis, Ihor Solodrai, Roman Gushchin, Amery Hung,
	Tejun Heo, Johannes Weiner, Michal Koutný, Meta kernel team,
	cgroups, linux-kernel

In the Meta fleet, tasks that only read or free cgroup bpf state often
wait minutes for cgroup_mutex:

- In one day, hung-task reports (blocked over 120 seconds) showed this
  for BPF_PROG_QUERY on 1,075 hosts, the cgroup bpf release work on 458
  hosts and link info on 94 hosts.
- The query callers are a container manager's health checks.  Over 30
  days, 6,700 hosts reported one hung.  The typical wait is 11 minutes,
  the longest 75 minutes.
- The cgroup_mutex holder is almost always a cgroup mkdir or rmdir that
  is waiting for kernfs_rwsem.

Only attach, detach and replace need cgroup_mutex.  They update the
effective arrays of all descendants, so the hierarchy must not change.

Add cgroup_bpf_mutex for the bpf state.  Attach, detach and replace take
cgroup_mutex and then cgroup_bpf_mutex.  Everything else (query, link
info, the release work, dropping an LSM attach type, freeing a cgroup
storage map) takes only cgroup_bpf_mutex.

cgroup_bpf_inherit() still runs at mkdir with only cgroup_mutex, so mkdir
never waits for cgroup_bpf_mutex.  It writes the new cgroup, which no one
else can see yet.  What it reads from the ancestors changes only under
cgroup_mutex, and the release work can't run for them because they are
online.

The release work no longer waits for cgroup_mutex, so attach must check
that the cgroup is still alive.  The previous patch does that.

With a test delay that makes a mkdir hold cgroup_mutex for 5 seconds,
BPF_PROG_QUERY and link info now return in under a millisecond instead
of 4.6 seconds.  Attach and detach still wait.

Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 include/linux/bpf-cgroup-defs.h |   1 +
 include/linux/bpf-cgroup.h      |   3 +
 kernel/bpf/cgroup.c             | 137 +++++++++++++++++++++-----------
 kernel/bpf/local_storage.c      |   8 +-
 4 files changed, 102 insertions(+), 47 deletions(-)

diff --git a/include/linux/bpf-cgroup-defs.h b/include/linux/bpf-cgroup-defs.h
index 0147b8bec973..671f27ba6a62 100644
--- a/include/linux/bpf-cgroup-defs.h
+++ b/include/linux/bpf-cgroup-defs.h
@@ -53,6 +53,7 @@ enum cgroup_bpf_attach_type {
 	MAX_CGROUP_BPF_ATTACH_TYPE
 };
 
+/* Locking: see cgroup_bpf_mutex in kernel/bpf/cgroup.c */
 struct cgroup_bpf {
 	/* array of effective progs in this cgroup */
 	struct bpf_prog_array __rcu *effective[MAX_CGROUP_BPF_ATTACH_TYPE];
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 02ef7f899b3d..dfa7a75507c9 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -421,6 +421,9 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
 	__ret;								       \
 })
 
+/* Take cgroup_mutex first if you need both.  See kernel/bpf/cgroup.c. */
+void cgroup_bpf_lock(void);
+void cgroup_bpf_unlock(void);
 int cgroup_bpf_prog_attach(const union bpf_attr *attr,
 			   enum bpf_prog_type ptype, struct bpf_prog *prog);
 int cgroup_bpf_prog_detach(const union bpf_attr *attr,
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 334237096759..ae082ac86130 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -206,13 +206,53 @@ unsigned int __cgroup_bpf_run_lsm_current(const void *ctx,
 	return ret;
 }
 
+/*
+ * cgroup_bpf_mutex protects the bpf state of all cgroups: prog lists,
+ * effective arrays, flags, revisions, storage lists, link->cgroup,
+ * link->map of struct_ops links and cgroup_lsm_atype[].
+ *
+ * Take cgroup_mutex first if you need both.  This mutex doesn't keep a
+ * cgroup alive or the hierarchy stable.  Attach, detach and replace walk
+ * the descendants, so they take both.  Everything else takes only this one.
+ *
+ * cgroup_bpf_inherit() runs at mkdir with only cgroup_mutex.  That's safe:
+ * it writes the new cgroup, which no one else can see yet, and what it
+ * reads from the ancestors changes only under cgroup_mutex.  The ancestors
+ * are online, and only rmdir, which needs cgroup_mutex, starts a release.
+ */
+static DEFINE_MUTEX(cgroup_bpf_mutex);
+
+#define cgroup_bpf_held()	lockdep_is_held(&cgroup_bpf_mutex)
+
+void cgroup_bpf_lock(void)
+{
+	mutex_lock(&cgroup_bpf_mutex);
+}
+
+void cgroup_bpf_unlock(void)
+{
+	mutex_unlock(&cgroup_bpf_mutex);
+}
+
+static void cgroup_bpf_lock_hierarchy(void)
+{
+	cgroup_lock();
+	mutex_lock(&cgroup_bpf_mutex);
+}
+
+static void cgroup_bpf_unlock_hierarchy(void)
+{
+	mutex_unlock(&cgroup_bpf_mutex);
+	cgroup_unlock();
+}
+
 #ifdef CONFIG_BPF_LSM
 static enum cgroup_bpf_attach_type
 bpf_cgroup_atype_find(enum bpf_attach_type attach_type, u32 attach_btf_id)
 {
 	int i;
 
-	lockdep_assert_held(&cgroup_mutex);
+	lockdep_assert_held(&cgroup_bpf_mutex);
 
 	if (attach_type != BPF_LSM_CGROUP)
 		return to_cgroup_bpf_attach_type(attach_type);
@@ -233,7 +273,7 @@ void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype)
 {
 	int i = cgroup_atype - CGROUP_LSM_START;
 
-	lockdep_assert_held(&cgroup_mutex);
+	lockdep_assert_held(&cgroup_bpf_mutex);
 
 	if (!cgroup_lsm_atype[i].attach_btf_id) {
 		cgroup_lsm_atype[i].attach_btf_id = attach_btf_id;
@@ -249,13 +289,13 @@ void bpf_cgroup_atype_put(int cgroup_atype)
 {
 	int i = cgroup_atype - CGROUP_LSM_START;
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 	if (--cgroup_lsm_atype[i].refcnt <= 0) {
 		WRITE_ONCE(cgroup_lsm_atype[i].returns_errno, true);
 		cgroup_lsm_atype[i].attach_btf_id = 0;
 	}
 	WARN_ON_ONCE(cgroup_lsm_atype[i].refcnt < 0);
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 }
 #else
 static enum cgroup_bpf_attach_type
@@ -459,7 +499,7 @@ static void cgroup_bpf_release(struct work_struct *work)
 
 	unsigned int atype;
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 
 	for (atype = 0; atype < ARRAY_SIZE(cgrp->bpf.progs); atype++) {
 		struct hlist_head *progs = &cgrp->bpf.progs[atype];
@@ -480,7 +520,7 @@ static void cgroup_bpf_release(struct work_struct *work)
 		}
 		old_array = rcu_dereference_protected(
 				cgrp->bpf.effective[atype],
-				lockdep_is_held(&cgroup_mutex));
+				cgroup_bpf_held());
 		bpf_cgroup_array_free(old_array, atype);
 	}
 
@@ -489,7 +529,7 @@ static void cgroup_bpf_release(struct work_struct *work)
 		bpf_cgroup_storage_free(storage);
 	}
 
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 
 	for (p = cgroup_parent(cgrp); p; p = cgroup_parent(p))
 		cgroup_bpf_put(p);
@@ -665,7 +705,9 @@ static void activate_effective_progs(struct cgroup *cgrp,
 				     enum cgroup_bpf_attach_type atype,
 				     struct bpf_prog_array *old_array)
 {
+	/* cgroup_bpf_inherit() holds only cgroup_mutex, see cgroup_bpf_mutex */
 	old_array = rcu_replace_pointer(cgrp->bpf.effective[atype], old_array,
+					lockdep_is_held(&cgroup_bpf_mutex) ||
 					lockdep_is_held(&cgroup_mutex));
 	/* free prog array after grace period, since __cgroup_bpf_run_*()
 	 * might be still walking the array
@@ -687,6 +729,9 @@ static int cgroup_bpf_inherit(struct cgroup *cgrp)
 	struct cgroup *p;
 	int ret, i;
 
+	/* See the comment above cgroup_bpf_mutex */
+	lockdep_assert_held(&cgroup_mutex);
+
 	ret = percpu_ref_init(&cgrp->bpf.refcnt, cgroup_bpf_release_fn, 0,
 			      GFP_KERNEL);
 	if (ret)
@@ -747,6 +792,8 @@ static int update_effective_progs(struct cgroup *cgrp,
 	struct cgroup_subsys_state *css;
 	int err;
 
+	lockdep_assert_held(&cgroup_bpf_mutex);
+
 	/* allocate and recompute effective prog arrays */
 	css_for_each_descendant_pre(css, &cgrp->self) {
 		struct cgroup *desc = container_of(css, struct cgroup, self);
@@ -954,7 +1001,7 @@ static int insert_pl_to_hlist(struct bpf_prog_list *pl, struct hlist_head *progs
  * @revision: bpf_prog_list revision
  *
  * Exactly one of @prog or @link can be non-null.
- * Must be called with cgroup_mutex held.
+ * Must be called with cgroup_mutex and cgroup_bpf_mutex held.
  */
 static int __cgroup_bpf_attach(struct cgroup *cgrp,
 			       struct bpf_prog *prog, struct bpf_prog *replace_prog,
@@ -1100,10 +1147,10 @@ static int cgroup_bpf_attach(struct cgroup *cgrp,
 {
 	int ret;
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 	ret = __cgroup_bpf_attach(cgrp, prog, replace_prog, link, type, flags,
 				  id_or_fd, revision);
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 	return ret;
 }
 
@@ -1181,7 +1228,7 @@ static void replace_effective_prog(struct cgroup *cgrp,
 
 		progs = rcu_dereference_protected(
 				desc->bpf.effective[atype],
-				lockdep_is_held(&cgroup_mutex));
+				cgroup_bpf_held());
 		item = &progs->items[pos];
 		prog_list_replace_item(pl, item);
 	}
@@ -1209,7 +1256,7 @@ static bool cgroup_bpf_storages_compatible(struct bpf_prog *old_prog,
  * @new_prog: &struct bpf_prog for the target BPF program with its refcnt
  *            incremented
  *
- * Must be called with cgroup_mutex held.
+ * Must be called with cgroup_mutex and cgroup_bpf_mutex held.
  */
 static int __cgroup_bpf_replace(struct cgroup *cgrp,
 				struct bpf_cgroup_link *link,
@@ -1257,7 +1304,7 @@ static int cgroup_bpf_replace(struct bpf_link *link, struct bpf_prog *new_prog,
 
 	cg_link = container_of(link, struct bpf_cgroup_link, link);
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 	/* link might have been auto-released by dying cgroup, so fail */
 	if (!cg_link->cgroup) {
 		ret = -ENOLINK;
@@ -1269,7 +1316,7 @@ static int cgroup_bpf_replace(struct bpf_link *link, struct bpf_prog *new_prog,
 	}
 	ret = __cgroup_bpf_replace(cg_link->cgroup, cg_link, new_prog);
 out_unlock:
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 	return ret;
 }
 
@@ -1340,7 +1387,7 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog_list *pl,
 
 		progs = rcu_dereference_protected(
 				desc->bpf.effective[atype],
-				lockdep_is_held(&cgroup_mutex));
+				cgroup_bpf_held());
 
 		/* Remove the program from the array */
 		WARN_ONCE(bpf_cgroup_array_delete_safe_at(progs, pos, atype),
@@ -1358,7 +1405,7 @@ static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog_list *pl,
  * @revision: bpf_prog_list revision
  *
  * At most one of @prog or @link can be non-NULL.
- * Must be called with cgroup_mutex held.
+ * Must be called with cgroup_mutex and cgroup_bpf_mutex held.
  */
 static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 			       struct bpf_cgroup_link *link, enum bpf_attach_type type,
@@ -1428,13 +1475,13 @@ static int cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 {
 	int ret;
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 	ret = __cgroup_bpf_detach(cgrp, prog, NULL, type, revision);
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 	return ret;
 }
 
-/* Must be called with cgroup_mutex held to avoid races. */
+/* Must be called with cgroup_bpf_mutex held. */
 static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
 			      union bpf_attr __user *uattr, u32 uattr_size)
 {
@@ -1488,7 +1535,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
 	for (atype = from_atype; atype <= to_atype; atype++) {
 		if (effective_query) {
 			effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
-							      lockdep_is_held(&cgroup_mutex));
+							      cgroup_bpf_held());
 			total_cnt += bpf_cgroup_array_length(effective, atype);
 		} else {
 			total_cnt += prog_list_length(&cgrp->bpf.progs[atype], NULL);
@@ -1518,7 +1565,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
 	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
 		if (effective_query) {
 			effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
-							      lockdep_is_held(&cgroup_mutex));
+							      cgroup_bpf_held());
 			cnt = min_t(int, bpf_cgroup_array_length(effective, atype), total_cnt);
 			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
 		} else {
@@ -1559,9 +1606,9 @@ static int cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
 {
 	int ret;
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 	ret = __cgroup_bpf_query(cgrp, attr, uattr, uattr_size);
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 	return ret;
 }
 
@@ -1629,11 +1676,11 @@ static void bpf_cgroup_link_release(struct bpf_link *link)
 	if (!cg_link->cgroup)
 		return;
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 
 	/* re-check cgroup under lock again */
 	if (!cg_link->cgroup) {
-		cgroup_unlock();
+		cgroup_bpf_unlock_hierarchy();
 		return;
 	}
 
@@ -1645,7 +1692,7 @@ static void bpf_cgroup_link_release(struct bpf_link *link)
 	cg = cg_link->cgroup;
 	cg_link->cgroup = NULL;
 
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 
 	cgroup_put(cg);
 }
@@ -1672,10 +1719,10 @@ static void bpf_cgroup_link_show_fdinfo(const struct bpf_link *link,
 		container_of(link, struct bpf_cgroup_link, link);
 	u64 cg_id = 0;
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 	if (cg_link->cgroup)
 		cg_id = cgroup_id(cg_link->cgroup);
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 
 	seq_printf(seq,
 		   "cgroup_id:\t%llu\n"
@@ -1691,10 +1738,10 @@ static int bpf_cgroup_link_fill_link_info(const struct bpf_link *link,
 		container_of(link, struct bpf_cgroup_link, link);
 	u64 cg_id = 0;
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 	if (cg_link->cgroup)
 		cg_id = cgroup_id(cg_link->cgroup);
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 
 	info->cgroup.cgroup_id = cg_id;
 	info->cgroup.attach_type = link->attach_type;
@@ -2971,11 +3018,11 @@ static int __cgroup_struct_ops_link_detach(struct bpf_link *link, bool wake_poll
 	struct bpf_map *map;
 	struct cgroup *cgrp;
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 
 	cgrp = cg_link->cgroup;
 	if (!cgrp) {
-		cgroup_unlock();
+		cgroup_bpf_unlock_hierarchy();
 		return 0;
 	}
 
@@ -2987,7 +3034,7 @@ static int __cgroup_struct_ops_link_detach(struct bpf_link *link, bool wake_poll
 			break;
 	}
 	if (WARN_ON_ONCE(!pl)) {
-		cgroup_unlock();
+		cgroup_bpf_unlock_hierarchy();
 		return -ENOENT;
 	}
 
@@ -3005,7 +3052,7 @@ static int __cgroup_struct_ops_link_detach(struct bpf_link *link, bool wake_poll
 	kfree(pl);
 	static_branch_dec(&cgroup_bpf_enabled_key[atype]);
 
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 
 	return 0;
 }
@@ -3028,15 +3075,15 @@ static void cgroup_struct_ops_link_show_fdinfo(const struct bpf_link *link, stru
 	struct bpf_cgroup_link *cg_link =
 		container_of(link, struct bpf_cgroup_link, link);
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 	if (!cg_link->cgroup) {
-		cgroup_unlock();
+		cgroup_bpf_unlock();
 		return;
 	}
 
 	seq_printf(seq, "map_id:\t%u\n", cg_link->map->id);
 	seq_printf(seq, "cgroup_id:\t%llu\n", cgroup_id(cg_link->cgroup));
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 }
 
 static int cgroup_struct_ops_link_fill_link_info(const struct bpf_link *link,
@@ -3044,15 +3091,15 @@ static int cgroup_struct_ops_link_fill_link_info(const struct bpf_link *link,
 {
 	struct bpf_cgroup_link *cg_link = container_of(link, struct bpf_cgroup_link, link);
 
-	cgroup_lock();
+	cgroup_bpf_lock();
 	if (!cg_link->cgroup) {
-		cgroup_unlock();
+		cgroup_bpf_unlock();
 		return 0;
 	}
 
 	info->struct_ops.map_id = cg_link->map->id;
 	info->struct_ops.cgroup_id = cgroup_id(cg_link->cgroup);
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 	return 0;
 }
 
@@ -3070,7 +3117,7 @@ static int cgroup_struct_ops_link_update(struct bpf_link *link, struct bpf_map *
 	if (!bpf_struct_ops_valid_to_reg(new_map))
 		return -EINVAL;
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 
 	cgrp = cg_link->cgroup;
 	if (!cgrp) {
@@ -3103,7 +3150,7 @@ static int cgroup_struct_ops_link_update(struct bpf_link *link, struct bpf_map *
 	cgrp->bpf.revisions[atype]++;
 
 out:
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 	return err;
 }
 
@@ -3175,7 +3222,7 @@ int cgroup_bpf_struct_ops_attach(struct bpf_map *map, const union bpf_attr *attr
 	atype = bpf_struct_ops_map_cgroup_atype(map);
 	progs = &cgrp->bpf.progs[atype];
 
-	cgroup_lock();
+	cgroup_bpf_lock_hierarchy();
 
 	/* See the cgroup_is_dead() check in __cgroup_bpf_attach() */
 	if (cgroup_is_dead(cgrp)) {
@@ -3218,12 +3265,12 @@ int cgroup_bpf_struct_ops_attach(struct bpf_map *map, const union bpf_attr *attr
 	cgrp->bpf.revisions[atype]++;
 	static_branch_inc(&cgroup_bpf_enabled_key[atype]);
 
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 
 	return bpf_link_settle(&link_primer);
 
 unlock:
-	cgroup_unlock();
+	cgroup_bpf_unlock_hierarchy();
 
 cleanup:
 	kfree(pl);
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 83cd527a2542..2cfbddeb1c7e 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -345,14 +345,18 @@ static void cgroup_storage_map_free(struct bpf_map *_map)
 	struct list_head *storages = &map->list;
 	struct bpf_cgroup_storage *storage, *stmp;
 
-	cgroup_lock();
+	/*
+	 * Attach and cgroup_bpf_release() change the same cgroup storage
+	 * lists under other maps' locks, so take cgroup_bpf_mutex.
+	 */
+	cgroup_bpf_lock();
 
 	list_for_each_entry_safe(storage, stmp, storages, list_map) {
 		bpf_cgroup_storage_unlink(storage);
 		bpf_cgroup_storage_free(storage);
 	}
 
-	cgroup_unlock();
+	cgroup_bpf_unlock();
 
 	WARN_ON(!RB_EMPTY_ROOT(&map->root));
 	WARN_ON(!list_empty(&map->list));
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex
  2026-11-12 17:27 ` [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex Shakeel Butt
@ 2026-11-12 17:41   ` sashiko-bot
  2026-11-12 18:23     ` Shakeel Butt
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-11-12 17:41 UTC (permalink / raw)
  To: Shakeel Butt, bpf; +Cc: tj, hannes, mkoutny

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] bpf: lost error codes in __cgroup_bpf_query() [1]
--

commit 20190389f8922442abed742617dfa9e59bf26340
Author: Shakeel Butt <shakeel.butt@linux.dev>

bpf, cgroup: Protect cgroup_bpf state with its own mutex

This commit introduces a dedicated cgroup_bpf_mutex to protect cgroup BPF
state so that operations not modifying the hierarchy do not have to contend
on cgroup_mutex.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 334237096759f..ae082ac86130b 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c

[ ... ]

> @@ -1518,7 +1565,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
>  	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
>  		if (effective_query) {
>  			effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
> -							      lockdep_is_held(&cgroup_mutex));
> +							      cgroup_bpf_held());
>  			cnt = min_t(int, bpf_cgroup_array_length(effective, atype), total_cnt);
>  			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
>  		} else {

[Severity: Medium]
This problem wasn't introduced by this patch, but can assigning ret from
bpf_cgroup_array_copy_to_user() drop an earlier -ENOSPC or -EFAULT error
[1]?

Before the loop, __cgroup_bpf_query() sets ret = -ENOSPC when
attr->query.prog_cnt < total_cnt:

kernel/bpf/cgroup.c:__cgroup_bpf_query() {
	...
	if (attr->query.prog_cnt < total_cnt) {
		total_cnt = attr->query.prog_cnt;
		ret = -ENOSPC;
	}

	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
		if (effective_query) {
			...
			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
	...
}

When effective_query is set, bpf_cgroup_array_copy_to_user() overwrites
ret with 0 when the truncated slice fits in the array, or a later
iteration (for BPF_LSM_CGROUP) can overwrite an earlier -EFAULT, causing
__cgroup_bpf_query() to return 0 on truncated or faulted queries.

[1] https://sashiko.dev/#/bug/linux-20442c38-bcc8-440c-9711-1977ea6cd04b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112172721.440275-1-shakeel.butt@linux.dev?part=3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex
  2026-11-12 17:41   ` sashiko-bot
@ 2026-11-12 18:23     ` Shakeel Butt
  0 siblings, 0 replies; 6+ messages in thread
From: Shakeel Butt @ 2026-11-12 18:23 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: bpf, tj, hannes, mkoutny

On Thu, Nov 12, 2026 at 05:41:57PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found no new issues
> in this patch, but noticed 1 related pre-existing issue it does not
> introduce:
> - [Medium] bpf: lost error codes in __cgroup_bpf_query() [1]
> --
> 
> commit 20190389f8922442abed742617dfa9e59bf26340
> Author: Shakeel Butt <shakeel.butt@linux.dev>
> 
> bpf, cgroup: Protect cgroup_bpf state with its own mutex
> 
> This commit introduces a dedicated cgroup_bpf_mutex to protect cgroup BPF
> state so that operations not modifying the hierarchy do not have to contend
> on cgroup_mutex.
> 
> > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > index 334237096759f..ae082ac86130b 100644
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
> 
> [ ... ]
> 
> > @@ -1518,7 +1565,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
> >  	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
> >  		if (effective_query) {
> >  			effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
> > -							      lockdep_is_held(&cgroup_mutex));
> > +							      cgroup_bpf_held());
> >  			cnt = min_t(int, bpf_cgroup_array_length(effective, atype), total_cnt);
> >  			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
> >  		} else {
> 
> [Severity: Medium]
> This problem wasn't introduced by this patch, but can assigning ret from
> bpf_cgroup_array_copy_to_user() drop an earlier -ENOSPC or -EFAULT error
> [1]?
> 
> Before the loop, __cgroup_bpf_query() sets ret = -ENOSPC when
> attr->query.prog_cnt < total_cnt:
> 
> kernel/bpf/cgroup.c:__cgroup_bpf_query() {
> 	...
> 	if (attr->query.prog_cnt < total_cnt) {
> 		total_cnt = attr->query.prog_cnt;
> 		ret = -ENOSPC;
> 	}
> 
> 	for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
> 		if (effective_query) {
> 			...
> 			ret = bpf_cgroup_array_copy_to_user(effective, prog_ids, cnt, atype);
> 	...
> }
> 
> When effective_query is set, bpf_cgroup_array_copy_to_user() overwrites
> ret with 0 when the truncated slice fits in the array, or a later
> iteration (for BPF_LSM_CGROUP) can overwrite an earlier -EFAULT, causing
> __cgroup_bpf_query() to return 0 on truncated or faulted queries.
> 
> [1] https://sashiko.dev/#/bug/linux-20442c38-bcc8-440c-9711-1977ea6cd04b

Seems like an independent bug. I will send the fix separately.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-11-12 18:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12 17:27 [PATCH bpf-next 0/3] bpf, cgroup: Don't wait for cgroup_mutex to read or free cgroup bpf state Shakeel Butt
2026-11-12 17:27 ` [PATCH bpf-next 1/3] bpf, cgroup: Don't read freed effective arrays in BPF_PROG_QUERY Shakeel Butt
2026-11-12 17:27 ` [PATCH bpf-next 2/3] bpf, cgroup: Don't attach to a removed cgroup Shakeel Butt
2026-11-12 17:27 ` [PATCH bpf-next 3/3] bpf, cgroup: Protect cgroup_bpf state with its own mutex Shakeel Butt
2026-11-12 17:41   ` sashiko-bot
2026-11-12 18:23     ` Shakeel Butt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox