From: "Serge E. Hallyn" <serge.hallyn@canonical.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: lkml <linux-kernel@vger.kernel.org>,
richard@nod.at, Andrew Morton <akpm@google.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Tejun Heo <tj@kernel.org>,
serge@hallyn.com
Subject: Re: [PATCH] user namespace: make signal.c respect user namespaces
Date: Sun, 25 Sep 2011 15:17:23 -0500 [thread overview]
Message-ID: <20110925201723.GA5288@sergelap> (raw)
In-Reply-To: <20110924163709.GA6776@redhat.com>
Quoting Oleg Nesterov (oleg@redhat.com):
> On 09/23, Serge E. Hallyn wrote:
> >
> > Quoting Oleg Nesterov (oleg@redhat.com):
> > > On 09/23, Serge E. Hallyn wrote:
> > > >
> > > > It looks like I can fix all the
> > > > cases
> > >
> > > except ptrace_signal(). Although we can simply ignore this case, imho.
> >
> > ptrace_signal() calls send_signal() though.
>
> Confused... I meant the "if (signr != info->si_signo)" case. This is
> simple, and I only meant that this case is not that important.
Yes, that's the case I was talking about. That then proceeds through
send_signal().
It appears to be the only (user) caller of send_signal() where the
info->si_pid and info->si_uid are not filled in from current. But I
just realized that it's not a problem for the same reason that it isn't
for the analogous pid code - since the target task is current, the test
for whether current userns != target userns will always return false, so
the code simply won't trigger :)
+ if (si_fromuser(info) && current_user_ns() != task_cred_xxx(t, user_ns)) {
+ rcu_read_lock();
+ if (!map_cred_ns(current_cred(), task_cred_xxx(t, user_ns)))
+ userns_rel = USERNS_ANCESTOR;
+ else
+ userns_rel = USERNS_OTHER;
+ rcu_read_unlock();
+ }
The whole new patch (so far only compile-tested) is below.
> > > > by checking whether si_fromuser(info)
> > >
> > > I am not sure... sys_rt_queueinfo() is nasty. Plus we have to handle
> > > the "fromkernel" case too. May be we can ignore this too.
> >
> > sys_rt_tgsigqueueinfo() still seems to go through send_signal().
>
> Yes. But how can you fix si_uid? We do not even know if it exists.
> Please look at siginfo/_uid, there is a union. We can't know what
> the caller of sys_rt_sigqueueinfo() puts in this location.
But it's a union alongside the pid. I don't understand the callers
well enough to say whether there is a bug in the pid handling. I'll
certainly try to take a wider look next week, but it seems to me
at least this patch should bring the uid namespacing code up to par
with the pid namespacing code for signals.
(patch follows - I'll resend if it passes more strenuous testing)
thanks,
-serge
From d73866bec4032bed383ba977d11f97d0ae3e9596 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn@canonical.com>
Date: Mon, 19 Sep 2011 18:07:55 +0100
Subject: [PATCH 1/1] user namespace: make signal.c respect user namespaces (v3)
__send_signal: convert the uid being sent in SI_USER to the target task's
user namespace.
do_notify_parent and do_notify_parent_cldstop: map task's uid to parent's
user namespace
ptrace_signal: map parent's uid into current's user namespace before
including in signal to current.
Changelog:
Sep 20: Inspired by Oleg's suggestion, define map_cred_ns() helper to
simplify callers and help make clear what we are translating
(which uid into which namespace). Passing the target task would
make callers even easier to read, but we pass in user_ns because
current_user_ns() != task_cred_xxx(current, user_ns).
Sep 20: As recommended by Oleg, also put task_pid_vnr() under rcu_read_lock
in ptrace_signal().
Sep 23: In send_signal(), detect when (user) signal is coming from an
ancestor or unrelated user namespace. Pass that on to __send_signal,
which sets si_uid to 0 or overflowuid if needed.
Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Tejun Heo <tj@kernel.org>
---
kernel/signal.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 291c970..d2ee505 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -27,6 +27,7 @@
#include <linux/capability.h>
#include <linux/freezer.h>
#include <linux/pid_namespace.h>
+#include <linux/user_namespace.h>
#include <linux/nsproxy.h>
#define CREATE_TRACE_POINTS
#include <trace/events/signal.h>
@@ -37,6 +38,14 @@
#include <asm/siginfo.h>
#include "audit.h" /* audit_signal_info() */
+extern int overflowuid;
+/* sender and target have same user namespace */
+#define USERNS_SAME 0
+/* sendor's userns is ancestor of target's */
+#define USERNS_ANCESTOR 1
+/* sendor's userns is child of target's or unrelated to it */
+#define USERNS_OTHER 2
+
/*
* SLAB caches for signal bits.
*/
@@ -1019,8 +1028,17 @@ static inline int legacy_queue(struct sigpending *signals, int sig)
return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
}
+/*
+ * map the uid in struct cred into user namespace *ns
+ */
+static inline uid_t map_cred_ns(const struct cred *cred,
+ struct user_namespace *ns)
+{
+ return user_ns_map_uid(ns, cred, cred->uid);
+}
+
static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
- int group, int from_ancestor_ns)
+ int group, int from_ancestor_ns, int userns_rel)
{
struct sigpending *pending;
struct sigqueue *q;
@@ -1073,7 +1091,8 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
q->info.si_code = SI_USER;
q->info.si_pid = task_tgid_nr_ns(current,
task_active_pid_ns(t));
- q->info.si_uid = current_uid();
+ q->info.si_uid = map_cred_ns(current_cred(),
+ task_cred_xxx(t, user_ns));
break;
case (unsigned long) SEND_SIG_PRIV:
q->info.si_signo = sig;
@@ -1086,6 +1105,16 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
copy_siginfo(&q->info, info);
if (from_ancestor_ns)
q->info.si_pid = 0;
+ switch (userns_rel) {
+ case USERNS_SAME:
+ break;
+ case USERNS_ANCESTOR:
+ q->info.si_uid = 0;
+ break;
+ default:
+ q->info.si_uid = overflowuid;
+ break;
+ }
break;
}
} else if (!is_si_special(info)) {
@@ -1117,13 +1146,24 @@ static int send_signal(int sig, struct siginfo *info, struct task_struct *t,
int group)
{
int from_ancestor_ns = 0;
+ int userns_rel = USERNS_SAME;
#ifdef CONFIG_PID_NS
from_ancestor_ns = si_fromuser(info) &&
!task_pid_nr_ns(current, task_active_pid_ns(t));
#endif
+#ifdef CONFIG_USER_NS
+ if (si_fromuser(info) && current_user_ns() != task_cred_xxx(t, user_ns)) {
+ rcu_read_lock();
+ if (!map_cred_ns(current_cred(), task_cred_xxx(t, user_ns)))
+ userns_rel = USERNS_ANCESTOR;
+ else
+ userns_rel = USERNS_OTHER;
+ rcu_read_unlock();
+ }
+#endif
- return __send_signal(sig, info, t, group, from_ancestor_ns);
+ return __send_signal(sig, info, t, group, from_ancestor_ns, userns_rel);
}
static void print_fatal_signal(struct pt_regs *regs, int signr)
@@ -1375,7 +1415,7 @@ int kill_pid_info_as_uid(int sig, struct siginfo *info, struct pid *pid,
if (sig) {
if (lock_task_sighand(p, &flags)) {
- ret = __send_signal(sig, info, p, 1, 0);
+ ret = __send_signal(sig, info, p, 1, 0, USERNS_SAME);
unlock_task_sighand(p, &flags);
} else
ret = -ESRCH;
@@ -1618,7 +1658,8 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
*/
rcu_read_lock();
info.si_pid = task_pid_nr_ns(tsk, tsk->parent->nsproxy->pid_ns);
- info.si_uid = __task_cred(tsk)->uid;
+ info.si_uid = map_cred_ns(__task_cred(tsk),
+ task_cred_xxx(tsk->parent, user_ns));
rcu_read_unlock();
info.si_utime = cputime_to_clock_t(cputime_add(tsk->utime,
@@ -1703,7 +1744,8 @@ static void do_notify_parent_cldstop(struct task_struct *tsk,
*/
rcu_read_lock();
info.si_pid = task_pid_nr_ns(tsk, parent->nsproxy->pid_ns);
- info.si_uid = __task_cred(tsk)->uid;
+ info.si_uid = map_cred_ns(__task_cred(tsk),
+ task_cred_xxx(parent, user_ns));
rcu_read_unlock();
info.si_utime = cputime_to_clock_t(tsk->utime);
@@ -2121,8 +2163,11 @@ static int ptrace_signal(int signr, siginfo_t *info,
info->si_signo = signr;
info->si_errno = 0;
info->si_code = SI_USER;
+ rcu_read_lock();
info->si_pid = task_pid_vnr(current->parent);
- info->si_uid = task_uid(current->parent);
+ info->si_uid = map_cred_ns(__task_cred(current->parent),
+ current_user_ns());
+ rcu_read_unlock();
}
/* If the (new) signal is now blocked, requeue it. */
--
1.7.0.4
next prev parent reply other threads:[~2011-09-25 20:17 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-19 21:45 [PATCH] user namespace: make signal.c respect user namespaces Serge E. Hallyn
2011-09-19 21:47 ` [PATCH] user namespace: usb: make usb urbs user namespace aware Serge E. Hallyn
2011-09-20 13:17 ` Oleg Nesterov
2011-09-20 13:33 ` Serge E. Hallyn
2011-09-21 5:01 ` [PATCH] user namespace: usb: make usb urbs user namespace aware (v2) Serge E. Hallyn
2011-09-21 18:31 ` Oleg Nesterov
2011-09-21 19:12 ` Serge E. Hallyn
2011-09-21 19:18 ` Greg KH
2011-09-23 1:27 ` [PATCH resend] " Serge E. Hallyn
2011-09-23 15:48 ` Alan Stern
2011-09-23 16:06 ` Serge E. Hallyn
2011-09-23 16:21 ` Alan Stern
2011-09-23 17:22 ` Serge E. Hallyn
2011-09-23 18:35 ` Alan Stern
2011-09-20 12:22 ` [PATCH] user namespace: make signal.c respect user namespaces Oleg Nesterov
2011-09-20 12:44 ` Serge E. Hallyn
2011-09-20 13:41 ` Oleg Nesterov
2011-09-20 14:39 ` [PATCH 0/2] (Was: user namespace: make signal.c respect user namespaces) Oleg Nesterov
2011-09-20 14:39 ` [PATCH 1/2] creds: kill __task_cred()->task_is_dead() check Oleg Nesterov
2011-09-20 15:14 ` drivers/staging/usbip/ abuses task_is_dead/exit_state Oleg Nesterov
2011-09-20 18:38 ` Greg KH
2012-03-06 17:39 ` ping: " Oleg Nesterov
2012-03-06 19:30 ` Tobias Klauser
2012-03-08 18:57 ` Oleg Nesterov
2012-03-13 11:45 ` Tobias Klauser
2012-03-13 18:07 ` [PATCH] staging: usbip: fix the usage of kthread_stop() Oleg Nesterov
2012-04-01 23:17 ` Oleg Nesterov
2012-04-02 8:11 ` Tobias Klauser
2011-09-20 15:28 ` [PATCH 1/2] creds: kill __task_cred()->task_is_dead() check Paul E. McKenney
2011-09-20 15:40 ` Oleg Nesterov
2011-09-20 15:48 ` Paul E. McKenney
2011-09-20 14:39 ` [PATCH 2/2] creds: __task_cred(current) doesn't need rcu_read_lock_held() Oleg Nesterov
2011-09-20 15:07 ` Serge Hallyn
2011-09-20 15:35 ` Oleg Nesterov
2011-09-20 16:19 ` David Howells
2011-09-20 16:38 ` Oleg Nesterov
2011-09-20 16:50 ` David Howells
2011-09-20 17:13 ` Oleg Nesterov
2011-09-20 16:27 ` [PATCH 1/2] creds: kill __task_cred()->task_is_dead() check David Howells
2011-09-20 15:39 ` [PATCH] user namespace: make signal.c respect user namespaces Serge Hallyn
2011-09-20 16:24 ` Oleg Nesterov
2011-09-20 16:45 ` Serge E. Hallyn
2011-09-20 18:17 ` Oleg Nesterov
2011-09-21 5:00 ` [PATCH] user namespace: make signal.c respect user namespaces (v2) Serge E. Hallyn
2011-09-20 17:48 ` [PATCH] user namespace: make signal.c respect user namespaces Oleg Nesterov
2011-09-20 18:53 ` Serge E. Hallyn
2011-09-21 17:53 ` Oleg Nesterov
2011-09-22 15:23 ` Serge Hallyn
2011-09-23 16:31 ` Serge E. Hallyn
2011-09-23 17:36 ` Oleg Nesterov
2011-09-23 21:20 ` Serge E. Hallyn
2011-09-24 16:37 ` Oleg Nesterov
2011-09-25 20:17 ` Serge E. Hallyn [this message]
2011-09-26 16:06 ` Oleg Nesterov
2011-09-27 14:28 ` Serge Hallyn
2011-09-27 14:38 ` Oleg Nesterov
2011-09-27 15:27 ` Serge Hallyn
2011-09-27 17:12 ` Oleg Nesterov
2011-10-04 17:42 ` Serge E. Hallyn
2011-10-09 19:00 ` Oleg Nesterov
2011-10-11 13:08 ` Serge E. Hallyn
2011-10-08 20:02 ` Serge E. Hallyn
2011-10-09 19:03 ` Oleg Nesterov
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=20110925201723.GA5288@sergelap \
--to=serge.hallyn@canonical.com \
--cc=akpm@google.com \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=richard@nod.at \
--cc=serge@hallyn.com \
--cc=tj@kernel.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;
as well as URLs for NNTP newsgroup(s).