Commit 09cdce82 authored Sep 01, 2021 by Eric W. Biederman Committed by Greg Kroah-Hartman Nov 26, 2021

signal: Remove the bogus sigkill_pending in ptrace_stop

commit 7d613f9f72ec8f90ddefcae038fdae5adb8404b3 upstream.

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: 3d749b9e ("ptrace: simplify ptrace_stop()->sigkill_pending() path")
Fixes: 1a669c2f ("Add arch_ptrace_stop")
Link: https://lkml.kernel.org/r/87pmsyx29t.fsf@disp2133


Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

parent 051ee719

kernel/signal.c

+4 −14

Original line number	Diff line number	Diff line
		@@ -2003,15 +2003,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.
		@@ -2038,17 +2029,16 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
		* 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);

		/*