Re: [patch] Fix BZ#20544 (assert function passed to atexit/on_exit/__cxa_atexit != NULL)

[Carlos O'Donell <carlos at redhat dot com>]

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);
>  


-- 
Cheers,
Carlos.