| Message ID | ZBDuVWiFj2jiz3i8@slm.duckdns.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b8a2e3f93d412114a1539ea97b59b3e6ed6e1f9a |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf/for-next] cgroup: Make current_cgns_cgroup_dfl() safe to call after exit_task_namespace() | expand |
Hello: This patch was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Tue, 14 Mar 2023 11:59:49 -1000 you wrote: > 332ea1f697be ("bpf: Add bpf_cgroup_from_id() kfunc") added > bpf_cgroup_from_id() which calls current_cgns_cgroup_dfl() through > cgroup_get_from_id(). However, BPF programs may be attached to a point where > current->nsproxy has already been cleared to NULL by exit_task_namespace() > and calling bpf_cgroup_from_id() would cause an oops. > > Just return the system-wide root if nsproxy has been cleared. This allows > all cgroups to be looked up after the task passed through > exit_task_namespace(), which semantically makes sense. Given that the only > way to get this behavior is through BPF programs, it seems safe but let's > see what others think. > > [...] Here is the summary with links: - [bpf/for-next] cgroup: Make current_cgns_cgroup_dfl() safe to call after exit_task_namespace() https://git.kernel.org/bpf/bpf-next/c/b8a2e3f93d41 You are awesome, thank you!
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index 8bfb0e2a87ee..c0da5cb9f193 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -1465,8 +1465,20 @@ static struct cgroup *current_cgns_cgroup_dfl(void) { struct css_set *cset; - cset = current->nsproxy->cgroup_ns->root_cset; - return __cset_cgroup_from_root(cset, &cgrp_dfl_root); + if (current->nsproxy) { + cset = current->nsproxy->cgroup_ns->root_cset; + return __cset_cgroup_from_root(cset, &cgrp_dfl_root); + } else { + /* + * NOTE: This function may be called from bpf_cgroup_from_id() + * on a task which has already passed exit_task_namespace() and + * NULL nsproxy. Fall back to cgrp_dfl_root which will make all + * cgroups visible for lookups. Given that BPF progs are + * privileged, this shouldn't create security concerns but there + * may be a better way to handle this. + */ + return &cgrp_dfl_root.cgrp; + } } /* look up cgroup associated with given css_set on the specified hierarchy */
332ea1f697be ("bpf: Add bpf_cgroup_from_id() kfunc") added bpf_cgroup_from_id() which calls current_cgns_cgroup_dfl() through cgroup_get_from_id(). However, BPF programs may be attached to a point where current->nsproxy has already been cleared to NULL by exit_task_namespace() and calling bpf_cgroup_from_id() would cause an oops. Just return the system-wide root if nsproxy has been cleared. This allows all cgroups to be looked up after the task passed through exit_task_namespace(), which semantically makes sense. Given that the only way to get this behavior is through BPF programs, it seems safe but let's see what others think. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Alexei Starovoitov <ast@kernel.org> Fixes: 332ea1f697be ("bpf: Add bpf_cgroup_from_id() kfunc") --- Hello, Given that this is only exposed through a commit in bpf/for-next, I think it'd be best to route it together. If this looks okay, please apply to bpf/for-next. Thanks. kernel/cgroup/cgroup.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)