LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: linux-kernel@vger.kernel.org
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Linux Containers <containers@lists.linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-arch@vger.kernel.org
Subject: [REVIEW][PATCH 23/26] signal: Move the computation of force into send_signal and correct it.
Date: Wed, 22 May 2019 19:39:13 -0500	[thread overview]
Message-ID: <20190523003916.20726-24-ebiederm@xmission.com> (raw)
In-Reply-To: <20190523003916.20726-1-ebiederm@xmission.com>

Forcing a signal or not allowing a pid namespace init to ignore
SIGKILL or SIGSTOP is more cleanly computed in send_signal.

There are two cases where we don't allow a pid namespace init
to ignore SIGKILL or SIGSTOP.  If the sending process is
from an ancestor pid namespace and as such is effectively
the god to the target process, and if the it is the kernel
that is sending the signal, not another application.

It is known that a process is from an ancestor pid namespace if
it can see it's target but it's target does not have a pid for
the sender in it's pid namespace.

It is know that a signal is sent from the kernel if si_code is set to
SI_KERNEL or info is SEND_SIG_PRIV (which ultimately generates
a signal with si_code == SI_KERNEL).

The only signals that matter are SIGKILL and SIGSTOP neither of
which can really be caught, and both of which always have a siginfo
layout that includes si_uid and si_pid.  Therefore we never need
to worry about forcing a signal when si_pid and si_uid are absent.

