Fix BZ#20544 (assert function passed to atexit/on_exit/__cxa_atexit != NULL)
Commit Message
On Thu, Nov 29, 2018 at 7:06 AM Florian Weimer <fweimer@redhat.com> wrote:
> Perhaps:
>
> stdlib: assert on NULL function pointer in atexit etc. [BZ #20544]
Thanks. Revised patch attached.
Comments
* Paul Pluzhnikov:
> On Thu, Nov 29, 2018 at 7:06 AM Florian Weimer <fweimer@redhat.com> wrote:
>
>> Perhaps:
>>
>> stdlib: assert on NULL function pointer in atexit etc. [BZ #20544]
>
> Thanks. Revised patch attached.
I think this version is okay.
Thanks,
Florian
On Thu, Nov 29, 2018 at 8:33 AM Florian Weimer <fweimer@redhat.com> wrote:
> I think this version is okay.
Thanks. I'll submit this on 2018-12-01 if there are no further
comments until then.
@@ -1,3 +1,11 @@
+2018-11-29 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-29 Florian Weimer <fweimer@redhat.com>
* posix/Makefile (before-compile): Remove testcases.h and
@@ -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 (bug 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 (bug 20544). */
+ assert (func != NULL);
+
__libc_lock_lock (__exit_funcs_lock);
new = __new_exitfn (&__exit_funcs);
new file mode 100644
@@ -0,0 +1,115 @@
+/* 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 <assert.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+
+extern int __cxa_atexit (void (*func) (void *), void *arg, void *d);
+extern int __cxa_at_quick_exit (void (*func) (void *), void *arg, void *d);
+
+/* GCC "knows" that atexit and on_exit should not be called with NULL
+ function pointer, and emits diagnostics if we try to do so.
+ Presumably it could emit a trap and drop the call altogether.
+
+ The aliases below are intended to bypass this. */
+
+extern int atexit_alias (void (*) (void)) __asm__ ("atexit");
+extern int at_quick_exit_alias (void (*) (void)) __asm__ ("at_quick_exit");
+extern int on_exit_alias (void (*) (void), void *) __asm__ ("on_exit");
+
+
+static void
+test_bz20544_atexit (void *closure)
+{
+ atexit_alias (NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+}
+
+static void
+test_bz20544_at_quick_exit (void *closure)
+{
+ at_quick_exit_alias (NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+}
+
+static void
+test_bz20544_on_exit (void *closure)
+{
+ on_exit_alias (NULL, NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+}
+
+static void
+test_bz20544_cxa_atexit (void *closure)
+{
+ __cxa_atexit (NULL, NULL, NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+}
+
+static void
+test_bz20544_cxa_at_quick_exit (void *closure)
+{
+ __cxa_at_quick_exit (NULL, NULL, NULL); /* Should assert. */
+ exit (EXIT_FAILURE);
+}
+
+static void
+test_one_fn (void (*test_fn) (void *))
+{
+ const char expected_error[] = "Assertion `func != NULL' failed.\n";
+ struct support_capture_subprocess result;
+ result = support_capture_subprocess (test_fn, NULL);
+ support_capture_subprocess_check (&result, "bz20544", -SIGABRT,
+ sc_allow_stderr);
+
+ if (strstr (result.err.buffer, expected_error) == NULL)
+ {
+ support_record_failure ();
+ printf ("Did not find expected string in error output:\n"
+ " expected: >>>%s<<<\n"
+ " actual: >>>%s<<<\n",
+ expected_error, result.err.buffer);
+ }
+
+ support_capture_subprocess_free (&result);
+}
+
+static int
+do_test (void)
+{
+#if defined (NDEBUG)
+ FAIL_UNSUPPORTED ("Assertions disabled (NDEBUG). "
+ "Can't verify that assertions fire.");
+#endif
+ test_one_fn (test_bz20544_atexit);
+ test_one_fn (test_bz20544_at_quick_exit);
+ test_one_fn (test_bz20544_on_exit);
+ test_one_fn (test_bz20544_cxa_atexit);
+ test_one_fn (test_bz20544_cxa_at_quick_exit);
+
+ return 0;
+}
+
+#include <support/test-driver.c>