[coreboot-gerrit] [S] Change in coreboot[main]: lib/jpeg: avoid calling malloc and free

[Nigel Tao (Code Review)]

Nigel Tao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )

Change subject: lib/jpeg: avoid calling malloc and free
......................................................................

lib/jpeg: avoid calling malloc and free

Since commit 1d029b40c9de ("lib/jpeg: Replace decoder with Wuffs'
implementation"), a relatively large heap allocation is needed to decode
many JPEGs for use as work area. The prior decoder did not need this,
but also had many limitations in the JPEGs it could decode, was not as
memory-safe and quickly crashed under fuzzing.

This commit keeps using Wuffs' JPEG decoder, but it no longer requires
any heap allocation (and thus configuring the heap size depending on how
big a bootsplash image you want to support).

Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba
Signed-off-by: Nigel Tao 
---
M src/lib/jpeg.c
1 file changed, 20 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/83895/1

diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c
index 242cf0c..4fa850b 100644
--- a/src/lib/jpeg.c
+++ b/src/lib/jpeg.c
@@ -85,6 +85,24 @@
    	return JPEG_DECODE_FAILED;
    }

+	/* Opting in to lower quality means that we can pass an empty slice as the
+	 * "work buffer" argument to wuffs_jpeg__decoder__decode_frame below.
+	 *
+	 * Decoding progressive (not sequential) JPEGs would still require dynamic
+	 * memory allocation (and the amount of work buffer required depends on the
+	 * image dimensions), but we choose to just reject progressive JPEGs. It is
+	 * simpler than sometimes calling malloc (which can fail, especially for
+	 * large allocations) and free.
+	 *
+	 * More commentary about these quirks is at
+	 * https://github.com/google/wuffs/blob/beaf45650085a16780b5f708b72daaeb1aa865c...
+	 */
+	wuffs_jpeg__decoder__set_quirk(
+		&dec, WUFFS_BASE__QUIRK_QUALITY,
+		WUFFS_BASE__QUIRK_QUALITY__VALUE__LOWER_QUALITY);
+	wuffs_jpeg__decoder__set_quirk(
+		&dec, WUFFS_JPEG__QUIRK_REJECT_PROGRESSIVE_JPEGS, 1);
+
    wuffs_base__image_config imgcfg;
    wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true);
    status = wuffs_jpeg__decoder__decode_image_config(&dec, &imgcfg, &src);
@@ -104,19 +122,9 @@
    	return JPEG_DECODE_FAILED;
    }

-	uint64_t workbuf_len_min_incl = wuffs_jpeg__decoder__workbuf_len(&dec).min_incl;
-	uint8_t *workbuf_array = malloc(workbuf_len_min_incl);
-	if ((workbuf_array == NULL) && workbuf_len_min_incl) {
-		return JPEG_DECODE_FAILED;
-	}
-
-	wuffs_base__slice_u8 workbuf =
-		wuffs_base__make_slice_u8(workbuf_array, workbuf_len_min_incl);
    status = wuffs_jpeg__decoder__decode_frame(&dec, &pixbuf, &src,
-						   WUFFS_BASE__PIXEL_BLEND__SRC, workbuf, NULL);
-
-	free(workbuf_array);
-
+						   WUFFS_BASE__PIXEL_BLEND__SRC,
+						   wuffs_base__empty_slice_u8(), NULL);
    if (status.repr) {
    	return JPEG_DECODE_FAILED;
    }