From patchwork Wed Nov 28 19:00:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 30367 Received: (qmail 63852 invoked by alias); 28 Nov 2018 19:01:13 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 63473 invoked by uid 89); 28 Nov 2018 19:01:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-31.4 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SEM_URI, SEM_URIRED, SPF_PASS, USER_IN_DEF_SPF_WL autolearn=ham version=3.3.2 spammy=sigsegv, SIGSEGV X-HELO: mail-yb1-f195.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3qs1A/rEokzKwSkov7cwBloJ4peQufRYjLi+TV3cKG4=; b=CMYLBFnR2Bdabo5bILyVotbRKnuSiCVKzwDPrY3IQYYZ5QEC4g86ayhYPNmxe/HNE8 O/Jt/FBdeyx7kpiYLPgyra4kkFp6Xj2DrQToyaRjjFnC5DAsWcmxpenX64JYGdsqMHpa 3oOyxlwh4baYfVJF0a2S+7a4DOyN9d9MSHAlE2pUUC2OSX+OiDgdhbYNm4d0kVa/RVk/ fwL3siBO2wqsGtrP1JoZFJlCpl33UPyAujK/e6b51S/nV/T6Slkv12cz7cj1pU+Pvge8 y7QX33w8OnDG0S8X8M6KRte1FiJq8wFhb3rioMFzEUshtEZeMuipi+RJPeAUU/H48y4n VaWw== MIME-Version: 1.0 References: <03b9e2fe-0851-77b2-2ad4-d99aeaa362a2@redhat.com> <20181113115906.0d5e5b22@tereshkova.br.ibm.com> <87h8gja680.fsf@oldenburg.str.redhat.com> In-Reply-To: From: Paul Pluzhnikov Date: Wed, 28 Nov 2018 11:00:28 -0800 Message-ID: Subject: Re: [patch] Fix BZ#20544 (assert function passed to atexit/on_exit/__cxa_atexit != NULL) To: Adhemerval Zanella Cc: GLIBC Devel , Florian Weimer On Wed, Nov 28, 2018 at 10:19 AM Adhemerval Zanella wrote: > Yes, this is the most portable way indeed to get the expected termination > value and I think we should use the same strategy on this tests. Revised patch attached. Thanks! diff --git a/ChangeLog b/ChangeLog index 485555ff48..5767fd56f8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2018-11-28 Paul Pluzhnikov + + [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-28 Stefan Liebler * sysdeps/s390/fpu/libm-test-ulps: Regenerated. diff --git a/stdlib/Makefile b/stdlib/Makefile index 98dbddc43c..8bce89fffe 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -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 diff --git a/stdlib/cxa_atexit.c b/stdlib/cxa_atexit.c index 6d65f7e615..c1f146bfee 100644 --- a/stdlib/cxa_atexit.c +++ b/stdlib/cxa_atexit.c @@ -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); diff --git a/stdlib/on_exit.c b/stdlib/on_exit.c index 5241e0d86f..a8be62c513 100644 --- a/stdlib/on_exit.c +++ b/stdlib/on_exit.c @@ -15,6 +15,7 @@ License along with the GNU C Library; if not, see . */ +#include #include #include "exit.h" #include @@ -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); diff --git a/stdlib/tst-bz20544.c b/stdlib/tst-bz20544.c new file mode 100644 index 0000000000..7730824297 --- /dev/null +++ b/stdlib/tst-bz20544.c @@ -0,0 +1,125 @@ +/* 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 + . */ + + +#include +#include +#include +#include +#include +#include +#include + +extern int __cxa_atexit (void (*func) (void *), void *arg, void *d); + +#pragma GCC diagnostic ignored "-Wnonnull" + +#ifndef NDEBUG +static void +do_assert_failure (void *closure) +{ + assert (1 == 0); /* Should assert. */ +} + +static void +do_test_bz20544_atexit (void *closure) +{ + atexit (NULL); /* Should assert. */ + exit (EXIT_FAILURE); +} + +static void +do_test_bz20544_on_exit (void *closure) +{ + on_exit (NULL, NULL); /* Should assert. */ + exit (EXIT_FAILURE); +} + +static void +do_test_bz20544_cxa_atexit (void *closure) +{ + __cxa_atexit (NULL, NULL, NULL); /* Should assert. */ + exit (EXIT_FAILURE); +} +#endif + +static int +do_test (void) +{ +#if defined(NDEBUG) + FAIL_UNSUPPORTED("Assertions disabled (NDEBUG). " + "Can't verify that assertions fire."); +#else + int assert_termination_status; + { + /* Initialize system dependent assert failure exit code + (could be SIGABRT or 128+SIGABRT). */ + struct support_capture_subprocess result; + result = support_capture_subprocess (do_assert_failure, NULL); + assert_termination_status = result.status; + support_capture_subprocess_free (&result); + + if (WIFSIGNALED(assert_termination_status) == 0 || + WTERMSIG(assert_termination_status) != SIGABRT) + { + FAIL_EXIT1 ("Unexpected assert failure termination status: %d", + assert_termination_status); + } + } + + { + struct support_capture_subprocess result; + result = support_capture_subprocess (do_test_bz20544_atexit, NULL); + support_capture_subprocess_check (&result, "bz20544", + assert_termination_status, + sc_allow_stderr); + TEST_COMPARE_STRING (result.err.buffer, + "tst-bz20544: cxa_atexit.c:41: __internal_atexit: " \ + "Assertion `func != NULL' failed.\n"); + support_capture_subprocess_free (&result); + } + + { + struct support_capture_subprocess result; + result = support_capture_subprocess (do_test_bz20544_on_exit, NULL); + support_capture_subprocess_check (&result, "bz20544", + assert_termination_status, + sc_allow_stderr); + TEST_COMPARE_STRING (result.err.buffer, + "tst-bz20544: on_exit.c:31: __on_exit: " \ + "Assertion `func != NULL' failed.\n"); + support_capture_subprocess_free (&result); + } + + { + struct support_capture_subprocess result; + result = support_capture_subprocess (do_test_bz20544_cxa_atexit, NULL); + support_capture_subprocess_check (&result, "bz20544", + assert_termination_status, + sc_allow_stderr); + TEST_COMPARE_STRING (result.err.buffer, + "tst-bz20544: cxa_atexit.c:41: __internal_atexit: " \ + "Assertion `func != NULL' failed.\n"); + support_capture_subprocess_free (&result); + } +#endif /* NDEBUG */ + + return 0; +} + +#include