This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Zack Weinberg <zackw at panix dot com>, libc-alpha at sourceware dot org, Siddhesh Poyarekar <siddhesh at gotplt dot org>
- Date: Tue, 15 Jan 2019 16:15:47 -0500
- Subject: Re: [PATCH] Tests for minimal signal handler functionality in MINSIGSTKSZ space.
- References: <20190115200526.4677-1-zackw@panix.com>
On 1/15/19 3:05 PM, Zack Weinberg wrote:
> We don’t have a full conclusion about the way forward for alternate
> signal stacks yet, but the notion that the very short list of things
> that ISO C says you can do in an async signal handler should all work
> in MINSIGSTKSZ space seemed to be well-received. As a first step,
> therefore, I propose to add tests to make sure those things do work.
>
> To facilitate this, there is a new set of test support routines for
> setting up alternate signal stacks; see support/xsignal.h for the API.
>
> All of these tests pass on x86-64. build-many-glibcs won’t tell us
> anything additional; the tests need to be run.
Thank you for doing this.
OK for master with comment addition in implementation.
You need Siddhesh to approve the extra tests.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> * support/xsignal.h (xalloc_sigstack, xfree_sigstack)
> (xget_sigstack_location): New test support functions.
> * support/xsigstack.c: New file, implementing them.
> * support/tst-xsigstack.c: New test for them.
> * support/Makefile: Update.
>
> * signal/tst-minsigstksz-1.c
> * signal/tst-minsigstksz-2.c
> * signal/tst-minsigstksz-3.c
> * signal/tst-minsigstksz-3a.c
> * signal/tst-minsigstksz-4.c: New tests.
> * signal/Makefile: Run them.
> ---
> signal/Makefile | 2 +
> signal/tst-minsigstksz-1.c | 131 ++++++++++++++++++++++++++++++++++++
> signal/tst-minsigstksz-2.c | 66 ++++++++++++++++++
> signal/tst-minsigstksz-3.c | 64 ++++++++++++++++++
> signal/tst-minsigstksz-3a.c | 69 +++++++++++++++++++
> signal/tst-minsigstksz-4.c | 65 ++++++++++++++++++
> support/Makefile | 2 +
> support/tst-xsigstack.c | 64 ++++++++++++++++++
> support/xsignal.h | 18 +++++
> support/xsigstack.c | 95 ++++++++++++++++++++++++++
> 10 files changed, 576 insertions(+)
> create mode 100644 signal/tst-minsigstksz-1.c
> create mode 100644 signal/tst-minsigstksz-2.c
> create mode 100644 signal/tst-minsigstksz-3.c
> create mode 100644 signal/tst-minsigstksz-3a.c
> create mode 100644 signal/tst-minsigstksz-4.c
> create mode 100644 support/tst-xsigstack.c
> create mode 100644 support/xsigstack.c
>
> diff --git a/signal/Makefile b/signal/Makefile
> index aa63434f47..d5a10d590d 100644
> --- a/signal/Makefile
> +++ b/signal/Makefile
> @@ -47,6 +47,8 @@ routines := signal raise killpg \
>
> tests := tst-signal tst-sigset tst-sigsimple tst-raise tst-sigset2 \
> tst-sigwait-eintr tst-sigaction \
> + tst-minsigstksz-1 tst-minsigstksz-2 tst-minsigstksz-3 \
> + tst-minsigstksz-3a tst-minsigstksz-4 \
OK.
>
> include ../Rules
>
> diff --git a/signal/tst-minsigstksz-1.c b/signal/tst-minsigstksz-1.c
> new file mode 100644
> index 0000000000..00344d5fbf
> --- /dev/null
> +++ b/signal/tst-minsigstksz-1.c
> @@ -0,0 +1,131 @@
> +/* Tests of signal delivery on an alternate stack (nonlethal).
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> + guaranteed to be well-defined inside an asynchronous signal handler:
> + * any operation on a lock-free atomic object
> + * assigning a value to an object declared as volatile sig_atomic_t
> + * calling abort, _Exit, quick_exit, or signal
> + * signal may only be called with its first argument equal to the
> + number of the signal that caused the handler to be called
> +
> + We use this list as a guideline for the set of operations that ought
> + also to be safe in a _synchronous_ signal delivered on an alternate
> + signal stack with only MINSIGSTKSZ bytes of space.
> +
> + This test program tests all of the above operations that do not,
> + one way or another, cause the program to be terminated. */
OK. Agreed.
> +
> +/* We do not try to test atomic operations exhaustively, only a simple
> + atomic counter increment. This is only safe if atomic_[u]int is
> + unconditionally lock-free. */
> +#ifdef __STDC_NO_ATOMICS__
> +# define TEST_ATOMIC_OPS 0
> +#else
> +# include <stdatomic.h>
> +# if ATOMIC_INT_LOCK_FREE != 2
> +# define TEST_ATOMIC_OPS 0
> +# else
> +# define TEST_ATOMIC_OPS 1
> +# endif
> +#endif
> +
> +static volatile sig_atomic_t signal_flag = 0;
> +static volatile sig_atomic_t signal_err = 0;
> +static void
> +handler_set_flag (int unused)
> +{
> + signal_flag = 1;
> +}
OK.
> +
> +static void
> +handler_set_flag_once (int sig)
> +{
> + signal_flag = 1;
> + if (signal (sig, SIG_IGN) == SIG_ERR)
> + /* It is not safe to call FAIL_EXIT1 here. Set another flag instead. */
> + signal_err = 1;
> +}
OK.
> +
> +#if TEST_ATOMIC_OPS
> +static atomic_uint signal_count = 0;
> +static void
> +handler_count_up_1 (int unused)
> +{
> + atomic_fetch_add (&signal_count, 1);
OK.
> +}
> +#endif
> +
> +int
> +do_test (void)
> +{
> + void *sstk = xalloc_sigstack (0);
OK.
> + struct sigaction sa;
> +
> + /* Test 1: setting a volatile sig_atomic_t flag. */
> + sa.sa_handler = handler_set_flag;
> + sa.sa_flags = SA_RESTART | SA_ONSTACK;
> + sigfillset (&sa.sa_mask);
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag): %m\n");
> +
> + TEST_VERIFY_EXIT (signal_flag == 0);
> + raise (SIGUSR1);
> + TEST_VERIFY_EXIT (signal_flag == 1);
> + signal_flag = 0;
> + raise (SIGUSR1);
> + TEST_VERIFY_EXIT (signal_flag == 1);
> + signal_flag = 0;
OK. Minimal test.
> +
> + /* Test 1: setting a volatile sig_atomic_t flag and then ignoring
> + further delivery of the signal. */
> + sa.sa_handler = handler_set_flag_once;
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_EXIT1 ("sigaction (SIGUSR1, handler_set_flag_once): %m\n");
> +
> + raise (SIGUSR1);
> + TEST_VERIFY_EXIT (signal_flag == 1);
> + /* Note: if signal_err is 1, a system call failed, but we can't
> + report the error code because errno is indeterminate. */
> + TEST_VERIFY_EXIT (signal_err == 0);
> +
> + signal_flag = 0;
> + raise (SIGUSR1);
> + TEST_VERIFY_EXIT (signal_flag == 0);
> + TEST_VERIFY_EXIT (signal_err == 0);
> +
> +#if TEST_ATOMIC_OPS
> + sa.sa_handler = handler_count_up_1;
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_EXIT1 ("sigaction (SIGUSR1, handler_count_up_1): %m\n");
> +
> + raise (SIGUSR1);
> + TEST_VERIFY_EXIT (atomic_load (&signal_count) == 1);
> + raise (SIGUSR1);
> + TEST_VERIFY_EXIT (atomic_load (&signal_count) == 2);
OK.
> +#endif
> +
> + xfree_sigstack (sstk);
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/signal/tst-minsigstksz-2.c b/signal/tst-minsigstksz-2.c
> new file mode 100644
> index 0000000000..3368dde6b8
> --- /dev/null
> +++ b/signal/tst-minsigstksz-2.c
> @@ -0,0 +1,66 @@
> +/* Tests of signal delivery on an alternate stack (abort).
OK.
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdlib.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> + guaranteed to be well-defined inside an asynchronous signal handler:
> + * any operation on a lock-free atomic object
> + * assigning a value to an object declared as volatile sig_atomic_t
> + * calling abort, _Exit, quick_exit, or signal
> + * signal may only be called with its first argument equal to the
> + number of the signal that caused the handler to be called
> +
> + We use this list as a guideline for the set of operations that ought
> + also to be safe in a _synchronous_ signal delivered on an alternate
> + signal stack with only MINSIGSTKSZ bytes of space.
> +
> + This test program tests calls to abort. Note that it does _not_
> + install a handler for SIGABRT, because that signal would also be
> + delivered on the alternate stack and MINSIGSTKSZ does not provide
> + enough space for delivery of nested signals. */
OK. I'm not sure if it's explicitly spelled out that the default disposition
for SIGABRT is to core, but I doubt that will change today. This test should
work as intended and if the default is not to core or terminate then you'll
notice the test failing.
> +
> +static void
> +handler (int unused)
> +{
> + abort ();
> +}
> +
> +int
> +do_test (void)
> +{
> + void *sstk = xalloc_sigstack (0);
> + struct sigaction sa;
> +
> + sa.sa_handler = handler;
> + sa.sa_flags = SA_RESTART | SA_ONSTACK;
> + sigfillset (&sa.sa_mask);
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> + raise (SIGUSR1);
OK.
> +
> + xfree_sigstack (sstk);
> + FAIL_RET ("test process was not terminated by abort in signal handler");
> +}
> +
> +#define EXPECTED_SIGNAL SIGABRT
> +#include <support/test-driver.c>
> diff --git a/signal/tst-minsigstksz-3.c b/signal/tst-minsigstksz-3.c
> new file mode 100644
> index 0000000000..a8d9a6369c
> --- /dev/null
> +++ b/signal/tst-minsigstksz-3.c
> @@ -0,0 +1,64 @@
> +/* Tests of signal delivery on an alternate stack (_Exit).
OK.
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdlib.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> + guaranteed to be well-defined inside an asynchronous signal handler:
> + * any operation on a lock-free atomic object
> + * assigning a value to an object declared as volatile sig_atomic_t
> + * calling abort, _Exit, quick_exit, or signal
> + * signal may only be called with its first argument equal to the
> + number of the signal that caused the handler to be called
> +
> + We use this list as a guideline for the set of operations that ought
> + also to be safe in a _synchronous_ signal delivered on an alternate
> + signal stack with only MINSIGSTKSZ bytes of space.
> +
> + This test program tests calls to _Exit. */
OK.
> +
> +#define EXPECTED_STATUS 3
> +
> +static void
> +handler (int unused)
> +{
> + _Exit (EXPECTED_STATUS);
OK. Good use an odd exist status of 3.
> +}
> +
> +int
> +do_test (void)
> +{
> + void *sstk = xalloc_sigstack (0);
> + struct sigaction sa;
> +
> + sa.sa_handler = handler;
> + sa.sa_flags = SA_RESTART | SA_ONSTACK;
> + sigfillset (&sa.sa_mask);
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> + raise (SIGUSR1);
> +
> + xfree_sigstack (sstk);
> + FAIL_RET ("test process was not terminated by _Exit in signal handler");
> +}
> +
> +#include <support/test-driver.c>
OK.
> diff --git a/signal/tst-minsigstksz-3a.c b/signal/tst-minsigstksz-3a.c
> new file mode 100644
> index 0000000000..b58b8d01ba
> --- /dev/null
> +++ b/signal/tst-minsigstksz-3a.c
> @@ -0,0 +1,69 @@
> +/* Tests of signal delivery on an alternate stack (_exit).
OK. Nice to use 3 and 3a for both _exit/_Exit since they are going to be
the same, but it's OK to test them distinctly.
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <unistd.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> + guaranteed to be well-defined inside an asynchronous signal handler:
> + * any operation on a lock-free atomic object
> + * assigning a value to an object declared as volatile sig_atomic_t
> + * calling abort, _Exit, quick_exit, or signal
> + * signal may only be called with its first argument equal to the
> + number of the signal that caused the handler to be called
> +
> + We use this list as a guideline for the set of operations that ought
> + also to be safe in a _synchronous_ signal delivered on an alternate
> + signal stack with only MINSIGSTKSZ bytes of space.
> +
> + This test program tests calls to _exit, which is the same function
> + as _Exit, but specified by POSIX rather than ISO C. For reasons
> + unknown to the author of this program, the C committee did not
> + think it could standardize _exit under that name; regardless, in a
> + POSIX-conformant environment, they should be completely
> + interchangeable. */
OK. Agreed.
> +
> +#define EXPECTED_STATUS 3
OK.
> +
> +static void
> +handler (int unused)
> +{
> + _exit (EXPECTED_STATUS);
OK.
> +}
> +
> +int
> +do_test (void)
> +{
> + void *sstk = xalloc_sigstack (0);
> + struct sigaction sa;
> +
> + sa.sa_handler = handler;
> + sa.sa_flags = SA_RESTART | SA_ONSTACK;
> + sigfillset (&sa.sa_mask);
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> + raise (SIGUSR1);
> +
> + xfree_sigstack (sstk);
> + FAIL_RET ("test process was not terminated by _exit in signal handler");
> +}
OK.
> +
> +#include <support/test-driver.c>
> diff --git a/signal/tst-minsigstksz-4.c b/signal/tst-minsigstksz-4.c
> new file mode 100644
> index 0000000000..0dc63b4dd4
> --- /dev/null
> +++ b/signal/tst-minsigstksz-4.c
> @@ -0,0 +1,65 @@
> +/* Tests of signal delivery on an alternate stack (quick_exit).
OK.
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/check.h>
> +#include <stdlib.h>
> +
> +/* C2011 7.4.1.1p5 specifies that only the following operations are
> + guaranteed to be well-defined inside an asynchronous signal handler:
> + * any operation on a lock-free atomic object
> + * assigning a value to an object declared as volatile sig_atomic_t
> + * calling abort, _Exit, quick_exit, or signal
> + * signal may only be called with its first argument equal to the
> + number of the signal that caused the handler to be called
> +
> + We use this list as a guideline for the set of operations that ought
> + also to be safe in a _synchronous_ signal delivered on an alternate
> + signal stack with only MINSIGSTKSZ bytes of space.
> +
> + This test program tests calls to quick_exit. Note that this is only
> + safe when there are no at_quick_exit callbacks. */
OK.
> +
> +#define EXPECTED_STATUS 3
> +
> +static void
> +handler (int unused)
> +{
> + quick_exit (EXPECTED_STATUS);
OK.
> +}
> +
> +int
> +do_test (void)
> +{
> + void *sstk = xalloc_sigstack (0);
> + struct sigaction sa;
> +
> + sa.sa_handler = handler;
> + sa.sa_flags = SA_RESTART | SA_ONSTACK;
> + sigfillset (&sa.sa_mask);
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> + raise (SIGUSR1);
> +
> + xfree_sigstack (sstk);
> + FAIL_RET ("test process was not terminated by quick_exit in signal handler");
OK.
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/support/Makefile b/support/Makefile
> index 93a5143016..7fb2de8cee 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -145,6 +145,7 @@ libsupport-routines = \
> xsetsockopt \
> xsigaction \
> xsignal \
> + xsigstack \
OK.
> xsocket \
> xstrdup \
> xstrndup \
> @@ -205,6 +206,7 @@ tests = \
> tst-test_compare_blob \
> tst-test_compare_string \
> tst-xreadlink \
> + tst-xsigstack \
OK.
>
> ifeq ($(run-built-tests),yes)
> tests-special = \
> diff --git a/support/tst-xsigstack.c b/support/tst-xsigstack.c
> new file mode 100644
> index 0000000000..42859c79e9
> --- /dev/null
> +++ b/support/tst-xsigstack.c
> @@ -0,0 +1,64 @@
> +/* Test of sigaltstack wrappers.
OK.
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +
> +#include <stdint.h>
> +#include <stdio.h>
> +
> +static volatile uintptr_t handler_stackaddr;
> +
> +static void
> +handler (int unused)
> +{
> + int var;
> + handler_stackaddr = (uintptr_t) &var;
> +}
> +
> +int
> +do_test (void)
> +{
> + void *sstk = xalloc_sigstack (0);
OK.
> +
> + unsigned char *sp;
> + size_t size;
> + xget_sigstack_location (sstk, &sp, &size);
OK.
> + printf ("signal stack installed: sp=%p size=%zu\n", sp, size);
> +
> + struct sigaction sa;
> + sa.sa_handler = handler;
> + sa.sa_flags = SA_RESTART | SA_ONSTACK;
> + sigfillset (&sa.sa_mask);
> + if (sigaction (SIGUSR1, &sa, 0))
> + FAIL_RET ("sigaction (SIGUSR1, handler): %m\n");
> +
> + raise (SIGUSR1);
> +
> + uintptr_t haddr = handler_stackaddr;
> + printf ("address of handler local variable: %p\n", (void *)haddr);
> + TEST_VERIFY ((uintptr_t)sp < haddr);
OK.
> + TEST_VERIFY (haddr < (uintptr_t)sp + size);
OK. Probably the best we can do.
> +
> + xfree_sigstack (sstk);
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/support/xsignal.h b/support/xsignal.h
> index 9ab8d1bfdd..3cf2a6e434 100644
> --- a/support/xsignal.h
> +++ b/support/xsignal.h
> @@ -37,6 +37,24 @@ void xsigaction (int sig, const struct sigaction *newact,
>
> void xpthread_sigmask (int how, const sigset_t *set, sigset_t *oldset);
>
> +/* Allocate and activate an alternate signal stack. This stack will
> + have at least MINSIGSTKSZ + size bytes of space, rounded up to a
> + whole number of pages. There will be large (at least 1 MiB)
> + inaccessible guard bands on either side of it. The return value is
> + a cookie that can be passed to xfree_sigstack to deactivate and
> + deallocate the stack again.
> + It is _not_ necessary to call sigaltstack() after calling this function.
> + Terminates the process on error. */
> +void *xalloc_sigstack (size_t size);
> +
> +/* Deactivate and deallocate a signal stack created by xalloc_sigstack. */
> +void xfree_sigstack (void *stack);
> +
> +/* Extract the actual address and size of the alternate signal stack from
> + the cookie returned by xalloc_sigstack. */
> +void xget_sigstack_location (const void *stack, unsigned char **addrp,
> + size_t *sizep);
OK.
> +
> __END_DECLS
>
> #endif /* SUPPORT_SIGNAL_H */
> diff --git a/support/xsigstack.c b/support/xsigstack.c
> new file mode 100644
> index 0000000000..74b459e2e6
> --- /dev/null
> +++ b/support/xsigstack.c
> @@ -0,0 +1,95 @@
> +/* sigaltstack wrappers.
> + Copyright (C) 2019 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
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <support/xsignal.h>
> +#include <support/support.h>
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/param.h> /* roundup, MAX */
> +
> +/* The "cookie" returned by xalloc_sigstack points to one of these
> + structures. */
> +struct sigstack_desc
> +{
> + void *alloc_base; /* Base address of the complete allocation. */
> + size_t alloc_size; /* Size of the complete allocation. */
> + stack_t alt_stack; /* The address and size of the stack itself. */
> + stack_t old_stack; /* The previous signal stack. */
> +};
OK.
> +
> +void *
> +xalloc_sigstack (size_t size)
> +{
> + size_t pagesize = sysconf (_SC_PAGESIZE);
> + if (pagesize == -1)
> + FAIL_EXIT1 ("sysconf (_SC_PAGESIZE): %m\n");
> +
Both of these choices need a comment explaining why or if they are
arbitrary, that they were just chosen at random.
/* Per the API we allocate MINSIGSTKSZ when xalloc_sigstack(0)
is called. */
> + size_t stacksize = roundup (size + MINSIGSTKSZ, pagesize);
Suggest:
/* We want a large enough guard to catch any jumps around the guard
page that might take us to another mapping. The choice of a 1MiB
guard as a minimum is somewhat arbitrary, but if the stacksize is
large we use that as the guard size on the assumption that an index
into an array of a stack sized variable might be off by one and jump
the entire stack. */
> + size_t guardsize = MAX (stacksize, roundup (1024 * 1024, pagesize));
> +
> + struct sigstack_desc *desc = xmalloc (sizeof (struct sigstack_desc));
> + desc->alloc_size = guardsize + stacksize + guardsize;
> + /* Use MAP_NORESERVE so that RAM will not be wasted on the guard
> + bands; touch all the pages of the actual stack before returning,
> + so we know they are allocated. */
> + desc->alloc_base = xmmap (0,
> + desc->alloc_size,
> + PROT_READ|PROT_WRITE,
> + MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE|MAP_STACK,
> + -1);
OK.
> +
> + xmprotect (desc->alloc_base, guardsize, PROT_NONE);
> + xmprotect (desc->alloc_base + guardsize + stacksize, guardsize, PROT_NONE);
> + memset (desc->alloc_base + guardsize, 0xA5, stacksize);
OK.
> +
> + desc->alt_stack.ss_sp = desc->alloc_base + guardsize;
> + desc->alt_stack.ss_flags = 0;
> + desc->alt_stack.ss_size = stacksize;
OK.
> +
> + if (sigaltstack (&desc->alt_stack, &desc->old_stack))
> + FAIL_EXIT1 ("sigaltstack (new stack: sp=%p, size=%zu, flags=%u): %m\n",
> + desc->alt_stack.ss_sp, desc->alt_stack.ss_size,
> + desc->alt_stack.ss_flags);
OK.
> +
> + return desc;
> +}
> +
> +void
> +xfree_sigstack (void *stack)
> +{
> + struct sigstack_desc *desc = stack;
> +
> + if (sigaltstack (&desc->old_stack, 0))
> + FAIL_EXIT1 ("sigaltstack (restore old stack: sp=%p, size=%zu, flags=%u): "
> + "%m\n", desc->old_stack.ss_sp, desc->old_stack.ss_size,
> + desc->old_stack.ss_flags);
> + xmunmap (desc->alloc_base, desc->alloc_size);
OK.
> + free (desc);
> +}
> +
> +void
> +xget_sigstack_location (const void *stack, unsigned char **addrp, size_t *sizep)
> +{
> + const struct sigstack_desc *desc = stack;
> + *addrp = desc->alt_stack.ss_sp;
> + *sizep = desc->alt_stack.ss_size;
OK.
> +}
>
--
Cheers,
Carlos.