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

[Eric Blake]

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;


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature