Re: [PATCH] ui/console: fix three double frees in png_save()

[Philippe Mathieu-Daudé]

+Kshitij

On Sun, Sep 18, 2022 at 6:24 PM Volker Rümelin <vr_qemu@t-online.de> wrote:
>
> The png_destroy_write_struct() function frees all memory used by
> libpng. Don't use the glib auto cleanup mechanism to free the
> memory allocated by libpng again. For the pixman image, use only the
> auto cleanup mechanism and remove the qemu_pixman_image_unref()
> function call to prevent another double free.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1210
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  ui/console.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ui/console.c b/ui/console.c
> index 765892f84f..030e75bc71 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -304,8 +304,8 @@ static bool png_save(int fd, pixman_image_t *image, Error 
> **errp)
>  {
>      int width = pixman_image_get_width(image);
>      int height = pixman_image_get_height(image);
> -    g_autofree png_struct *png_ptr = NULL;
> -    g_autofree png_info *info_ptr = NULL;
> +    png_struct *png_ptr;
> +    png_info *info_ptr = NULL;

No need to NULL-initialize.

Fixes: 9a0a119a38 ("Added parameter to take screenshot with screendump as PNG")
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      g_autoptr(pixman_image_t) linebuf =
>                              qemu_pixman_linebuf_create(PIXMAN_a8r8g8b8, 
> width);
>      uint8_t *buf = (uint8_t *)pixman_image_get_data(linebuf);
> @@ -346,7 +346,6 @@ static bool png_save(int fd, pixman_image_t *image, Error 
> **errp)
>          qemu_pixman_linebuf_fill(linebuf, image, width, 0, y);
>          png_write_row(png_ptr, buf);
>      }
> -    qemu_pixman_image_unref(linebuf);
>
>      png_write_end(png_ptr, NULL);
>
> --
> 2.35.3
>
>