| Message ID | CALoOobN2H0dRjGUT_XSwYSO0pE_kWyVDoGrg+dSN_J4J5zpyJQ@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On 08/27/2018 02:12 PM, Paul Pluzhnikov wrote: > Greetings, > > As discussed in bz#20544 and in > http://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers, > we should assert that the function passed to atexit, on_exit, etc. is > not NULL. This needs a test case that forks a child, runs the test, and catches a SIGABRT showing the failure resulting from the invalid API call. The test should cover all of the APIs we can tests by passing NULL. It is OK to call __cxa_atexit directly even though that might be invalid in a real program. > 2018-08-27 Paul Pluzhnikov <ppluzhnikov@google.com> > > [BZ #20544] > * stdlib/cxa_atexit.c (__internal_atexit): assert func != NULL. > * stdlib/on_exit.c (__on_exit): Likewise. > > > -- Paul Pluzhnikov > > > glibc-bz20544-20180825.txt > > > diff --git a/stdlib/cxa_atexit.c b/stdlib/cxa_atexit.c > index 6d65f7e615..978f873990 100644 > --- a/stdlib/cxa_atexit.c > +++ b/stdlib/cxa_atexit.c > @@ -36,6 +36,9 @@ __internal_atexit (void (*func) (void *), void *arg, void *d, > { > struct exit_function *new; > > + /* BZ#20544 */ This should comment on why the assert is here. /* As a QoI issue we detect NULL early with an assertion instead of a SIGSEGV at program exit when the handler is run. */ > + 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..0ead1eafd4 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 > <http://www.gnu.org/licenses/>. */ > > +#include <assert.h> OK. > #include <stdlib.h> > #include "exit.h" > #include <sysdep.h> > @@ -25,6 +26,9 @@ __on_exit (void (*func) (int status, void *arg), void *arg) > { > struct exit_function *new; > > + /* BZ#20544 */ Likewise. > + assert (func != NULL); > + > __libc_lock_lock (__exit_funcs_lock); > new = __new_exitfn (&__exit_funcs); >
diff --git a/stdlib/cxa_atexit.c b/stdlib/cxa_atexit.c index 6d65f7e615..978f873990 100644 --- a/stdlib/cxa_atexit.c +++ b/stdlib/cxa_atexit.c @@ -36,6 +36,9 @@ __internal_atexit (void (*func) (void *), void *arg, void *d, { struct exit_function *new; + /* 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..0ead1eafd4 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 <http://www.gnu.org/licenses/>. */ +#include <assert.h> #include <stdlib.h> #include "exit.h" #include <sysdep.h> @@ -25,6 +26,9 @@ __on_exit (void (*func) (int status, void *arg), void *arg) { struct exit_function *new; + /* BZ#20544 */ + assert (func != NULL); + __libc_lock_lock (__exit_funcs_lock); new = __new_exitfn (&__exit_funcs);
Greetings, As discussed in bz#20544 and in http://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers, we should assert that the function passed to atexit, on_exit, etc. is not NULL. 2018-08-27 Paul Pluzhnikov <ppluzhnikov@google.com> [BZ #20544] * stdlib/cxa_atexit.c (__internal_atexit): assert func != NULL. * stdlib/on_exit.c (__on_exit): Likewise.