All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: linux-kernel@vger.kernel.org, Kyle Huey <me@kylehuey.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrea Righi <andrea.righi@canonical.com>,
	Shuah Khan <shuah@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Will Drewry <wad@chromium.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	linux-hardening@vger.kernel.org,
	Robert O'Callahan <rocallahan@gmail.com>,
	Oliver Sang <oliver.sang@intel.com>,
	lkp@lists.01.org, lkp@intel.com
Subject: Re: [PATCH 1/2] signal: Don't always set SA_IMMUTABLE for forced signals
Date: Thu, 18 Nov 2021 15:54:02 -0800	[thread overview]
Message-ID: <202111181553.A4FDEB1@keescook> (raw)
In-Reply-To: <877dd5qfw5.fsf_-_@email.froward.int.ebiederm.org>

On Thu, Nov 18, 2021 at 04:04:58PM -0600, Eric W. Biederman wrote:
> 
> Recently to prevent issues with SECCOMP_RET_KILL and similar signals
> being changed before they are delivered SA_IMMUTABLE was added.
> 
> Unfortunately this broke debuggers[1][2] which reasonably expect to be
> able to trap synchronous SIGTRAP and SIGSEGV even when the target
> process is not configured to handle those signals.
> 
> Update force_sig_to_task to support both the case when we can
> allow the debugger to intercept and possibly ignore the
> signal and the case when it is not safe to let userspace
> known about the signal until the process has exited.
> 
> Reported-by: Kyle Huey <me@kylehuey.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Cc: stable@vger.kernel.org
> [1] https://lkml.kernel.org/r/CAP045AoMY4xf8aC_4QU_-j7obuEPYgTcnQQP3Yxk=2X90jtpjw@mail.gmail.com
> [2] https://lkml.kernel.org/r/20211117150258.GB5403@xsang-OptiPlex-902
> Fixes: 00b06da29cf9 ("signal: Add SA_IMMUTABLE to ensure forced siganls do not get changed")
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  kernel/signal.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 7c4b7ae714d4..02058c983bd6 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1298,6 +1298,12 @@ int do_send_sig_info(int sig, struct kernel_siginfo *info, struct task_struct *p
>  	return ret;
>  }
>  
> +enum sig_handler {
> +	HANDLER_CURRENT, /* If reachable use the current handler */
> +	HANDLER_SIG_DFL, /* Always use SIG_DFL handler semantics */
> +	HANDLER_EXIT,	 /* Only visible as the proces exit code */

Oh, I just noticed this typo "proces" -> "process"

-Kees

-- 
Kees Cook

WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: lkp@lists.01.org
Subject: Re: [PATCH 1/2] signal: Don't always set SA_IMMUTABLE for forced signals
Date: Thu, 18 Nov 2021 15:54:02 -0800	[thread overview]
Message-ID: <202111181553.A4FDEB1@keescook> (raw)
In-Reply-To: <877dd5qfw5.fsf_-_@email.froward.int.ebiederm.org>

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

On Thu, Nov 18, 2021 at 04:04:58PM -0600, Eric W. Biederman wrote:
> 
> Recently to prevent issues with SECCOMP_RET_KILL and similar signals
> being changed before they are delivered SA_IMMUTABLE was added.
> 
> Unfortunately this broke debuggers[1][2] which reasonably expect to be
> able to trap synchronous SIGTRAP and SIGSEGV even when the target
> process is not configured to handle those signals.
> 
> Update force_sig_to_task to support both the case when we can
> allow the debugger to intercept and possibly ignore the
> signal and the case when it is not safe to let userspace
> known about the signal until the process has exited.
> 
> Reported-by: Kyle Huey <me@kylehuey.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Cc: stable(a)vger.kernel.org
> [1] https://lkml.kernel.org/r/CAP045AoMY4xf8aC_4QU_-j7obuEPYgTcnQQP3Yxk=2X90jtpjw(a)mail.gmail.com
> [2] https://lkml.kernel.org/r/20211117150258.GB5403(a)xsang-OptiPlex-902
> Fixes: 00b06da29cf9 ("signal: Add SA_IMMUTABLE to ensure forced siganls do not get changed")
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  kernel/signal.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 7c4b7ae714d4..02058c983bd6 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1298,6 +1298,12 @@ int do_send_sig_info(int sig, struct kernel_siginfo *info, struct task_struct *p
>  	return ret;
>  }
>  
> +enum sig_handler {
> +	HANDLER_CURRENT, /* If reachable use the current handler */
> +	HANDLER_SIG_DFL, /* Always use SIG_DFL handler semantics */
> +	HANDLER_EXIT,	 /* Only visible as the proces exit code */

Oh, I just noticed this typo "proces" -> "process"

-Kees

-- 
Kees Cook

  parent reply	other threads:[~2021-11-18 23:54 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-17 18:47 [REGRESSION] 5.16rc1: SA_IMMUTABLE breaks debuggers Kyle Huey
2021-11-17 18:51 ` Kees Cook
2021-11-17 19:05   ` Kyle Huey
2021-11-17 19:09     ` Kyle Huey
2021-11-17 21:04       ` Eric W. Biederman
2021-11-17 21:54         ` Kees Cook
2021-11-17 23:24           ` Linus Torvalds
2021-11-18  0:05             ` Kees Cook
2021-11-18  0:15               ` Linus Torvalds
2021-11-18  0:37             ` Kyle Huey
2021-11-18  1:11               ` Linus Torvalds
2021-11-18  1:20                 ` Kyle Huey
2021-11-18  1:32                   ` Kees Cook
2021-11-18 16:10                     ` Eric W. Biederman
2021-11-19 16:07                       ` Kyle Huey
2021-11-19 16:35                         ` Kees Cook
2021-11-19 16:58                           ` Kyle Huey
2021-11-18 21:58                     ` [PATCH 0/2] SA_IMMUTABLE fixes Eric W. Biederman
2021-11-18 21:58                       ` Eric W. Biederman
2021-11-18 22:04                       ` [PATCH 1/2] signal: Don't always set SA_IMMUTABLE for forced signals Eric W. Biederman
2021-11-18 22:04                         ` Eric W. Biederman
2021-11-18 23:52                         ` Kees Cook
2021-11-18 23:52                           ` Kees Cook
2021-11-18 23:54                         ` Kees Cook [this message]
2021-11-18 23:54                           ` Kees Cook
2021-11-19 15:08                           ` Eric W. Biederman
2021-11-19 15:08                             ` Eric W. Biederman
2021-11-19  1:13                         ` Kyle Huey
2021-11-19  1:13                           ` Kyle Huey
2021-11-19 15:03                           ` Eric W. Biederman
2021-11-19 15:03                             ` Eric W. Biederman
2021-11-18 22:05                       ` [PATCH 2/2] signal: Replace force_fatal_sig with force_exit_sig when in doubt Eric W. Biederman
2021-11-18 22:05                         ` Eric W. Biederman
2021-11-18 23:53                         ` Kees Cook
2021-11-18 23:53                           ` Kees Cook
2021-11-19  1:12                       ` [PATCH 0/2] SA_IMMUTABLE fixes Kyle Huey
2021-11-19  1:12                         ` Kyle Huey
2021-11-19 15:41                         ` [GIT PULL] SA_IMMUTABLE fixes for v5.16-rc2 Eric W. Biederman
2021-11-19 15:41                           ` Eric W. Biederman
2021-11-19 19:46                           ` pr-tracker-bot
2021-11-19 19:46                             ` pr-tracker-bot
2021-11-17 22:29         ` [REGRESSION] 5.16rc1: SA_IMMUTABLE breaks debuggers Kyle Huey
2021-11-18  5:43 ` Thorsten Leemhuis
2021-11-20  6:13   ` Thorsten Leemhuis

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=202111181553.A4FDEB1@keescook \
    --to=keescook@chromium.org \
    --cc=andrea.righi@canonical.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=lkp@lists.01.org \
    --cc=luto@amacapital.net \
    --cc=me@kylehuey.com \
    --cc=oliver.sang@intel.com \
    --cc=rocallahan@gmail.com \
    --cc=shuah@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wad@chromium.org \
    /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.