Thanks for sharing this Masami, I've been looking into pre-cgroup_ns backport too. On Wed, Feb 16, 2022 at 08:40:37AM +0900, Masami Ichikawa <masami256@xxxxxxxxx> wrote: > [masami: Backport patch from 4.9. Adjust to use current_user_ns() to get current user_ns. > Fix conflict in cgroup_release_agent_write().] The condition to allow modifying release_agent is two-fold: a) caller is capabable(CAP_SYS_ADMIN), b) cgroup_ns is owned by init_user_ns. In pre-cgroup_ns kernels, it is IMO safer to consider all (=the only) cgroup_ns owned by init_user_ns. So the (positive) condition translates into capable(CAP_SYS_ADMIN) only. [ Additionally, there's invariant/implication capable(CAP_XXX) -> (current_user_ns() == &init_user_ns) , so the expression (current_user_ns() != &init_user_ns) || !capable(CAP_SYS_ADMIN) simplifies to !capable(CAP_SYS_ADMIN) . ] > @@ -2839,6 +2856,14 @@ static ssize_t cgroup_release_agent_write(struct kernfs_open_file *of, > > BUILD_BUG_ON(sizeof(cgrp->root->release_agent_path) < PATH_MAX); > > + /* > + * Release agent gets called with all capabilities, > + * require capabilities to set release agent. > + */ > + if ((of->file->f_cred->user_ns != &init_user_ns) || > + !capable(CAP_SYS_ADMIN)) > + return -EPERM; > + Following the reasoning above, the check simplifies too but it should be be against the opener, not the writer: file_ns_capable(of->file, &init_user_ns, CAP_SYS_ADMIN) (It seems to be to be incorrect even in the original commit. So I'll send a patch there to rectify that.) Michal
