Peter Maydell <[email protected]> writes:
> On Wed, 8 Oct 2025 at 20:17, Daan De Meyer <[email protected]> wrote:
>>
>> Signed-off-by: Daan De Meyer <[email protected]>
>> ---
>> hw/arm/boot.c | 2 +-
>> hw/core/loader.c | 36 ++++++++++++++++++++++++------------
>> hw/nvram/fw_cfg.c | 2 +-
>> include/hw/loader.h | 2 +-
>> 4 files changed, 27 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index e77d8679d8..c0dec0343a 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -826,7 +826,7 @@ static uint64_t load_aarch64_image(const char *filename,
>> hwaddr mem_base,
>> ssize_t size;
>>
>> /* On aarch64, it's the bootloader's job to uncompress the kernel. */
>> - size = load_image_gzipped_buffer(filename, LOAD_IMAGE_MAX_GUNZIP_BYTES,
>> + size = load_image_gzipped_buffer(filename,
>> LOAD_IMAGE_MAX_DECOMPRESSED_BYTES,
>> &buffer);
>
> I would either not bother renaming this constant, or else do
> it in a preliminary patch of its own in the series. (That
> makes the part of the patch which is making the functional
> change easier to read and review.)
>
>> @@ -882,14 +887,6 @@ ssize_t unpack_efi_zboot_image(uint8_t **buffer,
>> ssize_t *size)
>> return 0;
>> }
>>
>> - if (strcmp(header->compression_type, "gzip") != 0) {
>> - fprintf(stderr,
>> - "unable to handle EFI zboot image with \"%.*s\"
>> compression\n",
>> - (int)sizeof(header->compression_type) - 1,
>> - header->compression_type);
>> - return -1;
>> - }
>> -
>> ploff = ldl_le_p(&header->payload_offset);
>> plsize = ldl_le_p(&header->payload_size);
>>
>> @@ -898,8 +895,23 @@ ssize_t unpack_efi_zboot_image(uint8_t **buffer,
>> ssize_t *size)
>> return -1;
>> }
>>
>> - data = g_malloc(LOAD_IMAGE_MAX_GUNZIP_BYTES);
>> - bytes = gunzip(data, LOAD_IMAGE_MAX_GUNZIP_BYTES, *buffer + ploff,
>> plsize);
>> + data = g_malloc(LOAD_IMAGE_MAX_DECOMPRESSED_BYTES);
>> +
>> + if (strcmp(header->compression_type, "gzip") == 0) {
>> + bytes = gunzip(data, LOAD_IMAGE_MAX_DECOMPRESSED_BYTES, *buffer +
>> ploff, plsize);
>> +#ifdef CONFIG_ZSTD
>> + } else if (strcmp(header->compression_type, "zstd") == 0) {
>> + size_t ret = ZSTD_decompress(data,
>> LOAD_IMAGE_MAX_DECOMPRESSED_BYTES, *buffer + ploff, plsize);
>> + bytes = ZSTD_isError(ret) ? -1 : (ssize_t) ret;
>> +#endif
>> + } else {
>> + fprintf(stderr,
>> + "unable to handle EFI zboot image with \"%.*s\"
>> compression\n",
>> + (int)sizeof(header->compression_type) - 1,
>> + header->compression_type);
>> + return -1;
>
> Moving the "unrecognized compression type" error path down to
> here means that we have moved it below the g_malloc() of the
> data buffer, so we now need to g_free() to avoid a leak.
Could we not declare:
g_autofree uint8_t *data = NULL;
and drop the cleanup?
>
>> + }
>> +
>> if (bytes < 0) {
>> fprintf(stderr, "failed to decompress EFI zboot image\n");
>> g_free(data);
>
> Otherwise I think this looks OK.
>
> thanks
> -- PMM
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
