diff options
author | Oleg Nesterov <oleg@redhat.com> | 2013-09-11 17:19:41 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2013-09-11 18:56:20 -0400 |
commit | 40a0d32d1eaffe6aac7324ca92604b6b3977eb0e (patch) | |
tree | c736993570ca2b16f0b7963226fda1947ec2c6b8 /kernel/fork.c | |
parent | 5167246a8ad617df55717c2d901da5e2aedffcfa (diff) |
fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks
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 do the same check along with ->pid_ns check we already
have.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Andy Lutomirski <luto@amacapital.net>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Colin Walters <walters@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'kernel/fork.c')
-rw-r--r-- | kernel/fork.c | 24 |
1 files changed, 9 insertions, 15 deletions
diff --git a/kernel/fork.c b/kernel/fork.c index 68d508f2bfba..84703db06cf3 100644 --- a/kernel/fork.c +++ b/kernel/fork.c | |||
@@ -1173,13 +1173,16 @@ static struct task_struct *copy_process(unsigned long clone_flags, | |||
1173 | return ERR_PTR(-EINVAL); | 1173 | return ERR_PTR(-EINVAL); |
1174 | 1174 | ||
1175 | /* | 1175 | /* |
1176 | * If the new process will be in a different pid namespace don't | 1176 | * If the new process will be in a different pid or user namespace |
1177 | * allow it to share a thread group or signal handlers with the | 1177 | * do not allow it to share a thread group or signal handlers or |
1178 | * forking task. | 1178 | * parent with the forking task. |
1179 | */ | 1179 | */ |
1180 | if ((clone_flags & CLONE_SIGHAND) && (task_active_pid_ns(current) != | 1180 | if (clone_flags & (CLONE_SIGHAND | CLONE_PARENT)) { |
1181 | current->nsproxy->pid_ns_for_children)) | 1181 | if ((clone_flags & (CLONE_NEWUSER | CLONE_NEWPID)) || |
1182 | return ERR_PTR(-EINVAL); | 1182 | (task_active_pid_ns(current) != |
1183 | current->nsproxy->pid_ns_for_children)) | ||
1184 | return ERR_PTR(-EINVAL); | ||
1185 | } | ||
1183 | 1186 | ||
1184 | retval = security_task_create(clone_flags); | 1187 | retval = security_task_create(clone_flags); |
1185 | if (retval) | 1188 | if (retval) |
@@ -1576,15 +1579,6 @@ long do_fork(unsigned long clone_flags, | |||
1576 | long nr; | 1579 | long nr; |
1577 | 1580 | ||
1578 | /* | 1581 | /* |
1579 | * Do some preliminary argument and permissions checking before we | ||
1580 | * actually start allocating stuff | ||
1581 | */ | ||
1582 | if (clone_flags & (CLONE_NEWUSER | CLONE_NEWPID)) { | ||
1583 | if (clone_flags & (CLONE_THREAD|CLONE_PARENT)) | ||
1584 | return -EINVAL; | ||
1585 | } | ||
1586 | |||
1587 | /* | ||
1588 | * Determine whether and which event to report to ptracer. When | 1582 | * Determine whether and which event to report to ptracer. When |
1589 | * called from kernel_thread or CLONE_UNTRACED is explicitly | 1583 | * called from kernel_thread or CLONE_UNTRACED is explicitly |
1590 | * requested, no event is reported; otherwise, report if the event | 1584 | * requested, no event is reported; otherwise, report if the event |