[PATCH] Add support for zboot images compressed with zstd

[PATCH] Add support for zboot images compressed with zstd

[Daan De Meyer]

Signed-off-by: Daan De Meyer <daan.j.demeyer@gmail.com>
---
 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);
 
     if (size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 477661a025..a2647b0c9a 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -67,6 +67,11 @@
 
 #include <zlib.h>
 
+#ifdef CONFIG_ZSTD
+#include <zstd.h>
+#include <zstd_errors.h>
+#endif
+
 static int roms_loaded;
 
 /* return the size or -1 if error */
@@ -796,8 +801,8 @@ ssize_t load_image_gzipped_buffer(const char *filename, uint64_t max_sz,
         goto out;
     }
 
-    if (max_sz > LOAD_IMAGE_MAX_GUNZIP_BYTES) {
-        max_sz = LOAD_IMAGE_MAX_GUNZIP_BYTES;
+    if (max_sz > LOAD_IMAGE_MAX_DECOMPRESSED_BYTES) {
+        max_sz = LOAD_IMAGE_MAX_DECOMPRESSED_BYTES;
     }
 
     data = g_malloc(max_sz);
@@ -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;
+    }
+
     if (bytes < 0) {
         fprintf(stderr, "failed to decompress EFI zboot image\n");
         g_free(data);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index aa24050493..af3b112524 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1115,7 +1115,7 @@ void load_image_to_fw_cfg(FWCfgState *fw_cfg, uint16_t size_key,
 
     if (try_decompress) {
         size = load_image_gzipped_buffer(image_name,
-                                         LOAD_IMAGE_MAX_GUNZIP_BYTES, &data);
+                                         LOAD_IMAGE_MAX_DECOMPRESSED_BYTES, &data);
     }
 
     if (size == (size_t)-1) {
diff --git a/include/hw/loader.h b/include/hw/loader.h
index c96b5e141c..24b91ba02b 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -80,7 +80,7 @@ ssize_t load_image_mr(const char *filename, MemoryRegion *mr);
  * load_image_gzipped_buffer() will read. It prevents
  * g_malloc() in those functions from allocating a huge amount of memory.
  */
-#define LOAD_IMAGE_MAX_GUNZIP_BYTES (256 << 20)
+#define LOAD_IMAGE_MAX_DECOMPRESSED_BYTES (256 << 20)
 
 ssize_t load_image_gzipped_buffer(const char *filename, uint64_t max_sz,
                                   uint8_t **buffer);
-- 
2.51.0

Re: [PATCH] Add support for zboot images compressed with zstd

[Peter Maydell]

On Wed, 8 Oct 2025 at 20:17, Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
>
> Signed-off-by: Daan De Meyer <daan.j.demeyer@gmail.com>
> ---
>  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.

> +    }
> +
>      if (bytes < 0) {
>          fprintf(stderr, "failed to decompress EFI zboot image\n");
>          g_free(data);

Otherwise I think this looks OK.

thanks
-- PMM

Re: [PATCH] Add support for zboot images compressed with zstd

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 8 Oct 2025 at 20:17, Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
>>
>> Signed-off-by: Daan De Meyer <daan.j.demeyer@gmail.com>
>> ---
>>  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

Re: [PATCH] Add support for zboot images compressed with zstd

[Peter Maydell]

On Fri, 10 Oct 2025 at 12:53, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Wed, 8 Oct 2025 at 20:17, Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
> >> +    } 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?

You could, if you wanted to, convert the file to g_autofree, yes.
You would need to also adjust the tail end of the function
in the success path to do
  *buffer = g_realloc(g_steal_pointer(&data), bytes);
so we don't double-free (and drop the cleanup in other
error exit paths).

I don't mind either way, but if we want to convert to g_autofree
we should do it as a preliminary patch.

-- PMM

Re: [PATCH] Add support for zboot images compressed with zstd

[Peter Maydell]

On Fri, 10 Oct 2025 at 13:09, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 10 Oct 2025 at 12:53, Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> > > On Wed, 8 Oct 2025 at 20:17, Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
> > >> +    } 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?
>
> You could, if you wanted to, convert the file to g_autofree, yes.

Whoops -- I meant "convert the function" here; I wasn't
trying to suggest doing the whole file (which is probably
a bad idea).

-- PMM