glibc tst-minsigstksz-5 failure

glibc tst-minsigstksz-5 failure

[John David Anglin]

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

The glibc tst-minsigstksz-5 test fails with a protection id trap.
tst-minsigstksz (pid 19958): Protection id trap (code 7) at 00000000f5b00497

The problem seems to be that the signal return trampoline is placed on the alternate stack
but the alternate is not executable.  It is allocated by malloc.

dave@mx3210:~/gnu/glibc/objdir$ gdb tst-minsigstksz-5                           GNU gdb (Debian 10.1-2) 10.1.90.20210103-git
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "hppa-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
     <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tst-minsigstksz-5...
(gdb) break handler
Breakpoint 1 at 0x1079c: file tst-minsigstksz-5.c, line 43.
(gdb) r
Starting program: /home/dave/gnu/glibc/objdir/tst-minsigstksz-5

Program received signal SIGUSR1, User defined signal 1.
getpid () at ../sysdeps/unix/syscall-template.S:91
91      ../sysdeps/unix/syscall-template.S: No such file or directory.
(gdb) c
Continuing.

Breakpoint 1, handler (signo=16) at tst-minsigstksz-5.c:43
warning: Source file is more recent than executable.
43
(gdb) p/x $sp
$1 = 0xf5b00900
(gdb) p/x $rp
$2 = 0xf5b00494
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
<signal handler called>
(gdb) disass $pc-16,$pc+16
Dump of assembler code from 0xf5b00484 to 0xf5b004a4:
    0xf5b00484:  movib,>= 6,r6,0xf5b01af0
    0xf5b00488:  movib,>= 6,r6,0xf5b01af4
    0xf5b0048c:  movib,>= 6,r6,0xf5b01af8
    0xf5b00490:  movib,>= 6,r6,0xf5b01afc
=> 0xf5b00494:  ldi 1,r25
    0xf5b00498:  ldi ad,r20
    0xf5b0049c:  be,l 100(sr2,r0),sr0,r31
    0xf5b004a0:  nop
End of assembler dump.

tst-minsigstksz (pid 8032): Protection id trap (code 7) at 00000000f5b00497

dave@mx3210:/proc/8032$ cat maps
00010000-00012000 r-xp 00000000 08:11 6958050                            /home/dave/gnu/glibc/objdir/tst-minsigstksz-5
00012000-00013000 rwxp 00002000 08:11 6958050                            /home/dave/gnu/glibc/objdir/tst-minsigstksz-5
00013000-00034000 rwxp 00000000 00:00 0                                  [heap]
f3b00000-f7b01000 rw-p 00000000 00:00 0
f7b01000-f7b23000 rwxp 00000000 00:00 0                                  [stack]
f90b8000-f9239000 r-xp 00000000 08:25 38369008                           /lib/hppa-linux-gnu/libc-2.32.so
f9239000-f923b000 r--p 00181000 08:25 38369008                           /lib/hppa-linux-gnu/libc-2.32.so
f923b000-f9240000 rwxp 00183000 08:25 38369008                           /lib/hppa-linux-gnu/libc-2.32.so
f9240000-f9242000 rwxp 00000000 00:00 0
f98b8000-f98e2000 r-xp 00000000 08:25 38368971                           /lib/hppa-linux-gnu/ld-2.32.so
f98e2000-f98e3000 r--p 0002a000 08:25 38368971                           /lib/hppa-linux-gnu/ld-2.32.so
f98e3000-f98e7000 rwxp 0002b000 08:25 38368971                           /lib/hppa-linux-gnu/ld-2.32.so
f9afb000-f9b00000 rw-p 00000000 00:00 0

Stacks are normally executable on hppa so the trampoline works.  But user code wouldn't normally make an alternate
stack executable.

Dave
-- 
John David Anglin  dave.anglin@bell.net

[-- Attachment #2: tst-minsigstksz-5.c --]
[-- Type: text/plain, Size: 2572 bytes --]

/* Test of signal delivery on an alternate stack with MINSIGSTKSZ size.
   Copyright (C) 2020 Free Software Foundation, Inc.
   This file is part of the GNU C Library.

   The GNU C Library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
   License as published by the Free Software Foundation; either
   version 2.1 of the License, or (at your option) any later version.

   The GNU C Library is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
   Lesser General Public License for more details.

   You should have received a copy of the GNU Lesser General Public
   License along with the GNU C Library; if not, see
   <https://www.gnu.org/licenses/>.  */

#include <signal.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <stdio.h>
#include <errno.h>

#define FAIL_RET(s,...) \
  do { \
    printf (s, errno); \
    return 1; \
  } while (0)

static volatile sig_atomic_t handler_run;

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;
}

int
do_test (void)
{
  size_t stack_buffer_size = 64 * 1024 * 1024;
  void *stack_buffer = malloc (stack_buffer_size);
  void *stack_end = stack_buffer + stack_buffer_size;
  memset (stack_buffer, 0xCC, stack_buffer_size);

  void *stack_bottom = stack_buffer + (stack_buffer_size + MINSIGSTKSZ) / 2;
  void *stack_top = stack_bottom + MINSIGSTKSZ;
  stack_t stack =
    {
      .ss_sp = stack_bottom,
      .ss_size = MINSIGSTKSZ,
    };
  if (sigaltstack (&stack, NULL) < 0)
    FAIL_RET ("sigaltstack: %m\n");

  struct sigaction act =
    {
      .sa_handler = handler,
      .sa_flags = SA_ONSTACK,
    };
  if (sigaction (SIGUSR1, &act, NULL) < 0)
    FAIL_RET ("sigaction: %m\n");

  if (kill (getpid (), SIGUSR1) < 0)
    FAIL_RET ("kill: %m\n");

  if (handler_run != 1)
    FAIL_RET ("handler did not run\n");

  for (void *p = stack_buffer; p < stack_bottom; ++p)
    if (*(unsigned char *) p != 0xCC)
      FAIL_RET ("changed byte %ld bytes below configured stack\n",
		(long) (stack_bottom - p));
  for (void *p = stack_top; p < stack_end; ++p)
    if (*(unsigned char *) p != 0xCC)
      FAIL_RET ("changed byte %ld bytes above configured stack\n",
		(long) (p - stack_top));

  free (stack_buffer);

  return 0;
}

int
main (void)
{
  return do_test();
}

Re: glibc tst-minsigstksz-5 failure

[Helge Deller]

On 12/7/21 22:47, John David Anglin wrote:
> The glibc tst-minsigstksz-5 test fails with a protection id trap.
> tst-minsigstksz (pid 19958): Protection id trap (code 7) at 00000000f5b00497
>
> The problem seems to be that the signal return trampoline is placed
> on the alternate stack but the alternate is not executable.  It is
> allocated by malloc.> ...
> Stacks are normally executable on hppa so the trampoline works.  But
> user code wouldn't normally make an alternate stack executable.
True, I think most people just forget that such architectures exist.

Anyway, the glibc testcase is interesting.
The pretty similar sigaltstack testcase from LTP does work:
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/sigaltstack/sigaltstack01.c

Both use malloc() to allocate the memory, the only difference is the size allocated.
If you change the glibc testcase to:
-- size_t stack_buffer_size = 64 * 1024 * 1024;
++ size_t stack_buffer_size = SIGSTKSZ;
it allocates only 8kB instead of 64MB.

It seems glibc uses brk() in both cases, but when allocating 64MB it additionally adds a mmap() with READ/WRITE permissions only:
brk(0x606000)                           = 0x606000
mmap2(NULL, 67112960, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf303f000

This will then break - as you mentioned - the signal handling on parisc.

I see those options to fix it:
a) Fix the application to map the memory +x
b) Add some code to glibc to map the memory +x when sigaltstack is called.
c) Add some (parisc-only) code to kernel to set the permission.

I've looked into c), but that's not my favorite choice.

