Re: [PATCH] vasnprintf: fix potential use after free

[Pádraig Brady]

On 06/12/14 02:46, Eric Blake wrote:
> On 12/05/2014 06:23 PM, Pádraig Brady wrote:
>> * lib/vasnprintf.c (VASNPRINTF): Fix free-memory read,
>> flagged by clang-analyzer 3.4.2.
>> ---
>>  ChangeLog        | 6 ++++++
>>  lib/vasnprintf.c | 2 +-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
> 
>> +++ b/lib/vasnprintf.c
>> @@ -5184,13 +5184,13 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp,
>>                            free (result);
>>                          if (buf_malloced != NULL)
>>                            free (buf_malloced);
>> -                        CLEANUP ();
>>                          errno =
>>                            (saved_errno != 0
>>                             ? saved_errno
>>                             : (dp->conversion == 'c' || dp->conversion == 's'
>>                                ? EILSEQ
>>                                : EINVAL));
>> +                        CLEANUP ();
> 
> Ouch.  This is a bug.  The whole point of assigning to errno after
> CLEANUP() is that CLEANUP() may invalidate the value stored in errno.
> 
> I suggest doing something like:
> 
> if (saved_errno == 0)
>   saved_errno = dp->conversion == 'c' || dp->conversion == 's'
>     ? EILSEQ : EINVAL;
> CLEANUP();
> errno = saved_errno;

Oh right, I did think about it and looked at:
http://austingroupbugs.net/view.php?id=385
but jumped to the wrong conclusion that the updated standard
was just documenting existing practise.

thanks,
Pádraig