public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: torvalds@linuxfoundation.org, ebiederm@xmission.com,
	mkoutny@suse.com, axboe@kernel.dk, keescook@chromium.org,
	oleg@redhat.com, peterz@infradead.org, tglx@linutronix.de,
	jnewsome@torproject.org, legion@kernel.org, luto@amacapital.net,
	jannh@google.com
Cc: linux-kernel@vger.kernel.org, security@kernel.org,
	kernel-team@fb.com, Tejun Heo <tj@kernel.org>,
	syzbot+50f5cf33a284ce738b62@syzkaller.appspotmail.com
Subject: [PATCH 3/6] cgroup: Use open-time cgroup namespace for process migration perm checks
Date: Mon, 13 Dec 2021 09:18:30 -1000	[thread overview]
Message-ID: <20211213191833.916632-4-tj@kernel.org> (raw)
In-Reply-To: <20211213191833.916632-1-tj@kernel.org>

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>
Reported-by: syzbot+50f5cf33a284ce738b62@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/00000000000048c15c05d0083397@google.com
Fixes: 5136f6365ce3 ("cgroup: implement "nsdelegate" mount option")
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/cgroup/cgroup-internal.h |  2 ++
 kernel/cgroup/cgroup.c          | 28 +++++++++++++++++++---------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 0ced1e04177f..9ead575f9678 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -66,6 +66,8 @@ static inline struct cgroup_fs_context *cgroup_fc2context(struct fs_context *fc)
 }
 
 struct cgroup_file_ctx {
+	struct cgroup_namespace	*ns;
+
 	struct {
 		bool			started;
 		struct css_task_iter	iter;
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index a84631d08d98..cafb8c114a21 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3822,14 +3822,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;
 }
 
@@ -3840,13 +3845,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_cft(of);
 	struct cgroup_subsys_state *css;
@@ -3863,7 +3869,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)
@@ -4853,9 +4859,9 @@ static int cgroup_may_write(const struct cgroup *cgrp, struct super_block *sb)
 
 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;
 	int ret;
 
@@ -4884,11 +4890,12 @@ static int cgroup_procs_write_permission(struct cgroup *src_cgrp,
 
 static int cgroup_attach_permissions(struct cgroup *src_cgrp,
 				     struct cgroup *dst_cgrp,
-				     struct super_block *sb, bool threadgroup)
+				     struct super_block *sb, bool threadgroup,
+				     struct cgroup_namespace *ns)
 {
 	int ret = 0;
 
-	ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb);
+	ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb, ns);
 	if (ret)
 		return ret;
 
@@ -4905,6 +4912,7 @@ static int cgroup_attach_permissions(struct cgroup *src_cgrp,
 static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
 				    bool threadgroup)
 {
+	struct cgroup_file_ctx *ctx = of->priv;
 	struct cgroup *src_cgrp, *dst_cgrp;
 	struct task_struct *task;
 	const struct cred *saved_cred;
@@ -4932,7 +4940,8 @@ static ssize_t __cgroup_procs_write(struct kernfs_open_file *of, char *buf,
 	 */
 	saved_cred = override_creds(of->file->f_cred);
 	ret = cgroup_attach_permissions(src_cgrp, dst_cgrp,
-					of->file->f_path.dentry->d_sb, threadgroup);
+					of->file->f_path.dentry->d_sb,
+					threadgroup, ctx->ns);
 	revert_creds(saved_cred);
 	if (ret)
 		goto out_finish;
@@ -6152,7 +6161,8 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs)
 		goto err;
 
 	ret = cgroup_attach_permissions(cset->dfl_cgrp, dst_cgrp, sb,
-					!(kargs->flags & CLONE_THREAD));
+					!(kargs->flags & CLONE_THREAD),
+					current->nsproxy->cgroup_ns);
 	if (ret)
 		goto err;
 
-- 
2.34.1


  parent reply	other threads:[~2021-12-13 19:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13 19:18 [PATCHSET v2 cgroup/for-5.16-fixes] cgroup: Use open-time creds and namespace for migration perm checks Tejun Heo
2021-12-13 19:18 ` [PATCH 1/6] cgroup: Use open-time credentials for process migraton " Tejun Heo
2021-12-14 17:03   ` Michal Koutný
2021-12-13 19:18 ` [PATCH 2/6] cgroup: Allocate cgroup_file_ctx for kernfs_open_file->priv Tejun Heo
2021-12-13 19:29   ` Linus Torvalds
2021-12-13 19:56     ` Tejun Heo
2021-12-14 17:03   ` Michal Koutný
2021-12-14 19:44     ` [PATCH v3 " Tejun Heo
2021-12-15  7:37       ` Michal Koutný
2021-12-16  9:22   ` [cgroup] 27183b4e07: WARNING:at_mm/slab.c:#___cache_free kernel test robot
2021-12-16  9:43     ` Michal Koutný
2021-12-13 19:18 ` Tejun Heo [this message]
2021-12-14 17:04   ` [PATCH 3/6] cgroup: Use open-time cgroup namespace for process migration perm checks Michal Koutný
2021-12-13 19:18 ` [PATCH 4/6] selftests: cgroup: Make cg_create() use 0755 for permission instead of 0644 Tejun Heo
2021-12-14 17:04   ` Michal Koutný
2021-12-13 19:18 ` [PATCH 5/6] selftests: cgroup: Test open-time credential usage for migration checks Tejun Heo
2021-12-14 17:04   ` Michal Koutný
2021-12-13 19:18 ` [PATCH 6/6] selftests: cgroup: Test open-time cgroup namespace " Tejun Heo
2021-12-14 17:04   ` Michal Koutný
2022-01-06 21:05 ` [PATCHSET v2 cgroup/for-5.16-fixes] cgroup: Use open-time creds and namespace for migration perm checks Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2021-12-09 21:47 [PATCHSET " Tejun Heo
2021-12-09 21:47 ` [PATCH 3/6] cgroup: Use open-time cgroup namespace for process " Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211213191833.916632-4-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=ebiederm@xmission.com \
    --cc=jannh@google.com \
    --cc=jnewsome@torproject.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@fb.com \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mkoutny@suse.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=security@kernel.org \
    --cc=syzbot+50f5cf33a284ce738b62@syzkaller.appspotmail.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox