linux-kernel - Re: [PATCH v7 5/6] x86/signal: Detect and prevent an alternate signal stack overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 Mar 2021 21:14:27 +0100
From:   Florian Weimer <fw@...eb.enyo.de>
To:     "Bae\, Chang Seok via Libc-alpha" <libc-alpha@...rceware.org>
Cc:     Borislav Petkov <bp@...e.de>,
        "Bae\, Chang Seok" <chang.seok.bae@...el.com>,
        "linux-arch\@vger.kernel.org" <linux-arch@...r.kernel.org>,
        "Brown\, Len" <len.brown@...el.com>,
        "Luck\, Tony" <tony.luck@...el.com>,
        "jannh\@google.com" <jannh@...gle.com>,
        "x86\@kernel.org" <x86@...nel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "Dave.Martin\@arm.com" <Dave.Martin@....com>,
        "Hansen\, Dave" <dave.hansen@...el.com>,
        "luto\@kernel.org" <luto@...nel.org>,
        "linux-api\@vger.kernel.org" <linux-api@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "mingo\@kernel.org" <mingo@...nel.org>,
        "Shankar\, Ravi V" <ravi.v.shankar@...el.com>
Subject: Re: [PATCH v7 5/6] x86/signal: Detect and prevent an alternate signal stack overflow

* Chang Seok via Libc-alpha Bae:

> On Mar 25, 2021, at 09:20, Borislav Petkov <bp@...e.de> wrote:
>> 
>> $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=3453 -o tst-minsigstksz-2
>> $ ./tst-minsigstksz-2
>> tst-minsigstksz-2: changed byte 50 bytes below configured stack
>> 
>> Whoops.
>> 
>> And the debug print said:
>> 
>> [ 5395.252884] signal: get_sigframe: sp: 0x7f54ec39e7b8, sas_ss_sp: 0x7f54ec39e6ce, sas_ss_size 0xd7d
>> 
>> which tells me that, AFAICT, your check whether we have enough alt stack
>> doesn't seem to work in this case.
>
> Yes, in this case.
>
> tst-minsigstksz-2.c has this code:
>
> static void
> handler (int signo)
> {
>   /* Clear a bit of on-stack memory.  */
>   volatile char buffer[256];
>   for (size_t i = 0; i < sizeof (buffer); ++i)
>     buffer[i] = 0;
>   handler_run = 1;
> }
> …
>
>   if (handler_run != 1)
>     errx (1, "handler did not run");
>
>   for (void *p = stack_buffer; p < stack_bottom; ++p)
>     if (*(unsigned char *) p != 0xCC)
>       errx (1, "changed byte %zd bytes below configured stack\n",
>             stack_bottom - p);
> …
>
> I think the message comes from the handler’s overwriting, not from the kernel.
>
> The patch's check is to detect and prevent the kernel-induced overflow --
> whether alt stack enough for signal delivery itself.  The stack is possibly
> not enough for the signal handler's use as the kernel does not know for it.

Ahh, right.  When I wrote the test, I didn't know which turn the
kernel would eventually take, so the test is quite arbitrary.

The glibc dynamic loader uses XSAVE/XSAVEC as well, so you can
probably double the practical stack requirement if lazy binding is in
use and can be triggered from the signal handler.  Estimating stack
sizes is hard.