On Tue, Oct 14, 2025 at 02:14:18PM -0600, Logan Gunthorpe via Grub-devel wrote:
> Add zstd based io decompression.
>
> Based largely on the existing xzio, implement the same features using
> the zstd library already included in the project.
>
> Signed-off-by: Logan Gunthorpe <[email protected]>
> ---
>  Makefile.util.def           |   1 +
>  grub-core/Makefile.core.def |   7 ++
>  grub-core/io/zstdio.c       | 237 ++++++++++++++++++++++++++++++++++++
>  include/grub/file.h         |   3 +-
>  4 files changed, 247 insertions(+), 1 deletion(-)
>  create mode 100644 grub-core/io/zstdio.c
>
> diff --git a/Makefile.util.def b/Makefile.util.def
> index 038253b37a42..74786177f908 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -162,6 +162,7 @@ library = {
>    common = grub-core/io/gzio.c;
>    common = grub-core/io/xzio.c;
>    common = grub-core/io/lzopio.c;
> +  common = grub-core/io/zstdio.c;
>    common = grub-core/kern/ia64/dl_helper.c;
>    common = grub-core/kern/arm/dl_helper.c;
>    common = grub-core/kern/arm64/dl_helper.c;
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index a729cb6a8e61..a2635d61b6e8 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2512,6 +2512,13 @@ module = {
>    cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/minilzo 
> -DMINILZO_HAVE_CONFIG_H';
>  };
>
> +module = {
> +  name = zstdio;
> +  common = io/zstdio.c;
> +  cppflags = '-I$(srcdir)/lib/posix_wrap -I$(srcdir)/lib/zstd';
> +  cflags='-Wno-unreachable-code';
> +};
> +
>  module = {
>    name = testload;
>    common = commands/testload.c;
> diff --git a/grub-core/io/zstdio.c b/grub-core/io/zstdio.c
> new file mode 100644
> index 000000000000..01a5a45587a7
> --- /dev/null
> +++ b/grub-core/io/zstdio.c
> @@ -0,0 +1,237 @@
> +/* zstdio.c - decompression support for zstd */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2010  Free Software Foundation, Inc.

s/2010/2025/

> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/err.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/file.h>
> +#include <grub/fs.h>
> +#include <grub/dl.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#include "zstd.h"
> +
> +#define STREAM_HEADER_SIZE 16
> +
> +struct grub_zstdio

You do not need grub_ prefix if name is internal to the module.

> +{
> +  grub_file_t file;
> +  ZSTD_DCtx *dctx;
> +  grub_size_t insize;
> +  grub_size_t outsize;
> +
> +  ZSTD_outBuffer output;
> +  ZSTD_inBuffer input;
> +
> +  grub_off_t saved_offset;
> +  grub_uint8_t bufs[];
> +};
> +

Please drop this empty line...

> +typedef struct grub_zstdio *grub_zstdio_t;

... and add one here... :-)

> +static struct grub_fs grub_zstdio_fs;
> +
> +static int

Please use bool, including true/false, instead of int.

> +test_header (grub_file_t file)
> +{
> +  grub_zstdio_t zstdio = file->data;
> +  size_t zret;
> +
> +  zstdio->input.pos = 0;
> +  zstdio->output.pos = 0;
> +  zstdio->output.size = zstdio->outsize;
> +  zstdio->input.size = grub_file_read (zstdio->file, zstdio->bufs,
> +                                       STREAM_HEADER_SIZE);
> +  if (zstdio->input.size != STREAM_HEADER_SIZE)
> +    return 0;
> +
> +  zret = ZSTD_decompressStream (zstdio->dctx, &zstdio->output, 
> &zstdio->input);
> +  if (ZSTD_isError (zret))
> +    return 0;
> +
> +  return 1;
> +}

Missing empty line here...

> +static grub_file_t
> +grub_zstdio_open (grub_file_t io, enum grub_file_type type)
> +{
> +  grub_file_t file;
> +  grub_zstdio_t zstdio;
> +
> +  if (type & GRUB_FILE_TYPE_NO_DECOMPRESS)
> +    return io;
> +
> +  file = (grub_file_t) grub_zalloc (sizeof (grub_file_t));
> +  if (file == NULL)
> +    return 0;

s/0/NULL/

> +  zstdio = grub_zalloc (sizeof (grub_zstdio_t) + ZSTD_DStreamInSize () +
> +                        ZSTD_DStreamOutSize ());
> +  if (zstdio == NULL)
> +    {
> +      grub_free (file);
> +      return 0;

Ditto...

In general please use NULL instead of 0.

> +    }
> +
> +  zstdio->file = io;
> +  zstdio->insize = ZSTD_DStreamInSize ();
> +  zstdio->outsize = ZSTD_DStreamOutSize ();
> +  zstdio->input.src = zstdio->bufs;
> +  zstdio->output.dst = &zstdio->bufs[zstdio->insize];
> +
> +  file->device = io->device;
> +  file->data = zstdio;
> +  file->fs = &grub_zstdio_fs;
> +  file->size = GRUB_FILE_SIZE_UNKNOWN;
> +  file->not_easily_seekable = 1;
> +
> +  if (grub_file_tell (zstdio->file) != 0)
> +    grub_file_seek (zstdio->file, 0);

grub_file_seek() may fail. Please always check for errors.

> +  zstdio->dctx = ZSTD_createDCtx ();
> +  if (zstdio->dctx == NULL)
> +    {
> +      grub_free (file);
> +      grub_free (zstdio);
> +      return 0;

Again, s/0/NULL/...

> +    }
> +
> +  if (!test_header (file))

if (test_header (file) == false)

> +    {
> +      grub_errno = GRUB_ERR_NONE;
> +      grub_file_seek (io, 0);
> +      ZSTD_freeDCtx (zstdio->dctx);
> +      grub_free (zstdio);
> +      grub_free (file);
> +
> +      return io;
> +    }
> +
> +  return file;
> +}
> +
> +static grub_ssize_t
> +grub_zstdio_read (grub_file_t file, char *buf, grub_size_t len)
> +{
> +  grub_zstdio_t zstdio = file->data;
> +  grub_ssize_t ret = 0;
> +  grub_ssize_t readret;
> +  grub_off_t current_offset;
> +  grub_size_t zret;
> +
> +  /* If seek backward need to reset decoder and start from beginning of 
> file. */
> +  if (file->offset < zstdio->saved_offset)
> +    {
> +      ZSTD_initDStream (zstdio->dctx);
> +      zstdio->input.pos = 0;
> +      zstdio->input.size = 0;
> +      zstdio->output.pos = 0;
> +      zstdio->saved_offset = 0;
> +      grub_file_seek (zstdio->file, 0);

grub_file_seek() may fail...

> +    }
> +
> +  current_offset = zstdio->saved_offset;
> +
> +  while (len > 0)
> +    {
> +      zstdio->output.size = file->offset + ret + len - current_offset;

Is there any chance for overflow here? If yes then please use safe math
from include/grub/safemath.h.

> +      if (zstdio->output.size > zstdio->outsize)
> +        zstdio->output.size = zstdio->outsize;
> +      if (zstdio->input.pos == zstdio->input.size)
> +        {
> +       readret = grub_file_read (zstdio->file, zstdio->bufs,
> +                                 zstdio->insize);
> +       if (readret < 0)
> +         return -1;
> +
> +       zstdio->input.size = readret;
> +       zstdio->input.pos = 0;
> +     }
> +
> +      zret = ZSTD_decompressStream (zstdio->dctx, &zstdio->output,
> +                                    &zstdio->input);
> +      if (ZSTD_isError (zret))
> +     {
> +       grub_error (GRUB_ERR_BAD_COMPRESSED_DATA,
> +                   N_("zstd file corrupted or unsupported block options"));
> +       return -1;
> +        }
> +
> +      grub_off_t new_offset = current_offset + zstdio->output.pos;

Please do not mix variable definitions with the code. Define all stuff
at the beginning of the function or block.

> +      /* Store first chunk of data in buffer.  */
> +      if (file->offset <= new_offset)
> +        {
> +       grub_size_t delta = new_offset - (file->offset + ret);

OK, but missing empty line here.

> +       grub_memmove (buf, (grub_uint8_t *) zstdio->output.dst +
> +                        (zstdio->output.pos - delta),

Safe math? In general please check all math in this patch and use safe
math macros where needed.

> +                        delta);
> +       len -= delta;
> +       buf += delta;
> +       ret += delta;
> +     }
> +     current_offset = new_offset;
> +
> +     zstdio->output.pos = 0;
> +
> +     if (zstdio->input.pos == 0 && zstdio->output.pos == 0)
> +       break;
> +    }
> +
> +  if (ret >= 0)
> +    zstdio->saved_offset = file->offset + ret;
> +
> +  return ret;
> +}

Daniel

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to