[PATCH] signal: fix overflow_uid signal sender

[Jann Horn]

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 <jann@xxxxxxxxx>
---
 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