Takashi Iwai 26454b
From 2febd1914ddcd5b0dd851e02a5aced08977ff205 Mon Sep 17 00:00:00 2001
Takashi Iwai 26454b
From: Daniel Borkmann <daniel@iogearbox.net>
Takashi Iwai 26454b
Date: Mon, 27 Sep 2021 14:39:20 +0200
Takashi Iwai 26454b
Subject: [PATCH] bpf, cgroup: Assign cgroup in cgroup_sk_alloc when called from interrupt
Takashi Iwai 26454b
Git-commit: 78cc316e9583067884eb8bd154301dc1e9ee945c
Takashi Iwai 26454b
Patch-mainline: v5.15-rc4
Michal Koutný 590ab5
References: stable-5.14.19 bsc#1191279
Takashi Iwai 26454b
Takashi Iwai 26454b
[ Upstream commit 78cc316e9583067884eb8bd154301dc1e9ee945c ]
Takashi Iwai 26454b
Takashi Iwai 26454b
If cgroup_sk_alloc() is called from interrupt context, then just assign the
Takashi Iwai 26454b
root cgroup to skcd->cgroup. Prior to commit 8520e224f547 ("bpf, cgroups:
Takashi Iwai 26454b
Fix cgroup v2 fallback on v1/v2 mixed mode") we would just return, and later
Takashi Iwai 26454b
on in sock_cgroup_ptr(), we were NULL-testing the cgroup in fast-path, and
Takashi Iwai 26454b
iff indeed NULL returning the root cgroup (v ?: &cgrp_dfl_root.cgrp). Rather
Takashi Iwai 26454b
than re-adding the NULL-test to the fast-path we can just assign it once from
Takashi Iwai 26454b
cgroup_sk_alloc() given v1/v2 handling has been simplified. The migration from
Takashi Iwai 26454b
NULL test with returning &cgrp_dfl_root.cgrp to assigning &cgrp_dfl_root.cgrp
Takashi Iwai 26454b
directly does /not/ change behavior for callers of sock_cgroup_ptr().
Takashi Iwai 26454b
Takashi Iwai 26454b
syzkaller was able to trigger a splat in the legacy netrom code base, where
Takashi Iwai 26454b
the RX handler in nr_rx_frame() calls nr_make_new() which calls sk_alloc()
Takashi Iwai 26454b
and therefore cgroup_sk_alloc() with in_interrupt() condition. Thus the NULL
Takashi Iwai 26454b
skcd->cgroup, where it trips over on cgroup_sk_free() side given it expects
Takashi Iwai 26454b
a non-NULL object. There are a few other candidates aside from netrom which
Takashi Iwai 26454b
have similar pattern where in their accept-like implementation, they just call
Takashi Iwai 26454b
to sk_alloc() and thus cgroup_sk_alloc() instead of sk_clone_lock() with the
Takashi Iwai 26454b
corresponding cgroup_sk_clone() which then inherits the cgroup from the parent
Takashi Iwai 26454b
socket. None of them are related to core protocols where BPF cgroup programs
Takashi Iwai 26454b
are running from. However, in future, they should follow to implement a similar
Takashi Iwai 26454b
inheritance mechanism.
Takashi Iwai 26454b
Takashi Iwai 26454b
Additionally, with a !CONFIG_CGROUP_NET_PRIO and !CONFIG_CGROUP_NET_CLASSID
Takashi Iwai 26454b
configuration, the same issue was exposed also prior to 8520e224f547 due to
Takashi Iwai 26454b
commit e876ecc67db8 ("cgroup: memcg: net: do not associate sock with unrelated
Takashi Iwai 26454b
cgroup") which added the early in_interrupt() return back then.
Takashi Iwai 26454b
Takashi Iwai 26454b
Fixes: 8520e224f547 ("bpf, cgroups: Fix cgroup v2 fallback on v1/v2 mixed mode")
Takashi Iwai 26454b
Fixes: e876ecc67db8 ("cgroup: memcg: net: do not associate sock with unrelated cgroup")
Takashi Iwai 26454b
Reported-by: syzbot+df709157a4ecaf192b03@syzkaller.appspotmail.com
Takashi Iwai 26454b
Reported-by: syzbot+533f389d4026d86a2a95@syzkaller.appspotmail.com
Takashi Iwai 26454b
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Takashi Iwai 26454b
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Takashi Iwai 26454b
Tested-by: syzbot+df709157a4ecaf192b03@syzkaller.appspotmail.com
Takashi Iwai 26454b
Tested-by: syzbot+533f389d4026d86a2a95@syzkaller.appspotmail.com
Takashi Iwai 26454b
Acked-by: Tejun Heo <tj@kernel.org>
Takashi Iwai 26454b
Link: https://lore.kernel.org/bpf/20210927123921.21535-1-daniel@iogearbox.net
Takashi Iwai 26454b
Signed-off-by: Sasha Levin <sashal@kernel.org>
Takashi Iwai 26454b
Acked-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai 26454b
Takashi Iwai 26454b
---
Takashi Iwai 26454b
 kernel/cgroup/cgroup.c | 17 ++++++++++++-----
Takashi Iwai 26454b
 1 file changed, 12 insertions(+), 5 deletions(-)
Takashi Iwai 26454b
Takashi Iwai 26454b
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
Takashi Iwai 26454b
index 869a16f7684f..bfbed4c99f16 100644
Takashi Iwai 26454b
--- a/kernel/cgroup/cgroup.c
Takashi Iwai 26454b
+++ b/kernel/cgroup/cgroup.c
Takashi Iwai 26454b
@@ -6586,22 +6586,29 @@ int cgroup_parse_float(const char *input, unsigned dec_shift, s64 *v)
Takashi Iwai 26454b
 
Takashi Iwai 26454b
 void cgroup_sk_alloc(struct sock_cgroup_data *skcd)
Takashi Iwai 26454b
 {
Takashi Iwai 26454b
-	/* Don't associate the sock with unrelated interrupted task's cgroup. */
Takashi Iwai 26454b
-	if (in_interrupt())
Takashi Iwai 26454b
-		return;
Takashi Iwai 26454b
+	struct cgroup *cgroup;
Takashi Iwai 26454b
 
Takashi Iwai 26454b
 	rcu_read_lock();
Takashi Iwai 26454b
+	/* Don't associate the sock with unrelated interrupted task's cgroup. */
Takashi Iwai 26454b
+	if (in_interrupt()) {
Takashi Iwai 26454b
+		cgroup = &cgrp_dfl_root.cgrp;
Takashi Iwai 26454b
+		cgroup_get(cgroup);
Takashi Iwai 26454b
+		goto out;
Takashi Iwai 26454b
+	}
Takashi Iwai 26454b
+
Takashi Iwai 26454b
 	while (true) {
Takashi Iwai 26454b
 		struct css_set *cset;
Takashi Iwai 26454b
 
Takashi Iwai 26454b
 		cset = task_css_set(current);
Takashi Iwai 26454b
 		if (likely(cgroup_tryget(cset->dfl_cgrp))) {
Takashi Iwai 26454b
-			skcd->cgroup = cset->dfl_cgrp;
Takashi Iwai 26454b
-			cgroup_bpf_get(cset->dfl_cgrp);
Takashi Iwai 26454b
+			cgroup = cset->dfl_cgrp;
Takashi Iwai 26454b
 			break;
Takashi Iwai 26454b
 		}
Takashi Iwai 26454b
 		cpu_relax();
Takashi Iwai 26454b
 	}
Takashi Iwai 26454b
+out:
Takashi Iwai 26454b
+	skcd->cgroup = cgroup;
Takashi Iwai 26454b
+	cgroup_bpf_get(cgroup);
Takashi Iwai 26454b
 	rcu_read_unlock();
Takashi Iwai 26454b
 }
Takashi Iwai 26454b
 
Takashi Iwai 26454b
-- 
Takashi Iwai 26454b
2.26.2
Takashi Iwai 26454b