Helge

Re: glibc tst-minsigstksz-5 failure

[Helge Deller]

On 12/8/21 13:51, Helge Deller wrote:
> On 12/7/21 22:47, John David Anglin wrote:
>> The glibc tst-minsigstksz-5 test fails with a protection id trap.
>> tst-minsigstksz (pid 19958): Protection id trap (code 7) at 00000000f5b00497
>>
>> The problem seems to be that the signal return trampoline is placed
>> on the alternate stack but the alternate is not executable.  It is
>> allocated by malloc.> ...
>> Stacks are normally executable on hppa so the trampoline works.  But
>> user code wouldn't normally make an alternate stack executable.
> True, I think most people just forget that such architectures exist.
>
> Anyway, the glibc testcase is interesting.
> The pretty similar sigaltstack testcase from LTP does work:
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/sigaltstack/sigaltstack01.c
>
> Both use malloc() to allocate the memory, the only difference is the size allocated.
> If you change the glibc testcase to:
> -- size_t stack_buffer_size = 64 * 1024 * 1024;
> ++ size_t stack_buffer_size = SIGSTKSZ;
> it allocates only 8kB instead of 64MB.
>
> It seems glibc uses brk() in both cases, but when allocating 64MB it additionally adds a mmap() with READ/WRITE permissions only:
> brk(0x606000)                           = 0x606000
> mmap2(NULL, 67112960, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf303f000
>
> This will then break - as you mentioned - the signal handling on parisc.
>
> I see those options to fix it:
> a) Fix the application to map the memory +x
> b) Add some code to glibc to map the memory +x when sigaltstack is called.
> c) Add some (parisc-only) code to kernel to set the permission.

Option d):
Enhance the kernel to create a per-process new page and map it +rx into the userspace
at process start time. Kernel sets up the page to contain the signal trampoline code.

Option e):
Finally implement vDSO, which then includes option d)

With options d) and e) we could get completely rid of executable stacks.

Helge

Re: glibc tst-minsigstksz-5 failure

[John David Anglin]

On 2021-12-08 8:05 a.m., Helge Deller wrote:
> On 12/8/21 13:51, Helge Deller wrote:
>> On 12/7/21 22:47, John David Anglin wrote:
>>> The glibc tst-minsigstksz-5 test fails with a protection id trap.
>>> tst-minsigstksz (pid 19958): Protection id trap (code 7) at 00000000f5b00497
>>>
>>> The problem seems to be that the signal return trampoline is placed
>>> on the alternate stack but the alternate is not executable.  It is
>>> allocated by malloc.> ...
>>> Stacks are normally executable on hppa so the trampoline works.  But
>>> user code wouldn't normally make an alternate stack executable.
>> True, I think most people just forget that such architectures exist.
>>
>> Anyway, the glibc testcase is interesting.
>> The pretty similar sigaltstack testcase from LTP does work:
>> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/sigaltstack/sigaltstack01.c
>>
>> Both use malloc() to allocate the memory, the only difference is the size allocated.
>> If you change the glibc testcase to:
>> -- size_t stack_buffer_size = 64 * 1024 * 1024;
>> ++ size_t stack_buffer_size = SIGSTKSZ;
>> it allocates only 8kB instead of 64MB.
>> It seems glibc uses brk() in both cases, but when allocating 64MB it additionally adds a mmap() with READ/WRITE permissions only:
>> brk(0x606000)                           = 0x606000
>> mmap2(NULL, 67112960, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf303f000
>>
>> This will then break - as you mentioned - the signal handling on parisc.
>>
>> I see those options to fix it:/usr/include/hppa-linux-gnu/asm/signal.h:#define MINSIGSTKSZ    2048
I wonder about defining MINSIGSTKSZ to 2048 as it is smaller than a page.

mprotect requires a page aligned address.  Alternate stack isn't going to be page aligned if it is allocated by
malloc.  Malloc alignment isn't sufficient for nominal 64-byte stack alignment specified in runtime.
>> /usr/include/hppa-linux-gnu/asm/signal.h:#define SIGSTKSZ       8192
>>
>> a) Fix the application to map the memory +x
Doesn't fix problem..
>> b) Add some code to glibc to map the memory +x when sigaltstack is called.
See mprotect comment.
>> c) Add some (parisc-only) code to kernel to set the permission.
Again, I think region needs to be page aligned.
> Option d):
> Enhance the kernel to create a per-process new page and map it +rx into the userspace
> at process start time. Kernel sets up the page to contain the signal trampoline code.
>
> Option e):
> Finally implement vDSO, which then includes option d)
>
> With options d) and e) we could get completely rid of executable stacks.
I like the later two options.

