Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44680C433FE for ; Wed, 24 Nov 2021 12:33:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344197AbhKXMgd (ORCPT ); Wed, 24 Nov 2021 07:36:33 -0500 Received: from mail.kernel.org ([198.145.29.99]:44396 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344033AbhKXMa0 (ORCPT ); Wed, 24 Nov 2021 07:30:26 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id A6888610A5; Wed, 24 Nov 2021 12:18:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1637756329; bh=BGLcJV2YQo4Vepl0wzyWojKgXLkuEtY2Q0csXY7nthY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CtItpQCxh8TCG7akN8qiaydpYqLWeIWqekeCIYQJmg7e/COqJCGLv9Sc3Qf02xuLU vPcBo9ioVAQBrNgDhLgH3VslBLmVdIVXRmhHG7AW96qI/2i9w8okyez9gRO7Kyd+tk CzqZpm0zCMmaypkO1qj8onwe23r8xUUPNR1T8inY= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Kees Cook , "Eric W. Biederman" Subject: [PATCH 4.14 049/251] signal: Remove the bogus sigkill_pending in ptrace_stop Date: Wed, 24 Nov 2021 12:54:51 +0100 Message-Id: <20211124115711.952461359@linuxfoundation.org> X-Mailer: git-send-email 2.34.0 In-Reply-To: <20211124115710.214900256@linuxfoundation.org> References: <20211124115710.214900256@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Eric W. Biederman 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: 3d749b9e676b ("ptrace: simplify ptrace_stop()->sigkill_pending() path") Fixes: 1a669c2f16d4 ("Add arch_ptrace_stop") Link: https://lkml.kernel.org/r/87pmsyx29t.fsf@disp2133 Reviewed-by: Kees Cook Signed-off-by: "Eric W. Biederman" Signed-off-by: Greg Kroah-Hartman --- kernel/signal.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1839,16 +1839,6 @@ static inline int may_ptrace_stop(void) } /* - * Return non-zero if there is a SIGKILL that should be waking us up. - * Called with the siglock held. - */ -static int 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. * * This should be the path for all ptrace stops. @@ -1873,17 +1863,16 @@ static void ptrace_stop(int exit_code, i * 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(¤t->sighand->siglock); arch_ptrace_stop(exit_code, info); spin_lock_irq(¤t->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); /*