Re: [PATCH] avoid -Wuse-after-free [BZ #26779]

[Paul Eggert <eggert@cs.ucla.edu>]

On 1/15/22 16:21, Martin Sebor via Libc-alpha wrote:

> +		      intptr_t ip_new_pool = (intptr_t)new_pool;
> +		      intptr_t ptr_diff = ip_new_pool - ip_string_space;

This sort of thing can have undefined behavior due to signed integer 
overflow. To avoid that problem on glibc platforms, you can use 
uintptr_t instead of intptr_t.

> -			      map[i].alias += new_pool - string_space;
> -			      map[i].value += new_pool - string_space;
> +			      map[i].alias += ptr_diff;
> +			      map[i].value += ptr_diff;

Although this might pacify GCC 12, it continues to have the undefined 
behavior that GCC is evidently warning about, since the old values of 
map[i].alias and map[i].value have been freed and this means programs 
cannot use those pointers even if only to add something to them. It's 
conceivable that future GCC versions will figure this out and generate 
code that doesn't do what we want here.

One simple workaround would be for the .alias and .value components to 
be offsets into the storage pool, instead of being absolute pointers.

> --- a/stdlib/setenv.c
> +++ b/stdlib/setenv.c
> @@ -150,7 +150,9 @@ __add_to_environ (const char *name, const char *value, const char *combined,
>      {
>        char **new_environ;
>  
> -      /* We allocated this space; we can extend it.  */
> +      /* We allocated this space; we can extend it.  Avoid using the raw
> +	 reallocated pointer to avoid GCC -Wuse-after-free.  */
> +      uintptr_t ip_last_environ = (uintptr_t)last_environ;
>        new_environ = (char **) realloc (last_environ,
>  				       (size + 2) * sizeof (char *));
>        if (new_environ == NULL)

Cleaner would be to leave the old code alone, except to add this before 
reallocating:

       bool we_allocated_environ = __environ == last_environ;

 > ...
> -      if (__environ != last_environ)
> +      if ((uintptr_t)__environ != ip_last_environ)

And then change the above line to "if (! we_allocated_environ)".

>  	memcpy ((char *) new_environ, (char *) __environ,
>  		size * sizeof (char *));
>