Fix BZ#20544 (assert function passed to atexit/on_exit/__cxa_atexit != NULL)
Commit Message
On Tue, Nov 13, 2018 at 5:59 AM Gabriel F. T. Gomes
<gabriel@inconstante.eti.br> wrote:
> FAIL_UNSUPPORTED ("Assert disabled, can't verify that assertions fire.");
Thanks. Revised patch attached.
Comments
On Tue, 13 Nov 2018, Paul Pluzhnikov wrote:
>Thanks. Revised patch attached.
Looks good to me.
* Paul Pluzhnikov:
> + atexit (NULL); /* Should assert. */
> + exit (EXIT_FAILURE);
I think this will print assertion failure messages to the build log,
which is confusing to anyone reviewing it for unexpected errors. Can we
avoid this?
Thanks,
Florian
@@ -1,3 +1,11 @@
+2018-11-13 Paul Pluzhnikov <ppluzhnikov@google.com>
+
+ [BZ #20544]
+ * stdlib/cxa_atexit.c (__internal_atexit): assert func != NULL.
+ * stdlib/on_exit.c (__on_exit): Likewise.
+ * stdlib/Makefile (tests): Add tst-bz20544.
+ * stdlib/tst-bz20544.c: New test.
+
2018-11-13 Florian Weimer <fweimer@redhat.com>
* malloc/malloc.c (fastbin_push_entry): New function.
@@ -87,7 +87,7 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \
tst-makecontext-align test-bz22786 tst-strtod-nan-sign \
tst-swapcontext1 tst-setcontext4 tst-setcontext5 \
tst-setcontext6 tst-setcontext7 tst-setcontext8 \
- tst-setcontext9
+ tst-setcontext9 tst-bz20544
tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
tst-tls-atexit tst-tls-atexit-nodelete
@@ -36,6 +36,10 @@ __internal_atexit (void (*func) (void *), void *arg, void *d,
{
struct exit_function *new;
+ /* As a QoI issue we detect NULL early with an assertion instead
+ of a SIGSEGV at program exit when the handler is run. BZ#20544 */
+ assert (func != NULL);
+
__libc_lock_lock (__exit_funcs_lock);
new = __new_exitfn (listp);
@@ -15,6 +15,7 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
+#include <assert.h>
#include <stdlib.h>
#include "exit.h"
#include <sysdep.h>
@@ -25,6 +26,10 @@ __on_exit (void (*func) (int status, void *arg), void *arg)
{
struct exit_function *new;
+ /* As a QoI issue we detect NULL early with an assertion instead
+ of a SIGSEGV at program exit when the handler is run. BZ#20544 */
+ assert (func != NULL);
+
__libc_lock_lock (__exit_funcs_lock);
new = __new_exitfn (&__exit_funcs);
new file mode 100644
@@ -0,0 +1,72 @@
+/* Verify atexit, on_exit, etc. abort on NULL function pointer.
+ Copyright (C) 2018 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 <signal.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+
+extern int __cxa_atexit (void (*func) (void *), void *arg, void *d);
+
+#pragma GCC diagnostic ignored "-Wnonnull"
+
+static int
+do_test (void)
+{
+#if defined(NDEBUG)
+ FAIL_UNSUPPORTED("Assertions disabled (NDEBUG). "
+ "Can't verify that assertions fire.");
+#else
+ int j;
+
+ for (j = 0; j < 3; j++) {
+ pid_t pid = xfork ();
+ if (pid == 0) {
+ /* Child. */
+ switch (j) {
+ case 0:
+ atexit (NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+ case 1:
+ on_exit (NULL, NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+ case 2:
+ __cxa_atexit (NULL, NULL, NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+ default:
+ /* We shouldn't be here. */
+ exit (EXIT_FAILURE);
+ }
+ } else {
+ /* Parent. Verify child exited with SIGABRT. */
+ int status;
+
+ TEST_VERIFY (pid == xwaitpid (pid, &status, 0));
+ TEST_VERIFY (WIFSIGNALED (status));
+ TEST_VERIFY (WTERMSIG (status) == SIGABRT);
+ }
+ }
+#endif /* NDEBUG */
+
+ return 0;
+}
+
+#include <support/test-driver.c>