From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>,
Brad Spengler <spender@grsecurity.net>,
Colin Walters <walters@redhat.com>,
Pavel Emelyanov <xemul@parallels.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 3/3] fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks
Date: Thu, 22 Aug 2013 19:10:04 +0200 [thread overview]
Message-ID: <20130822171004.GA20324@redhat.com> (raw)
In-Reply-To: <20130822170939.GA20296@redhat.com>
do_fork() denies CLONE_THREAD | CLONE_PARENT if NEWUSER | NEWPID.
Then later copy_process() denies CLONE_SIGHAND if the new process
will be in a different pid namespace (task_active_pid_ns() doesn't
match current->nsproxy->pid_ns).
This looks confusing and inconsistent. CLONE_NEWPID is very similar
to the case when ->pid_ns was already unshared, we want the same
restrictions so copy_process() should also nack CLONE_PARENT.
And it would be better to deny CLONE_NEWUSER && CLONE_SIGHAND as
well just for consistency.
Kill the "CLONE_NEWUSER | CLONE_NEWPID" check in do_fork() and
change copy_process() to the same check along with nsproxy->pid_ns
we already have.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/fork.c | 22 ++++++++--------------
1 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index 8d56338..fae2ff7 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1173,12 +1173,15 @@ static struct task_struct *copy_process(unsigned long clone_flags,
return ERR_PTR(-EINVAL);
/*
- * If the new process will be in a different pid namespace don't
- * allow the creation of threads, or share the signal handlers.
+ * If the new process will be in a different pid or user namespace
+ * don't allow the creation of threads, or share the signal handlers,
+ * or share the parent.
*/
- if ((clone_flags & CLONE_SIGHAND) &&
- (task_active_pid_ns(current) != current->nsproxy->pid_ns))
- return ERR_PTR(-EINVAL);
+ if (clone_flags & (CLONE_SIGHAND | CLONE_PARENT)) {
+ if ((clone_flags & (CLONE_NEWUSER | CLONE_NEWPID)) ||
+ (task_active_pid_ns(current) != current->nsproxy->pid_ns))
+ return ERR_PTR(-EINVAL);
+ }
retval = security_task_create(clone_flags);
if (retval)
@@ -1575,15 +1578,6 @@ long do_fork(unsigned long clone_flags,
long nr;
/*
- * Do some preliminary argument and permissions checking before we
- * actually start allocating stuff
- */
- if (clone_flags & (CLONE_NEWUSER | CLONE_NEWPID)) {
- if (clone_flags & (CLONE_THREAD|CLONE_PARENT))
- return -EINVAL;
- }
-
- /*
* Determine whether and which event to report to ptracer. When
* called from kernel_thread or CLONE_UNTRACED is explicitly
* requested, no event is reported; otherwise, report if the event
--
1.5.5.1
next prev parent reply other threads:[~2013-08-22 17:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-22 17:09 [PATCH 0/3] namespaces && fork fixes/cleanups Oleg Nesterov
2013-08-22 17:09 ` [PATCH 1/3] pidns: fix vfork() after unshare(CLONE_NEWPID) Oleg Nesterov
2013-08-22 17:59 ` Andy Lutomirski
2013-08-22 18:22 ` Oleg Nesterov
2013-08-22 17:10 ` [PATCH 2/3] pidns: kill the unnecessary CLONE_NEWPID in copy_process() Oleg Nesterov
2013-08-22 18:05 ` Andy Lutomirski
2013-08-22 17:10 ` Oleg Nesterov [this message]
2013-08-22 18:10 ` [PATCH 3/3] fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks Andy Lutomirski
2013-08-22 18:15 ` Oleg Nesterov
2013-08-22 18:29 ` Andy Lutomirski
2013-08-22 18:32 ` Oleg Nesterov
2013-08-22 19:11 ` Andy Lutomirski
2013-08-23 13:59 ` Oleg Nesterov
2013-08-23 17:42 ` Andy Lutomirski
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=20130822171004.GA20324@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=spender@grsecurity.net \
--cc=torvalds@linux-foundation.org \
--cc=walters@redhat.com \
--cc=xemul@parallels.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.