On Wed, Nov 29, 2017 at 12:44:32AM +0000, Nick Terrell wrote:
> >> It looks like XZ had the same issue, and they make the decompression
> >> context a static object (grep for GRUB_EMBED_DECOMPRESSOR). We could
> >> potentially do the same and statically allocate the workspace:
> >>
> >> ```
> >> /* Could also be size_t */
> >> #define BTRFS_ZSTD_WORKSPACE_SIZE_U64 (155984 / sizeof(uint64_t))
> >> static uint64_t workspace[BTRFS_ZSTD_WORKSPACE_SIZE_U64];
> >>
> >> /* ... */
> >>
> >> assert(sizeof(workspace) >= ZSTD_DCtxWorkspaceBound());
> >> ```
> >
> > Interesting, thanks for the tip, I'll try it next.
And it worked.
> > I've meanwhile tried to tweak the numbers, the maximum block for zstd,
> > that squeezed the DCtx somewhere under 48k, with block size 8k. Still
> > enomem.
>
> Sadly the block size has to stay 128 KiB, since that is the block size that
> is used for compression.
>
> > I've tried to add some debugging prints to see what numbers get actually
> > passed to the allocator, but did not see anything printed. I'm sure
> > there is a more intelligent way to test the grub changes. So far each
> > test loop takes quite some time, as I build the rpm package, test it in
> > a VM and have to recreate the environmet each time.
>
> Is the code in github.com/kdave/grub in the btrfs-zstd branch up to date?
> btrfs.c:1230 looks like it will error for zstd compression, but not with
> ENOMEM. btrfs.c:1272 [2] looks like a typo. Maybe the second is causing
> the compressed block to be interpreted as uncompressed, and causes a
> too-large allocation?
I had a different branch with patches from openSUSE, so the diffs apply with
minimal efforts to the package. The branch btrfs-zstd has been synced up. The
ENOMEM error was not from the file decompression but from the zstdio.c module,
that does something different, now disabled.
I got a bit further, with a few debugging messages, this check fails:
957 total_size = grub_le_to_cpu32 (grub_get_unaligned32 (ibuf));
958 ibuf += sizeof (total_size);
959
960 if (isize < total_size) {
961 grub_error (GRUB_ERR_BAD_FS, "ASSERTION: isize < total_size: %ld < %ld",
962 isize, total_size);
963 return -1;
964 }
with: "isize < total_size: 61440 < -47205080"
total_size is u32, but wrngly printed as signed long so it's negative, but
would be wrong anyway. This looks like the compressed format is not read
correctly.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
