|
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 |
|