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 <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
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(-)
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 */
--
2.39.2
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[email protected]>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html