Skip to content

Commit

Permalink
cgroup: Use open-time cgroup namespace for process migration perm checks
Browse files Browse the repository at this point in the history 
BugLink: https://bugs.launchpad.net/bugs/1973831

commit e57457641613fef0d147ede8bd6a3047df588b95 upstream.

cgroup process migration permission checks are performed at write time as
whether a given operation is allowed or not is dependent on the content of
the write - the PID. This currently uses current's cgroup namespace which is
a potential security weakness as it may allow scenarios where a less
privileged process tricks a more privileged one into writing into a fd that
it created.

This patch makes cgroup remember the cgroup namespace at the time of open
and uses it for migration permission checks instad of current's. Note that
this only applies to cgroup2 as cgroup1 doesn't have namespace support.

This also fixes a use-after-free bug on cgroupns reported in

 https://lore.kernel.org/r/00000000000048c15c05d0083397@google.com

Note that backporting this fix also requires the preceding patch.

Reported-by: "Eric W. Biederman" <ebiederm@xmission.com>
Suggested-by: Linus Torvalds <torvalds@linuxfoundation.org>
Cc: Michal Koutný <mkoutny@suse.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Michal Koutný <mkoutny@suse.com>
Reported-by: syzbot+50f5cf33a284ce738b62@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/00000000000048c15c05d0083397@google.com
Fixes: 5136f63 ("cgroup: implement "nsdelegate" mount option")
Signed-off-by: Tejun Heo <tj@kernel.org>
[mkoutny: v5.10: duplicate ns check in procs/threads write handler, adjust context]
Signed-off-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[OP: backport to v4.14: drop changes to cgroup_attach_permissions() and
cgroup_css_set_fork(), adjust cgroup_procs_write_permission() calls]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
  • Loading branch information
@htejun @LukeNow
htejun authored and LukeNow committed Jun 15, 2022
1 parent c327016 commit 89642b3
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 7 deletions.
2 changes: 2 additions & 0 deletions kernel/cgroup/cgroup-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
struct cgroup_pidlist;

struct cgroup_file_ctx {
struct cgroup_namespace *ns;

struct {
void *trigger;
} psi;
Expand Down
24 changes: 17 additions & 7 deletions kernel/cgroup/cgroup.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3436,14 +3436,19 @@ static int cgroup_file_open(struct kernfs_open_file *of)
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;

ctx->ns = current->nsproxy->cgroup_ns;
get_cgroup_ns(ctx->ns);
of->priv = ctx;

if (!cft->open)
return 0;

ret = cft->open(of);
if (ret)
if (ret) {
put_cgroup_ns(ctx->ns);
kfree(ctx);
}
return ret;
}

Expand All @@ -3454,13 +3459,14 @@ static void cgroup_file_release(struct kernfs_open_file *of)

if (cft->release)
cft->release(of);
put_cgroup_ns(ctx->ns);
kfree(ctx);
}

static ssize_t cgroup_file_write(struct kernfs_open_file *of, char *buf,
size_t nbytes, loff_t off)
{
struct cgroup_namespace *ns = current->nsproxy->cgroup_ns;
struct cgroup_file_ctx *ctx = of->priv;
struct cgroup *cgrp = of->kn->parent->priv;
struct cftype *cft = of->kn->priv;
struct cgroup_subsys_state *css;
Expand All @@ -3474,7 +3480,7 @@ static ssize_t cgroup_file_write(struct kernfs_open_file *of, char *buf,
*/
if ((cgrp->root->flags & CGRP_ROOT_NS_DELEGATE) &&
!(cft->flags & CFTYPE_NS_DELEGATABLE) &&
ns != &init_cgroup_ns && ns->root_cset->dfl_cgrp == cgrp)
ctx->ns != &init_cgroup_ns && ctx->ns->root_cset->dfl_cgrp == cgrp)
return -EPERM;

if (cft->write)
Expand Down Expand Up @@ -4419,9 +4425,9 @@ static int cgroup_procs_show(struct seq_file *s, void *v)

static int cgroup_procs_write_permission(struct cgroup *src_cgrp,
struct cgroup *dst_cgrp,
struct super_block *sb)
struct super_block *sb,
struct cgroup_namespace *ns)
{
struct cgroup_namespace *ns = current->nsproxy->cgroup_ns;
struct cgroup *com_cgrp = src_cgrp;
struct inode *inode;
int ret;
Expand Down Expand Up @@ -4457,6 +4463,7 @@ static int cgroup_procs_write_permission(struct cgroup *src_cgrp,
static ssize_t cgroup_procs_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
struct cgroup_file_ctx *ctx = of->priv;
struct cgroup *src_cgrp, *dst_cgrp;
struct task_struct *task;
const struct cred *saved_cred;
Expand All @@ -4483,7 +4490,8 @@ static ssize_t cgroup_procs_write(struct kernfs_open_file *of,
*/
saved_cred = override_creds(of->file->f_cred);
ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp,
of->file->f_path.dentry->d_sb);
of->file->f_path.dentry->d_sb,
ctx->ns);
revert_creds(saved_cred);
if (ret)
goto out_finish;
Expand All @@ -4506,6 +4514,7 @@ static void *cgroup_threads_start(struct seq_file *s, loff_t *pos)
static ssize_t cgroup_threads_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
struct cgroup_file_ctx *ctx = of->priv;
struct cgroup *src_cgrp, *dst_cgrp;
struct task_struct *task;
const struct cred *saved_cred;
Expand Down Expand Up @@ -4534,7 +4543,8 @@ static ssize_t cgroup_threads_write(struct kernfs_open_file *of,
*/
saved_cred = override_creds(of->file->f_cred);
ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp,
of->file->f_path.dentry->d_sb);
of->file->f_path.dentry->d_sb,
ctx->ns);
revert_creds(saved_cred);
if (ret)
goto out_finish;
Expand Down

0 comments on commit 89642b3

Please sign in to comment.