All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: <linux-kernel@vger.kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Al Viro <viro@ZenIV.linux.org.uk>, <linux-api@vger.kernel.org>,
	Kees Cook <keescook@chromium.org>
Subject: [PATCH 1/6] signal: Remove the bogus sigkill_pending in ptrace_stop
Date: Thu, 23 Sep 2021 19:09:34 -0500	[thread overview]
Message-ID: <87pmsyx29t.fsf@disp2133> (raw)
In-Reply-To: <87v92qx2c6.fsf@disp2133> (Eric W. Biederman's message of "Thu, 23 Sep 2021 19:08:09 -0500")


The existence of sigkill_pending is a little silly as it is
functionally a duplicate of fatal_signal_pending that is used in
exactly one place.

Checking for pending fatal signals and returning early in ptrace_stop
is actively harmful.  It casues the ptrace_stop called by
ptrace_signal to return early before setting current->exit_code.
Later when ptrace_signal reads the signal number from
current->exit_code is undefined, making it unpredictable what will
happen.

Instead rely on the fact that schedule will not sleep if there is a
pending signal that can awaken a task.

Removing the explict sigkill_pending test fixes fixes ptrace_signal
when ptrace_stop does not stop because current->exit_code is always
set to to signr.

Cc: stable@vger.kernel.org
Fixes: 3d749b9e676b ("ptrace: simplify ptrace_stop()->sigkill_pending() path")
Fixes: 1a669c2f16d4 ("Add arch_ptrace_stop")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 952741f6d0f9..9f2dc9cf3208 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2182,15 +2182,6 @@ static inline bool may_ptrace_stop(void)
 	return true;
 }
 
-/*
- * Return non-zero if there is a SIGKILL that should be waking us up.
- * Called with the siglock held.
- */
-static bool sigkill_pending(struct task_struct *tsk)
-{
-	return sigismember(&tsk->pending.signal, SIGKILL) ||
-	       sigismember(&tsk->signal->shared_pending.signal, SIGKILL);
-}
 
 /*
  * This must be called with current->sighand->siglock held.
@@ -2217,17 +2208,16 @@ static void ptrace_stop(int exit_code, int why, int clear_code, kernel_siginfo_t
 		 * calling arch_ptrace_stop, so we must release it now.
 		 * To preserve proper semantics, we must do this before
 		 * any signal bookkeeping like checking group_stop_count.
-		 * Meanwhile, a SIGKILL could come in before we retake the
-		 * siglock.  That must prevent us from sleeping in TASK_TRACED.
-		 * So after regaining the lock, we must check for SIGKILL.
 		 */
 		spin_unlock_irq(&current->sighand->siglock);
 		arch_ptrace_stop(exit_code, info);
 		spin_lock_irq(&current->sighand->siglock);
-		if (sigkill_pending(current))
-			return;
 	}
 
+	/*
+	 * schedule() will not sleep if there is a pending signal that
+	 * can awaken the task.
+	 */
 	set_special_state(TASK_TRACED);
 
 	/*
-- 
2.20.1


  reply	other threads:[~2021-09-24  0:09 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24  0:08 [PATCH 0/6] per signal_struct coredumps Eric W. Biederman
2021-09-24  0:09 ` Eric W. Biederman [this message]
2021-09-24 15:22   ` [PATCH 1/6] signal: Remove the bogus sigkill_pending in ptrace_stop Kees Cook
2021-09-24 15:48     ` Eric W. Biederman
2021-09-24 19:06       ` Kees Cook
2021-09-24  0:10 ` [PATCH 2/6] ptrace: Remove the unnecessary arguments from arch_ptrace_stop Eric W. Biederman
2021-09-24 15:26   ` Kees Cook
2021-09-24  0:10 ` [PATCH 3/6] exec: Check for a pending fatal signal instead of core_state Eric W. Biederman
2021-09-24 15:38   ` Kees Cook
2021-09-24  0:11 ` [PATCH 4/6] exit: Factor coredump_exit_mm out of exit_mm Eric W. Biederman
2021-09-24 18:28   ` Kees Cook
2021-09-24  0:11 ` [PATCH 5/6] coredump: Don't perform any cleanups before dumping core Eric W. Biederman
2021-09-24 18:51   ` Kees Cook
2021-09-24 21:28     ` Eric W. Biederman
2021-09-24 21:41       ` Kees Cook
2021-09-24  0:12 ` [PATCH 6/6] coredump: Limit coredumps to a single thread group Eric W. Biederman
2021-09-24 18:56   ` Kees Cook
2021-10-06 17:03     ` Eric W. Biederman
2021-11-19 16:03   ` Kyle Huey
2021-11-19 17:38     ` Eric W. Biederman
2021-09-24  5:59 ` [PATCH 0/6] per signal_struct coredumps Kees Cook
2021-09-24 14:00   ` Eric W. Biederman
2021-09-24 15:22 ` [PATCH 2/6] ptrace: Remove the unnecessary arguments from arch_ptrace_stop Eric W. Biederman
2021-09-24 15:22   ` 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=87pmsyx29t.fsf@disp2133 \
    --to=ebiederm@xmission.com \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.