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

["Bae, Chang Seok" <chang.seok.bae@xxxxxxxxx>]

On Mar 16, 2021, at 04:52, Borislav Petkov <bp@xxxxxxx> wrote:
> On Mon, Mar 15, 2021 at 11:52:14PM -0700, Chang S. Bae wrote:
>> @@ -272,7 +275,8 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
>> 	 * If we are on the alternate signal stack and would overflow it, don't.
>> 	 * Return an always-bogus address instead so we will die with SIGSEGV.
>> 	 */
>> -	if (onsigstack && !likely(on_sig_stack(sp)))
>> +	if (onsigstack && unlikely(sp <= current->sas_ss_sp ||
>> +				   sp - current->sas_ss_sp > current->sas_ss_size))
>> 		return (void __user *)-1L;
> 
> So clearly I'm missing something because trying to trigger the test case
> in the bugzilla:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=153531
> 
> on current tip/master doesn't work. Runs with MY_MINSIGSTKSZ under 2048
> fail with:
> 
> tst-minsigstksz-2: sigaltstack: Cannot allocate memory
> 
> and above 2048 don't overwrite bytes below the stack.
> 
> So something else is missing. How did you test this patch?

I suspect the AVX-512 states not enabled there.

When I ran it under a machine without AVX-512 like this, it didn’t show the
overwrite message:

    $ cat /proc/cpuinfo | grep -m1 "model name”
    model name      : Intel(R) Core(TM) i9-10900K CPU @ 3.70GHz

    $ sudo dmesg | grep "Enabled xstate”
    [    0.000000] x86/fpu: Enabled xstate features 0x1f, context size is 960
    bytes, using ‘compacted’ format.

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=2047
    $ ./a.out
    a.out: sigaltstack: Cannot allocate memory

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=2048
    $ ./a.out

When do it again with AVX-512, it did show the message:

    $ cat /proc/cpuinfo  | grep -m1 "model name”
    model name      : Intel(R) Core(TM) i9-7940X CPU @ 3.10GHz

    $ sudo dmesg | grep "Enabled xstate”
    [    0.000000] x86/fpu: Enabled xstate features 0xff, context size is 2560
    bytes, using 'compacted' format.

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=2048
    $ ./a.out
    a.out: changed byte 1412 bytes below configured stack

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=3490
    $ ./a.out
    a.out: changed byte 21 bytes below configured stack

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=3491
    $ ./a.out


Also, on the second machine, without this patch:

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=3191
    $ ./a.out
    a.out: changed byte 319 bytes below configured stack

But with this patch, it gave segfault with a too-small size:

    $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=3191
    $ ./a.out
    Segmentation fault (core dumped)

Thanks,
Chang