Re: [PATCH v2 4/5] avoid -Wuse-after-free [BZ #26779]

[Carlos O'Donell <carlos@redhat.com>]

On 1/25/22 12:49, Carlos O'Donell wrote:
> On 1/24/22 19:58, Martin Sebor via Libc-alpha wrote:
>> On 1/24/22 17:52, Martin Sebor wrote:
>>> This is a repost of the original patch but broken down by source
>>> file and with some suppression done by #pragma GCC diagnostic
>>> instead of conversion to intptr_t.  It also adds fixes for
>>> the same problem in the test suite that I overlooked before.
>>
>> The attached patch suppresses the -Wuse-after-free instance in
>> stdlib/setenv.c.
>>
>>>
>>> On 1/15/22 17:21, Martin Sebor wrote:
>>>> GCC 12 features a couple of new warnings designed to detect uses
>>>> of pointers made invalid by the pointees lifetimes having ended.
>>>> Building Glibc with the enhanced GCC exposes a few such uses,
>>>> mostly after successful calls to realloc.  The attached patch
>>>> avoids the new warnings by converting the pointers to uintptr_t
>>>> first and using the converted integers instead.
>>>>
>>>> The patch suppresses all instances of the warning at the strictest
>>>> setting (-Wuse-after-free=3), which includes even uses in equality
>>>> expressions.  The default setting approved for GCC 12 is
>>>> -Wuse-after-free=2, which doesn't warn on such uses to accommodate
>>>> the pointer-adjustment-after-realloc idiom.  At the default setting,
>>>> the changes to ldconfig.c and setenv are not necessary.
>>>>
>>>> Martin
>>>
> 
> OK for glibc 2.35, please push this commit.
> 
> Expected commit message (three lines)
> ~~~
> io: Fix use-after-free in ftw [BZ #26779]


Should be:
~~~
stdlib: Avoid -Wuse-after-free [BZ #26779]

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
~~~

> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ~~~
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 
>> diff --git a/stdlib/setenv.c b/stdlib/setenv.c
>> index c3d2cee7b6..2176cbac31 100644
>> --- 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;
> 
> OK. Create a temporary pointer.
> 
>>        new_environ = (char **) realloc (last_environ,
>>  				       (size + 2) * sizeof (char *));
>>        if (new_environ == NULL)
>> @@ -159,7 +161,7 @@ __add_to_environ (const char *name, const char *value, const char *combined,
>>  	  return -1;
>>  	}
>>  
>> -      if (__environ != last_environ)
>> +      if ((uintptr_t)__environ != ip_last_environ)
> 
> OK. Lastly, use the temporary pointer for the comparison.
> 
>>  	memcpy ((char *) new_environ, (char *) __environ,
>>  		size * sizeof (char *));
>>  
> 
> 


-- 
Cheers,
Carlos.