Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1202088
| From | ebiederm@xmission.com (Eric W. Biederman) |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] user_ns: use correct check for single-threadedness |
| Date | 2015-08-06 23:30 +0200 |
| Message-ID | <pUATD-4f3-7@gated-at.bofh.it> (permalink) |
| References | (2 earlier) <pRlo5-5DA-15@gated-at.bofh.it> <pUaFQ-811-19@gated-at.bofh.it> <pUbix-xJ-7@gated-at.bofh.it> <pUc4W-1s6-15@gated-at.bofh.it> <pUv7z-4og-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Oleg Nesterov <oleg@redhat.com> writes:
> On 08/05, Eric W. Biederman wrote:
>>
>> So I have to ask.
>
> I hope you are asking someone else, not me ;) I never understood what
> exactly we try to restrict and why.
I think I was just asking rhetorically, and asking myself.
>> Is it possible to rework these checks such that we
>> look at the sighand struct and signal sharing handling sharing instead
>> of the count on the mm_struct?
>
> Then why we can't simply check thread_group_empty() == T ? Why should we
> worry about CLONE_SIGHAND at all?
<rant>
thread_group_empty() for a thread group with a single member is a
confusing name.
</rant>
CLONE_SIGHAND is just a hair excessive, you have to at least look at
your per thread register to see which namespace to interpret the values
of the signals in. I suppose that is confusing but not totally fatal.
Without changing the semantics, just correcting the implementation
of the code we have now this is the code I get:
diff --git a/kernel/fork.c b/kernel/fork.c
index 1bfefc6f96a4..c95757c15fcb 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1866,13 +1866,13 @@ static int check_unshare_flags(unsigned long unshare_flags)
CLONE_NEWUSER|CLONE_NEWPID))
return -EINVAL;
/*
- * Not implemented, but pretend it works if there is nothing to
- * unshare. Note that unsharing CLONE_THREAD or CLONE_SIGHAND
- * needs to unshare vm.
+ * Not implemented, but pretend it works if there is nothing
+ * to unshare. Note that unsharing the address space or the
+ * signal handlers also need to unshare the signal queues (aka
+ * CLONE_THREAD).
*/
- if (unshare_flags & (CLONE_THREAD | CLONE_SIGHAND | CLONE_VM)) {
- /* FIXME: get_task_mm() increments ->mm_users */
- if (atomic_read(¤t->mm->mm_users) > 1)
+ if (unshare_flags & CLONE_THREAD) {
+ if (!thread_group_empty(current))
return -EINVAL;
}
@@ -1936,21 +1936,23 @@ SYSCALL_DEFINE1(unshare, unsigned long, unshare_flags)
int err;
/*
- * If unsharing a user namespace must also unshare the thread.
+ * If unsharing a user namespace must also unshare the signal
+ * handlers and unshare the filesystem root and working
+ * directories.
*/
if (unshare_flags & CLONE_NEWUSER)
- unshare_flags |= CLONE_THREAD | CLONE_FS;
- /*
- * If unsharing a thread from a thread group, must also unshare vm.
- */
- if (unshare_flags & CLONE_THREAD)
- unshare_flags |= CLONE_VM;
+ unshare_flags |= CLONE_SIGHAND | CLONE_FS;
/*
* If unsharing vm, must also unshare signal handlers.
*/
if (unshare_flags & CLONE_VM)
unshare_flags |= CLONE_SIGHAND;
/*
+ * If unsharing a signal handlers, must also unshare the signal queues.
+ */
+ if (unshare_flags & CLONE_SIGHAND)
+ unshare_flags |= CLONE_THREAD;
+ /*
* If unsharing namespace, must also unshare filesystem information.
*/
if (unshare_flags & CLONE_NEWNS)
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 4109f8320684..45a5cbf97715 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -976,8 +976,8 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
if (user_ns == current_user_ns())
return -EINVAL;
- /* Threaded processes may not enter a different user namespace */
- if (atomic_read(¤t->mm->mm_users) > 1)
+ /* Shared signal handlers must live in the same user namespace */
+ if (atomic_read(¤t->sighand->count) > 1)
return -EINVAL;
if (current->fs->users != 1)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Re: [PATCH] user_ns: use correct check for single-threadedness Oleg Nesterov <oleg@redhat.com> - 2015-08-05 19:30 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness ebiederm@xmission.com (Eric W. Biederman) - 2015-08-05 20:10 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness ebiederm@xmission.com (Eric W. Biederman) - 2015-08-05 21:00 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness Oleg Nesterov <oleg@redhat.com> - 2015-08-06 16:10 +0200
[PATCH 0/2] userns: Creation logic fixes ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 03:30 +0200
[PATCH 2/2] userns,pidns: Force thread group sharing, not signal handler sharing ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 03:40 +0200
Re: [PATCH 2/2] userns,pidns: Force thread group sharing, not signal handler sharing Oleg Nesterov <oleg@redhat.com> - 2015-08-12 19:30 +0200
[PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 03:40 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-12 20:00 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 20:50 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-13 15:00 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-13 17:50 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-13 18:20 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-13 18:40 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-13 19:00 +0200
Re: [PATCH 1/2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-14 20:10 +0200
[PATCH v2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 22:10 +0200
Re: [PATCH v2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-13 15:00 +0200
Re: [PATCH v2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-13 18:10 +0200
Re: [PATCH v2] unshare: Unsharing a thread does not require unsharing a vm Oleg Nesterov <oleg@redhat.com> - 2015-08-13 18:40 +0200
Re: [PATCH v2] unshare: Unsharing a thread does not require unsharing a vm ebiederm@xmission.com (Eric W. Biederman) - 2015-08-13 18:50 +0200
Re: [PATCH 0/2] userns: Creation logic fixes Kees Cook <keescook@chromium.org> - 2015-08-12 08:30 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 03:30 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness Oleg Nesterov <oleg@redhat.com> - 2015-08-12 16:50 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness ebiederm@xmission.com (Eric W. Biederman) - 2015-08-12 17:20 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness Oleg Nesterov <oleg@redhat.com> - 2015-08-06 17:20 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness ebiederm@xmission.com (Eric W. Biederman) - 2015-08-06 23:30 +0200
Re: [PATCH] user_ns: use correct check for single-threadedness Oleg Nesterov <oleg@redhat.com> - 2015-08-07 04:50 +0200
csiph-web