[PATCH] signal: prevent evasion of SA_IMMUTABLE signals

[Oleg Nesterov <oleg@redhat.com>]

force_sig_info_to_task(HANDLER_EXIT) sets SA_IMMUTABLE to ensure a forced
fatal signal cannot be ignored or caught by userspace; it must always
terminate the target. However, if get_signal() dequeues another synchronous
signal first, and that signal has a handler and its sa_mask includes the
fatal SA_IMMUTABLE signal, the task can return to userspace and survive.

So dequeue_synchronous_signal() must always dequeue an SA_IMMUTABLE signal
first. But it relies on the SI_FROMKERNEL() check and picks the first one
it sees in pending->list, and thus we have the following problems:

- If the same signal was already pending and blocked, the new siginfo
  with .si_code > 0 will be lost.

  Change __send_signal_locked() to bypass the legacy_queue() check in
  this case.

- If force_sig_info_to_task() races with another synchronous/SI_FROMKERNEL
  signal, that signal can be picked first.

  Change __send_signal_locked() to add an SA_IMMUTABLE at the start
  of pending->list.

- SA_IMMUTABLE implies override_rlimit == true, but GFP_ATOMIC can fail
  anyway.

  Change __send_signal_locked() to escalate to SIGKILL in this (very
  unlikely) case.

  Not perfect and perhaps deserves WARN() or pr_warn_ratelimited(), but
  better than nothing.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 9924489c43a5..e4605daa8d04 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1034,6 +1034,11 @@ static void complete_signal(int sig, struct task_struct *p, enum pid_type type)
 	return;
 }
 
+static inline bool sa_immutable(struct sighand_struct *sighand, int sig)
+{
+	return sighand->action[sig - 1].sa.sa_flags & SA_IMMUTABLE;
+}
+
 static inline bool legacy_queue(struct sigpending *signals, int sig)
 {
 	return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
@@ -1042,6 +1047,7 @@ static inline bool legacy_queue(struct sigpending *signals, int sig)
 static int __send_signal_locked(int sig, struct kernel_siginfo *info,
 				struct task_struct *t, enum pid_type type, bool force)
 {
+	bool immutable = sa_immutable(t->sighand, sig);
 	struct sigpending *pending;
 	struct sigqueue *q;
 	int override_rlimit;
@@ -1055,12 +1061,12 @@ static int __send_signal_locked(int sig, struct kernel_siginfo *info,
 
 	pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending;
 	/*
-	 * Short-circuit ignored signals and support queuing
-	 * exactly one non-rt signal, so that we can get more
-	 * detailed information about the cause of the signal.
+	 * Queue exactly one non-rt signal so that we can get more
+	 * detailed information about the cause. But we must never
+	 * lose the siginfo for an SA_IMMUTABLE signal.
 	 */
 	result = TRACE_SIGNAL_ALREADY_PENDING;
-	if (legacy_queue(pending, sig))
+	if (legacy_queue(pending, sig) && !immutable)
 		goto ret;
 
 	result = TRACE_SIGNAL_DELIVERED;
@@ -1087,7 +1093,12 @@ static int __send_signal_locked(int sig, struct kernel_siginfo *info,
 	q = sigqueue_alloc(sig, t, GFP_ATOMIC, override_rlimit);
 
 	if (q) {
-		list_add_tail(&q->list, &pending->list);
+		/* Ensure dequeue_synchronous_signal() sees SA_IMMUTABLE first */
+		if (immutable)
+			list_add(&q->list, &pending->list);
+		else
+			list_add_tail(&q->list, &pending->list);
+
 		switch ((unsigned long) info) {
 		case (unsigned long) SEND_SIG_NOINFO:
 			clear_siginfo(&q->info);
@@ -1130,6 +1141,9 @@ static int __send_signal_locked(int sig, struct kernel_siginfo *info,
 		 * send the signal, but the *info bits are lost.
 		 */
 		result = TRACE_SIGNAL_LOSE_INFO;
+		/* The task must not escape SA_IMMUTABLE; escalate to SIGKILL */
+		if (immutable)
+			sig = SIGKILL;
 	}
 
 out_set:
-- 
2.52.0