So handle the two special cases of info and the case when si_pid and
si_uid are present.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index b2f0cf3a68aa..0da35880261e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1057,7 +1057,7 @@ static inline bool legacy_queue(struct sigpending *signals, int sig)
 }
 
 static int __send_signal(int sig, struct kernel_siginfo *info, struct task_struct *t,
-			enum pid_type type, int from_ancestor_ns)
+			enum pid_type type, bool force)
 {
 	struct sigpending *pending;
 	struct sigqueue *q;
@@ -1067,8 +1067,7 @@ static int __send_signal(int sig, struct kernel_siginfo *info, struct task_struc
 	assert_spin_locked(&t->sighand->siglock);
 
 	result = TRACE_SIGNAL_IGNORED;
-	if (!prepare_signal(sig, t,
-			from_ancestor_ns || (info == SEND_SIG_PRIV)))
+	if (!prepare_signal(sig, t, force))
 		goto ret;
 
 	pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending;
@@ -1198,13 +1197,17 @@ static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
 static int send_signal(int sig, struct kernel_siginfo *info, struct task_struct *t,
 			enum pid_type type)
 {
-	int from_ancestor_ns = 0;
-
-#ifdef CONFIG_PID_NS
-	from_ancestor_ns = si_fromuser(info) &&
-			   !task_pid_nr_ns(current, task_active_pid_ns(t));
-#endif
-	if (!is_si_special(info) && has_si_pid_and_uid(info)) {
+	/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
+	bool force = false;
+
+	if (info == SEND_SIG_NOINFO) {
+		/* Force if sent from an ancestor pid namespace */
+		force = !task_pid_nr_ns(current, task_active_pid_ns(t));
+	} else if (info == SEND_SIG_PRIV) {
+		/* Don't ignore kernel generated signals */
+		force = true;
+	} else if (has_si_pid_and_uid(info)) {
+		/* SIGKILL and SIGSTOP is special or has ids */
 		struct user_namespace *t_user_ns;
 
 		rcu_read_lock();
@@ -1215,10 +1218,16 @@ static int send_signal(int sig, struct kernel_siginfo *info, struct task_struct
 		}
 		rcu_read_unlock();
 
-		if (!task_pid_nr_ns(current, task_active_pid_ns(t)))
+		/* A kernel generated signal? */
+		force = (info->si_code == SI_KERNEL);
+
+		/* From an ancestor pid namespace? */
+		if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
 			info->si_pid = 0;
+			force = true;
+		}
 	}
-	return __send_signal(sig, info, t, type, from_ancestor_ns);
+	return __send_signal(sig, info, t, type, force);
 }
 
 static void print_fatal_signal(int signr)
@@ -1509,7 +1518,7 @@ int kill_pid_usb_asyncio(int sig, int errno, sigval_t addr,
 
 	if (sig) {
 		if (lock_task_sighand(p, &flags)) {
-			ret = __send_signal(sig, &info, p, PIDTYPE_TGID, 0);
+			ret = __send_signal(sig, &info, p, PIDTYPE_TGID, false);
 			unlock_task_sighand(p, &flags);
 		} else
 			ret = -ESRCH;
-- 
2.21.0


  parent reply	other threads:[~2019-05-23  0:44 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23  0:38 [REVIEW][PATCH 00/26] signal: Remove task argument from force_sig_info Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 01/26] signal: Correct namespace fixups of si_pid and si_uid Eric W. Biederman
     [not found]   ` <20190529131503.F2AC221871@mail.kernel.org>
2019-05-29 15:18     ` Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 02/26] signal/ptrace: Simplify and fix PTRACE_KILL Eric W. Biederman
2019-05-29 14:35   ` Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 03/26] signal/arm64: Use force_sig not force_sig_fault for SIGKILL Eric W. Biederman
2019-05-23 10:17   ` Will Deacon
2019-05-23 14:59     ` Eric W. Biederman
2019-05-23 16:11     ` [REVIEW][PATCHv2 " Eric W. Biederman
2019-05-23 16:15       ` Will Deacon
2019-05-23 20:59         ` Eric W. Biederman
2019-05-24 10:00           ` Will Deacon
2019-05-24 22:36             ` Eric W. Biederman
2019-05-29 15:12               ` Will Deacon
2019-05-29 15:34                 ` Eric W. Biederman
2019-05-23 10:21   ` [REVIEW][PATCH " Dave Martin
2019-05-23 14:53     ` Eric W. Biederman
2019-05-23 16:12       ` Dave P Martin
2019-05-23 21:00         ` Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 04/26] signal/drbd: Use send_sig not force_sig Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 05/26] signal/bpfilter: Fix bpfilter_kernl to use " Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 06/26] signal/pid_namespace: Fix reboot_pid_ns " Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 07/26] signal/cifs: Fix cifs_put_tcp_session to call send_sig instead of force_sig Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 08/26] signal: Remove task parameter from force_sigsegv Eric W. Biederman
2019-05-23  0:38 ` [REVIEW][PATCH 09/26] signal: Remove task parameter from force_sig Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 10/26] signal: Remove task parameter from force_sig_mceerr Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 11/26] signal/x86: Remove task parameter from send_sigtrap Eric W. Biederman
2019-05-28 18:18   ` Thomas Gleixner
2019-05-23  0:39 ` [REVIEW][PATCH 12/26] signal/um: " Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 13/26] signal/sh: Remove tsk parameter from force_sig_info_fault Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 14/26] signal/riscv: Remove tsk parameter from do_trap Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 15/26] signal/nds32: Remove tsk parameter from send_sigtrap Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 16/26] signal/arm: Remove tsk parameter from ptrace_break Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 17/26] signal/arm: Remove tsk parameter from __do_user_fault Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 18/26] signal/unicore32: " Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 19/26] signal: Explicitly call force_sig_fault on current Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 20/26] signal: Use force_sig_fault_to_task for the two calls that don't deliver to current Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 21/26] signal: Remove the task parameter from force_sig_fault Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 22/26] signal: Properly set TRACE_SIGNAL_LOSE_INFO in __send_signal Eric W. Biederman
2019-05-23  0:39 ` Eric W. Biederman [this message]
2019-05-23  0:39 ` [REVIEW][PATCH 24/26] signal: Generate the siginfo in force_sig Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 25/26] signal: Factor force_sig_info_to_task out of force_sig_info Eric W. Biederman
2019-05-23  0:39 ` [REVIEW][PATCH 26/26] signal: Remove the signal number and task parameters from force_sig_info Eric W. Biederman
2019-05-24 23:35 ` [REVIEW][PATCH 00/26] signal: Remove task argument " Eric W. Biederman
2019-05-29 15:37 ` Eric W. Biederman

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=20190523003916.20726-24-ebiederm@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --subject='Re: [REVIEW][PATCH 23/26] signal: Move the computation of force into send_signal and correct it.' \
    /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

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).