linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: Borislav Petkov <bp@alien8.de>
Cc: "Bae, Chang Seok" <chang.seok.bae@intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	"Cooper, Andrew" <andrew.cooper3@citrix.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	"Gross, Jurgen" <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, X86 ML <x86@kernel.org>,
	"Brown, Len" <len.brown@intel.com>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"H. J. Lu" <hjl.tools@gmail.com>,
	Dave Martin <Dave.Martin@arm.com>, Jann Horn <jannh@google.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Carlos O'Donell <carlos@redhat.com>,
	"Luck, Tony" <tony.luck@intel.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	libc-alpha <libc-alpha@sourceware.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Linux API <linux-api@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 5/6] x86/signal: Detect and prevent an alternate signal stack overflow
Date: Mon, 03 May 2021 07:30:21 +0200	[thread overview]
Message-ID: <877dkg8jv6.fsf@oldenburg.str.redhat.com> (raw)
In-Reply-To: <20210414120608.GE10709@zn.tnic> (Borislav Petkov's message of "Wed, 14 Apr 2021 14:06:08 +0200")

* Borislav Petkov:

>> One possibility is that the sigaltstack size check prevents application
>> from running which work just fine today because all they do is install a
>> stack overflow handler, and stack overflow does not actually happen.
>
> So sigaltstack(2) says in the NOTES:
>
>        Functions  called  from  a signal handler executing on an alternate signal stack
>        will also use the alternate signal stack.  (This also applies  to  any  handlers
>        invoked for other signals while the process is executing on the alternate signal
>        stack.)  Unlike the standard stack, the system does not automatically extend the
>        alternate  signal  stack.   Exceeding the allocated size of the alternate signal
>        stack will lead to unpredictable results.
>
>> So if sigaltstack fails and the application checks the result of the
>> system call, it probably won't run at all. Shifting the diagnostic to
>> the pointer where the signal would have to be delivered is perhaps the
>> only thing that can be done.
>
> So using the example from the same manpage:
>
>        The most common usage of an alternate signal stack is to handle the SIGSEGV sig‐
>        nal that is generated if the space available for the normal process stack is ex‐
>        hausted: in this case, a signal handler for SIGSEGV cannot  be  invoked  on  the
>        process stack; if we wish to handle it, we must use an alternate signal stack.
>
> and considering these "unpredictable results" would it make sense or
> even be at all possible to return SIGFAIL from that SIGSEGV signal
> handler which should run on the sigaltstack but that sigaltstack
> overflows?
>
> I think we wanna be able to tell the process through that previously
> registered SIGSEGV handler which is supposed to run on the sigaltstack,
> that that stack got overflowed.

Just to be clear, I'm worried about the case where an application
installs a stack overflow handler, but stack overflow does not regularly
happen at run time.  GNU m4 is an example.  Today, for most m4 scripts,
it's totally fine to have an alternative signal stack which is too
small.  If the kernel returned an error for the sigaltstack call, m4
wouldn't start anymore, independently of the script.  Which is worse
than memory corruption with some scripts, I think.

> Or is this use case obsolete and this is not what people do at all?

It's widely used in currently-maintained software.  It's the only way to
recover from stack overflows without boundary checks on every function
call.

Does the alternative signal stack actually have to contain the siginfo_t
data?  I don't think it has to be contiguous.  Maybe the kernel could
allocate and map something behind the processes back if the sigaltstack
region is too small?

And for the stack overflow handler, the kernel could treat SIGSEGV with
a sigaltstack region that is too small like the SIG_DFL handler.  This
would make m4 work again.

Thanks,
Florian


  reply	other threads:[~2021-05-03  5:30 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-16  6:52 [PATCH v7 0/6] x86: Improve Minimum Alternate Stack Size Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 1/6] uapi: Define the aux vector AT_MINSIGSTKSZ Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 2/6] x86/signal: Introduce helpers to get the maximum signal frame size Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 3/6] x86/elf: Support a new ELF aux vector AT_MINSIGSTKSZ Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 4/6] selftest/sigaltstack: Use the AT_MINSIGSTKSZ aux vector if available Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 5/6] x86/signal: Detect and prevent an alternate signal stack overflow Chang S. Bae
2021-03-16 11:52   ` Borislav Petkov
2021-03-16 18:26     ` Bae, Chang Seok
2021-03-25 16:20       ` Borislav Petkov
2021-03-25 17:21         ` Bae, Chang Seok
2021-03-25 20:14           ` Florian Weimer
2021-03-25 18:13   ` Andy Lutomirski
2021-03-25 18:54     ` Borislav Petkov
2021-03-25 21:11       ` Bae, Chang Seok
2021-03-25 21:27         ` Borislav Petkov
2021-03-26  4:56       ` Andy Lutomirski
2021-03-26 10:30         ` Borislav Petkov
2021-04-12 22:30           ` Bae, Chang Seok
2021-04-14 10:12             ` Borislav Petkov
2021-04-14 11:30               ` Florian Weimer
2021-04-14 12:06                 ` Borislav Petkov
2021-05-03  5:30                   ` Florian Weimer [this message]
2021-05-03 11:17                     ` Borislav Petkov
2021-03-26  4:58     ` Andy Lutomirski
2021-03-16  6:52 ` [PATCH v7 6/6] selftest/x86/signal: Include test cases for validating sigaltstack Chang S. Bae
2021-03-17 10:06 ` [PATCH v7 0/6] x86: Improve Minimum Alternate Stack Size Ingo Molnar
2021-03-17 10:44   ` Ingo Molnar
2021-03-19 18:12     ` Len Brown
2021-03-20 17:32       ` Ingo Molnar

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=877dkg8jv6.fsf@oldenburg.str.redhat.com \
    --to=fweimer@redhat.com \
    --cc=Dave.Martin@arm.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=carlos@redhat.com \
    --cc=chang.seok.bae@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=hjl.tools@gmail.com \
    --cc=jannh@google.com \
    --cc=jgross@suse.com \
    --cc=len.brown@intel.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=ravi.v.shankar@intel.com \
    --cc=sstabellini@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).