Dave

-- 
John David Anglin  dave.anglin@bell.net

Re: glibc tst-minsigstksz-5 failure

[Sven Schnelle]

John David Anglin <dave.anglin@bell.net> writes:

> On 2021-12-08 8:05 a.m., Helge Deller wrote:
>> On 12/8/21 13:51, Helge Deller wrote:
>>> On 12/7/21 22:47, John David Anglin wrote:
>>>> The glibc tst-minsigstksz-5 test fails with a protection id trap.
>>>> tst-minsigstksz (pid 19958): Protection id trap (code 7) at 00000000f5b00497
>>>>
>>>> The problem seems to be that the signal return trampoline is placed
>>>> on the alternate stack but the alternate is not executable.  It is
>>>> allocated by malloc.> ...
>>>> Stacks are normally executable on hppa so the trampoline works.  But
>>>> user code wouldn't normally make an alternate stack executable.
>>> True, I think most people just forget that such architectures exist.
>>>
>>> Anyway, the glibc testcase is interesting.
>>> The pretty similar sigaltstack testcase from LTP does work:
>>> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/sigaltstack/sigaltstack01.c
>>>
>>> Both use malloc() to allocate the memory, the only difference is the size allocated.
>>> If you change the glibc testcase to:
>>> -- size_t stack_buffer_size = 64 * 1024 * 1024;
>>> ++ size_t stack_buffer_size = SIGSTKSZ;
>>> it allocates only 8kB instead of 64MB.
>>> It seems glibc uses brk() in both cases, but when allocating 64MB it additionally adds a mmap() with READ/WRITE permissions only:
>>> brk(0x606000)                           = 0x606000
>>> mmap2(NULL, 67112960, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xf303f000
>>>
>>> This will then break - as you mentioned - the signal handling on parisc.
>>>
>>> I see those options to fix it:/usr/include/hppa-linux-gnu/asm/signal.h:#define MINSIGSTKSZ    2048
> I wonder about defining MINSIGSTKSZ to 2048 as it is smaller than a page.
>
> mprotect requires a page aligned address.  Alternate stack isn't going to be page aligned if it is allocated by
> malloc.  Malloc alignment isn't sufficient for nominal 64-byte stack alignment specified in runtime.
>>> /usr/include/hppa-linux-gnu/asm/signal.h:#define SIGSTKSZ       8192
>>>
>>> a) Fix the application to map the memory +x
> Doesn't fix problem..
>>> b) Add some code to glibc to map the memory +x when sigaltstack is called.
> See mprotect comment.
>>> c) Add some (parisc-only) code to kernel to set the permission.
> Again, I think region needs to be page aligned.
>> Option d):
>> Enhance the kernel to create a per-process new page and map it +rx into the userspace
>> at process start time. Kernel sets up the page to contain the signal trampoline code.
>>
>> Option e):
>> Finally implement vDSO, which then includes option d)
>>
>> With options d) and e) we could get completely rid of executable stacks.
> I like the later two options.

Note that programs are allowed to unmap the vdso. Valgrind is doing that (guess it
doesn't support hppa anyways). OTOH valgrind uses SA_RESTORER, so it
would still work. While i don't think the possibility of unmapping the
vdso would be a real issue, i thought i mention it.

Regards
Sven