[PATCH v2] lib: gui: png_pico: fix use-after-free and double-free in png_open

[Johannes Schneider]

png_open() set img->data from png_info->image->data and then called
png_alloc_free_all(), which freed every buffer the picopng allocator
tracks -- including the decoded pixel buffer.  Callers held a
dangling img->data, and the later png_close() free()'d it again.

Add png_alloc_detach() to drop a tracked address from the allocator
without freeing it, transferring ownership to the caller, and use it
in png_open() before png_alloc_free_all() runs.

Suggested-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
Signed-off-by: Johannes Schneider <johannes.schneider at leica-geosystems.com>
---
 lib/gui/picopng.c  | 13 ++++++++++++-
 lib/gui/picopng.h  |  7 +++++++
 lib/gui/png_pico.c |  8 +++++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/lib/gui/picopng.c b/lib/gui/picopng.c
index 80f03beb68..ae733fde0f 100644
--- a/lib/gui/picopng.c
+++ b/lib/gui/picopng.c
@@ -103,7 +103,7 @@ static void png_alloc_free(void *addr)
 	free(addr);
 }
 
-void png_alloc_free_all()
+void png_alloc_free_all(void)
 {
 	while (png_alloc_tail) {
 		void *addr = png_alloc_tail->addr;
@@ -112,6 +112,17 @@ void png_alloc_free_all()
 	}
 }
 
+void *png_alloc_detach(void *addr)
+{
+	png_alloc_node_t *node = png_alloc_find_node(addr);
+
+	if (!node)
+		return NULL;
+
+	png_alloc_remove_node(node);
+	return addr;
+}
+
 /*************************************************************************************************/
 
 __maybe_unused static void vector32_cleanup(vector32_t *p)
diff --git a/lib/gui/picopng.h b/lib/gui/picopng.h
index a17dd14b0c..bad5f4c6c4 100644
--- a/lib/gui/picopng.h
+++ b/lib/gui/picopng.h
@@ -28,6 +28,13 @@ typedef struct {
 PNG_info_t *PNG_decode(const uint8_t *in, uint32_t size);
 void png_alloc_free_all(void);
 
+/*
+ * Remove @addr from the picopng allocator's bookkeeping without freeing it,
+ * transferring ownership of the buffer to the caller (who must free() it).
+ * Returns @addr on success or NULL if @addr was not tracked.
+ */
+void *png_alloc_detach(void *addr);
+
 unsigned picopng_zlib_decompress(unsigned char* out, size_t outsize,
 				 const unsigned char* in, size_t insize);
 
diff --git a/lib/gui/png_pico.c b/lib/gui/png_pico.c
index 029fee2a40..bf6eddb74b 100644
--- a/lib/gui/png_pico.c
+++ b/lib/gui/png_pico.c
@@ -67,7 +67,13 @@ struct image *png_open(char *inbuf, int insize)
 	img->width = png_info->width;
 	img->height = png_info->height;
 	img->bits_per_pixel = 4 << 3;
-	img->data = png_info->image->data;
+
+	/* detach so png_alloc_free_all() below leaves the pixel buffer alive */
+	img->data = png_alloc_detach(png_info->image->data);
+	if (!img->data) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	pr_debug("png: %d x %d data at 0x%p\n", img->width, img->height, img->data);
 

base-commit: 651343da8af78d134d7ead4d2b36095d7ddc2d8f
-- 
2.43.0