On Mon, Sep 27, 2021 at 02:39:20PM +0200, Daniel Borkmann wrote: > If cgroup_sk_alloc() is called from interrupt context, then just assign the > root cgroup to skcd->cgroup. Prior to commit 8520e224f547 ("bpf, cgroups: > Fix cgroup v2 fallback on v1/v2 mixed mode") we would just return, and later > on in sock_cgroup_ptr(), we were NULL-testing the cgroup in fast-path, and > iff indeed NULL returning the root cgroup (v ?: &cgrp_dfl_root.cgrp). Rather > than re-adding the NULL-test to the fast-path we can just assign it once from > cgroup_sk_alloc() given v1/v2 handling has been simplified. The migration from > NULL test with returning &cgrp_dfl_root.cgrp to assigning &cgrp_dfl_root.cgrp > directly does /not/ change behavior for callers of sock_cgroup_ptr(). > > syzkaller was able to trigger a splat in the legacy netrom code base, where > the RX handler in nr_rx_frame() calls nr_make_new() which calls sk_alloc() > and therefore cgroup_sk_alloc() with in_interrupt() condition. Thus the NULL > skcd->cgroup, where it trips over on cgroup_sk_free() side given it expects > a non-NULL object. There are a few other candidates aside from netrom which > have similar pattern where in their accept-like implementation, they just call > to sk_alloc() and thus cgroup_sk_alloc() instead of sk_clone_lock() with the > corresponding cgroup_sk_clone() which then inherits the cgroup from the parent > socket. None of them are related to core protocols where BPF cgroup programs > are running from. However, in future, they should follow to implement a similar > inheritance mechanism. > > Additionally, with a !CONFIG_CGROUP_NET_PRIO and !CONFIG_CGROUP_NET_CLASSID > configuration, the same issue was exposed also prior to 8520e224f547 due to > commit e876ecc67db8 ("cgroup: memcg: net: do not associate sock with unrelated > cgroup") which added the early in_interrupt() return back then. > > Fixes: 8520e224f547 ("bpf, cgroups: Fix cgroup v2 fallback on v1/v2 mixed mode") > Fixes: e876ecc67db8 ("cgroup: memcg: net: do not associate sock with unrelated cgroup") > Reported-by: syzbot+df709157a4ecaf192b03@xxxxxxxxxxxxxxxxxxxxxxxxx > Reported-by: syzbot+533f389d4026d86a2a95@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx> > Tested-by: syzbot+df709157a4ecaf192b03@xxxxxxxxxxxxxxxxxxxxxxxxx > Tested-by: syzbot+533f389d4026d86a2a95@xxxxxxxxxxxxxxxxxxxxxxxxx > Cc: Tejun Heo <tj@xxxxxxxxxx> > Cc: Stanislav Fomichev <sdf@xxxxxxxxxx> Acked-by: Tejun Heo <tj@xxxxxxxxxx> Thanks. -- tejun