This fixes the case where a signal that was sent by a user-namespaced
process appears to come from a different uid. This happens if the following
conditions are met:
- sender is in a user namespace
- sender's uid isn't mapped into sender's user namespace
- target is in the init user namespace
In this case, the sender's uid is first mapped down to the overflowuid via
from_kuid_munged() in sys_kill(), then mapped back up to a kuid (-1) and
down into the init namespace (overflowuid again) in
userns_fixup_signal_uid(). To observe this behavior, run the following
program:
=======================================
$ cat signal_userns_poc.c
#define _GNU_SOURCE
#include <sched.h>
#include <err.h>
#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <sys/wait.h>
void handle_sigusr1(int sig, siginfo_t *info, void *uctx) {
printf("got SIGUSR1 from uid %d\n", info->si_uid);
}
int main(void) {
struct sigaction act = {
.sa_sigaction = handle_sigusr1,
.sa_flags = SA_SIGINFO
};
if (sigaction(SIGUSR1, &act, NULL))
err(1, "sigaction");
pid_t parent = getpid();
pid_t child = fork();
if (child == -1)
err(1, "fork");
if (child == 0) {
if (unshare(CLONE_NEWUSER))
err(1, "unshare");
kill(parent, SIGUSR1);
return 0;
}
int status;
waitpid(child, &status, 0);
return 0;
}
$ gcc -o signal_userns_poc signal_userns_poc.c -Wall
$ ./signal_userns_poc
got SIGUSR1 from uid 65534
=======================================
While this is probably harmless - incorrect sender uids will always be
either the overflowuid or a uid that was mapped to the overflowuid in the
sender's namespace -, it's still an unnecessary loss of information.
Fix it by always storing the kuid in non-SI_KERNEL signals until the final
uid is computed in userns_fixup_signal_uid().
Signed-off-by: Jann Horn <j...@thejh.net>
---
ipc/mqueue.c | 9 +--------
kernel/signal.c | 22 +++++++++++-----------
2 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 8cbd6e6894d5..8bb0dda68fec 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -73,7 +73,6 @@ struct mqueue_inode_info {
struct sigevent notify;
struct pid *notify_owner;
- struct user_namespace *notify_user_ns;
struct user_struct *user; /* user who created, for accounting */
struct sock *notify_sock;
struct sk_buff *notify_cookie;
@@ -240,7 +239,6 @@ static struct inode *mqueue_get_inode(struct super_block
*sb,
INIT_LIST_HEAD(&info->e_wait_q[0].list);
INIT_LIST_HEAD(&info->e_wait_q[1].list);
info->notify_owner = NULL;
- info->notify_user_ns = NULL;
info->qsize = 0;
info->user = NULL; /* set when all is ok */
info->msg_tree = RB_ROOT;
@@ -643,7 +641,7 @@ static void __do_notify(struct mqueue_inode_info *info)
rcu_read_lock();
sig_i.si_pid = task_tgid_nr_ns(current,
ns_of_pid(info->notify_owner));
- sig_i.si_uid = from_kuid_munged(info->notify_user_ns,
current_uid());
+ sig_i.si_uid = from_kuid(&init_user_ns, current_uid());
rcu_read_unlock();
kill_pid_info(info->notify.sigev_signo,
@@ -656,9 +654,7 @@ static void __do_notify(struct mqueue_inode_info *info)
}
/* after notification unregisters process */
put_pid(info->notify_owner);
- put_user_ns(info->notify_user_ns);
info->notify_owner = NULL;
- info->notify_user_ns = NULL;
}
wake_up(&info->wait_q);
}
@@ -683,9 +679,7 @@ static void remove_notification(struct mqueue_inode_info
*info)
netlink_sendskb(info->notify_sock, info->notify_cookie);
}
put_pid(info->notify_owner);
- put_user_ns(info->notify_user_ns);
info->notify_owner = NULL;
- info->notify_user_ns = NULL;
}
static int mq_attr_ok(struct ipc_namespace *ipc_ns, struct mq_attr *attr)
@@ -1301,7 +1295,6 @@ SYSCALL_DEFINE2(mq_notify, mqd_t, mqdes,
}
info->notify_owner = get_pid(task_tgid(current));
- info->notify_user_ns = get_user_ns(current_user_ns());
inode->i_atime = inode->i_ctime = current_time(inode);
}
spin_unlock(&info->lock);
diff --git a/kernel/signal.c b/kernel/signal.c
index 75761acc77cf..b0afbf9a30dd 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -951,7 +951,7 @@ static inline int legacy_queue(struct sigpending *signals,
int sig)
#ifdef CONFIG_USER_NS
static inline void userns_fixup_signal_uid(struct siginfo *info, struct
task_struct *t)
{
- if (current_user_ns() == task_cred_xxx(t, user_ns))
+ if (&init_user_ns == task_cred_xxx(t, user_ns))
return;
if (SI_FROMKERNEL(info))
@@ -959,7 +959,7 @@ static inline void userns_fixup_signal_uid(struct siginfo
*info, struct task_str
rcu_read_lock();
info->si_uid = from_kuid_munged(task_cred_xxx(t, user_ns),
- make_kuid(current_user_ns(),
info->si_uid));
+ make_kuid(&init_user_ns, info->si_uid));
rcu_read_unlock();
}
#else
@@ -1027,7 +1027,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 = from_kuid_munged(current_user_ns(),
current_uid());
+ q->info.si_uid = from_kuid(&init_user_ns,
+ current_uid());
break;
case (unsigned long) SEND_SIG_PRIV:
q->info.si_signo = sig;
@@ -1609,8 +1610,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
*/
rcu_read_lock();
info.si_pid = task_pid_nr_ns(tsk, task_active_pid_ns(tsk->parent));
- info.si_uid = from_kuid_munged(task_cred_xxx(tsk->parent, user_ns),
- task_uid(tsk));
+ info.si_uid = from_kuid(&init_user_ns, task_uid(tsk));
rcu_read_unlock();
task_cputime(tsk, &utime, &stime);
@@ -1695,7 +1695,7 @@ static void do_notify_parent_cldstop(struct task_struct
*tsk,
*/
rcu_read_lock();
info.si_pid = task_pid_nr_ns(tsk, task_active_pid_ns(parent));
- info.si_uid = from_kuid_munged(task_cred_xxx(parent, user_ns),
task_uid(tsk));
+ info.si_uid = from_kuid(&init_user_ns, task_uid(tsk));
rcu_read_unlock();
task_cputime(tsk, &utime, &stime);
@@ -1904,7 +1904,7 @@ static void ptrace_do_notify(int signr, int exit_code,
int why)
info.si_signo = signr;
info.si_code = exit_code;
info.si_pid = task_pid_vnr(current);
- info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
+ info.si_uid = from_kuid(&init_user_ns, current_uid());
/* Let the debugger run. */
ptrace_stop(exit_code, why, 1, &info);
@@ -2113,8 +2113,8 @@ static int ptrace_signal(int signr, siginfo_t *info)
info->si_code = SI_USER;
rcu_read_lock();
info->si_pid = task_pid_vnr(current->parent);
- info->si_uid = from_kuid_munged(current_user_ns(),
- task_uid(current->parent));
+ info->si_uid = from_kuid(&init_user_ns,
+ task_uid(current->parent));
rcu_read_unlock();
}
@@ -2852,7 +2852,7 @@ SYSCALL_DEFINE2(kill, pid_t, pid, int, sig)
info.si_errno = 0;
info.si_code = SI_USER;
info.si_pid = task_tgid_vnr(current);
- info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
+ info.si_uid = from_kuid(&init_user_ns, current_uid());
return kill_something_info(sig, &info, pid);
}
@@ -2895,7 +2895,7 @@ static int do_tkill(pid_t tgid, pid_t pid, int sig)
info.si_errno = 0;
info.si_code = SI_TKILL;
info.si_pid = task_tgid_vnr(current);
- info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
+ info.si_uid = from_kuid(&init_user_ns, current_uid());
return do_send_specific(tgid, pid, sig, &info);
}
--
2.1.4
