All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Kyle Huey <me@kylehuey.com>
Cc: Kees Cook <keescook@chromium.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	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>,
	open list <linux-kernel@vger.kernel.org>,
	linux-hardening@vger.kernel.org,
	"Robert O'Callahan" <rocallahan@gmail.com>
Subject: Re: [REGRESSION] 5.16rc1: SA_IMMUTABLE breaks debuggers
Date: Wed, 17 Nov 2021 17:11:14 -0800	[thread overview]
Message-ID: <CAHk-=wiCRbSvUi_TnQkokLeM==_+Tow0GsQXnV3UYwhsxirPwg@mail.gmail.com> (raw)
In-Reply-To: <CAP045ApYXxhiAfmn=fQM7_hD58T-yx724ctWFHO4UAWCD+QapQ@mail.gmail.com>

On Wed, Nov 17, 2021 at 4:37 PM Kyle Huey <me@kylehuey.com> wrote:
>
> This fixes most of the issues with rr, but it still changes the ptrace
> behavior for the double-SIGSEGV case

Hmm. I think that's because of how "force_sigsgv()" works.

I absolutely detest that function.

So we have signal_setup_done() doing that

        if (failed)
                force_sigsegv(ksig->sig);

and then force_sigsegv() has that completely insane

        if (sig == SIGSEGV)
                force_fatal_sig(SIGSEGV);
        else
                force_sig(SIGSEGV);

behavior.

And I think I know the _reason_ for that complete insanity: when
SIGSEGV takes a SIGSEGV, and there is a handler, we need to stop
trying to send more SIGSEGV's.

But it does mean that with my change, that second SIGSEGV now ends up
being that SA_IMMUTABLE kind, so yeah, it broke the debugger test -
where catching the second SIGSEGV is actually somewhat sensible (ok,
not really, but at least understandable)

End result: I think we want not a boolean, but a three-way choice for
that force_sig_info_to_task() thing:

 - unconditionally fatal (for things that just want to force an exit
and used to do do_exit())

 - ignore valid and unblocked handler (for that SIGSEGV recursion
case, aka force "sigdfl")

 - catching signal ok

So my one-liner isn't sufficient. It wants some kind of nasty enum.

At least the enum can be entirely internal to kernel/signal.c, I
think. No need to expose this all to anything else.

            Linus

  reply	other threads:[~2021-11-18  1:11 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 [this message]
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
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='CAHk-=wiCRbSvUi_TnQkokLeM==_+Tow0GsQXnV3UYwhsxirPwg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=andrea.righi@canonical.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=me@kylehuey.com \
    --cc=rocallahan@gmail.com \
    --cc=shuah@kernel.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.