All of lore.kernel.org
 help / color / mirror / Atom feed
* Using memdisk with grub2 and a gzip-compressed ISO
@ 2015-04-23 14:23 David Shaw
  2015-04-23 16:59 ` Andrei Borzenkov
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: David Shaw @ 2015-04-23 14:23 UTC (permalink / raw)
  To: grub-devel, syslinux; +Cc: Andrei Borzenkov

Hello,

Pardon the cross-post to two lists, but this problem seems to lie in a somewhat gray area between grub2 and memdisk.  Basically, the problem is this: it used to be possible to boot a compressed ISO image via memdisk and grub legacy, but this no longer works with grub2.

Here's the setup: Two machines, both with 4GiB of RAM.  The only difference between the two is that one is using grub legacy (0.97) and the other is using grub2 (2.02).  I am using memdisk from syslinux 4.02 (though for completeness I also tried 4.05 and 6.03, with the same results).  The ISO image itself is 3388047 bytes gzip compressed and 9394176 bytes uncompressed.

The grub legacy configuration:

   title boot ISO image
           kernel /memdisk iso
           initrd /my-image.iso.gz

The grub2 configuration:

   menuentry 'boot ISO image' {
     linux16 /memdisk iso
     initrd16 /my-image.iso.gz
   }

If anyone would like to see it, the sample gzip-compressed ISO image is\x01 at http://www.jabberwocky.com/my-image.iso.gz

Now to the problem: When booting the box with grub 0.97, it works.  The ISO boots and the right things happen.  Here's the output from memdisk:

  Ramdisk at 0x37cb4000, length 0x0033b28f
  gzip image: decompressed addr 0xbf5fa800, len 0x008f5800: ok

When booting the box with grub2 2.02, it does not work.  The error is:

  Ramdisk at 0x37979000, length 0x0033b290
  gzip image: decompressed addr 0xbfff7000, len 0x00008f58: failed
  Decompression error: output buffer overrun

I'm not sure if this is related to the problem, but note the length in the "Ramdisk" line from grub legacy is one byte shorter than the length from grub2.  Also note that the length given in the "gzip image" line is shifted one byte to the left in the grub legacy version (i.e. it's exactly 0x100 times larger).

1) I have tried this with memdisk from syslinux 4.02, 4.05, and 6.03 with the same results each time.
2) Changing the level of compression (i.e. gzip -1 instead of gzip -9) does not make a difference.
3) This works fine with an uncompressed image with grub2.  This also works fine with a zip-compressed image with grub2.  It only seems to fail with a gzip-compressed image with grub2.

Andrei Borzenkov kindly analyzed the image and suggested I contact both the syslinux and grub groups as he has a notion of what went wrong.  Andrei, can you fill in anything I missed?

Thanks,

David



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-23 14:23 Using memdisk with grub2 and a gzip-compressed ISO David Shaw
@ 2015-04-23 16:59 ` Andrei Borzenkov
  2015-04-23 22:17 ` [syslinux] " H. Peter Anvin
  2015-04-29 17:45 ` Andrei Borzenkov
  2 siblings, 0 replies; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-23 16:59 UTC (permalink / raw)
  To: David Shaw; +Cc: grub-devel, syslinux

В Thu, 23 Apr 2015 10:23:09 -0400
David Shaw <dshaw@jabberwocky.com> пишет:

> Hello,
> 
> Pardon the cross-post to two lists, but this problem seems to lie in a somewhat gray area between grub2 and memdisk.  Basically, the problem is this: it used to be possible to boot a compressed ISO image via memdisk and grub legacy, but this no longer works with grub2.
> 
> Here's the setup: Two machines, both with 4GiB of RAM.  The only difference between the two is that one is using grub legacy (0.97) and the other is using grub2 (2.02).  I am using memdisk from syslinux 4.02 (though for completeness I also tried 4.05 and 6.03, with the same results).  The ISO image itself is 3388047 bytes gzip compressed and 9394176 bytes uncompressed.
> 
> The grub legacy configuration:
> 
>    title boot ISO image
>            kernel /memdisk iso
>            initrd /my-image.iso.gz
> 
> The grub2 configuration:
> 
>    menuentry 'boot ISO image' {
>      linux16 /memdisk iso
>      initrd16 /my-image.iso.gz
>    }
> 
> If anyone would like to see it, the sample gzip-compressed ISO image is\x01 at http://www.jabberwocky.com/my-image.iso.gz
> 
> Now to the problem: When booting the box with grub 0.97, it works.  The ISO boots and the right things happen.  Here's the output from memdisk:
> 
>   Ramdisk at 0x37cb4000, length 0x0033b28f
>   gzip image: decompressed addr 0xbf5fa800, len 0x008f5800: ok
> 
> When booting the box with grub2 2.02, it does not work.  The error is:
> 
>   Ramdisk at 0x37979000, length 0x0033b290
>   gzip image: decompressed addr 0xbfff7000, len 0x00008f58: failed
>   Decompression error: output buffer overrun
> 
> I'm not sure if this is related to the problem, but note the length in the "Ramdisk" line from grub legacy is one byte shorter than the length from grub2.  Also note that the length given in the "gzip image" line is shifted one byte to the left in the grub legacy version (i.e. it's exactly 0x100 times larger).
> 
> 1) I have tried this with memdisk from syslinux 4.02, 4.05, and 6.03 with the same results each time.
> 2) Changing the level of compression (i.e. gzip -1 instead of gzip -9) does not make a difference.
> 3) This works fine with an uncompressed image with grub2.  This also works fine with a zip-compressed image with grub2.  It only seems to fail with a gzip-compressed image with grub2.
> 
> Andrei Borzenkov kindly analyzed the image and suggested I contact both the syslinux and grub groups as he has a notion of what went wrong.  Andrei, can you fill in anything I missed?
> 

What happens is as following. grub initrd command supports both legacy
initrd and initramfs. Initramfs is concatenation of (compressed) cpio
archives. For uncompressed cpio archive start of archive and end of
trailer are required to be aligned on 4 bytes boundary according to
Documentation/early-userspace/buffer-format.txt. Kernel actually checks
at least start alignment.

Kernel is pretty liberal in what it gets. It will ignore any amount of
zero padding between individual archives (and at the beginning as well)
and treats initrd size only as upper boundary - it will decompress
stream starting from the beginning, not assuming anything about total
compressed stream size. This allows grub to simply pad files on 4 bytes
from both ends without actually knowing anything about content.

Here where the problem happens. When loaded by grub initrd gets extra
zero byte padding, thus shifting total size by one byte, because
syslinux apparently assumes initrd size is always exact and interprets
last 4 bytes as size.

I'd actually expect that if syslinux is using Linux compatible
protocol to load initrd, it accepts at least the same data as Linux
would. May be it could relax requirement for exact size for initrd?
Especially as it apparently happens only with some compression
algorithms.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-23 14:23 Using memdisk with grub2 and a gzip-compressed ISO David Shaw
  2015-04-23 16:59 ` Andrei Borzenkov
@ 2015-04-23 22:17 ` H. Peter Anvin
  2015-04-24  3:14   ` Andrei Borzenkov
  2015-04-29 17:45 ` Andrei Borzenkov
  2 siblings, 1 reply; 16+ messages in thread
From: H. Peter Anvin @ 2015-04-23 22:17 UTC (permalink / raw)
  To: David Shaw, grub-devel, syslinux; +Cc: Andrei Borzenkov

On 04/23/2015 07:23 AM, David Shaw via Syslinux wrote:
> 
> I'm not sure if this is related to the problem, but note the length
> in the "Ramdisk" line from grub legacy is one byte shorter than the
> length from grub2.  Also note that the length given in the "gzip
> image" line is shifted one byte to the left in the grub legacy
> version (i.e. it's exactly 0x100 times larger).
> 

That would definitely break using a compressed ramdisk.  And yes, this
is why the length is wrong.

This would be a Grub2 bug.

	-hpa




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-23 22:17 ` [syslinux] " H. Peter Anvin
@ 2015-04-24  3:14   ` Andrei Borzenkov
  2015-04-24  3:30     ` H. Peter Anvin
  0 siblings, 1 reply; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-24  3:14 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: grub-devel, David Shaw, syslinux

В Thu, 23 Apr 2015 15:17:02 -0700
"H. Peter Anvin" <hpa@zytor.com> пишет:

> On 04/23/2015 07:23 AM, David Shaw via Syslinux wrote:
> > 
> > I'm not sure if this is related to the problem, but note the length
> > in the "Ramdisk" line from grub legacy is one byte shorter than the
> > length from grub2.  Also note that the length given in the "gzip
> > image" line is shifted one byte to the left in the grub legacy
> > version (i.e. it's exactly 0x100 times larger).
> > 
> 
> That would definitely break using a compressed ramdisk.

It does not break for kernel. What makes is so different in case of
syslinux?

>                                                        And yes, this
> is why the length is wrong.
> 
> This would be a Grub2 bug.
> 
> 	-hpa
> 
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-24  3:14   ` Andrei Borzenkov
@ 2015-04-24  3:30     ` H. Peter Anvin
  2015-04-24  3:41       ` Andrei Borzenkov
  0 siblings, 1 reply; 16+ messages in thread
From: H. Peter Anvin @ 2015-04-24  3:30 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: grub-devel, David Shaw, syslinux

On 04/23/2015 08:14 PM, Andrei Borzenkov wrote:
> В Thu, 23 Apr 2015 15:17:02 -0700
> "H. Peter Anvin" <hpa@zytor.com> пишет:
> 
>> On 04/23/2015 07:23 AM, David Shaw via Syslinux wrote:
>>>
>>> I'm not sure if this is related to the problem, but note the length
>>> in the "Ramdisk" line from grub legacy is one byte shorter than the
>>> length from grub2.  Also note that the length given in the "gzip
>>> image" line is shifted one byte to the left in the grub legacy
>>> version (i.e. it's exactly 0x100 times larger).
>>>
>>
>> That would definitely break using a compressed ramdisk.
> 
> It does not break for kernel. What makes is so different in case of
> syslinux?
> 

Padding the image doesn't matter for the kernel, but memdisk relies on
being able to see where the end of the compressed image is, because that
is where the length is located.

	-hpa




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-24  3:30     ` H. Peter Anvin
@ 2015-04-24  3:41       ` Andrei Borzenkov
  2015-04-24  4:39         ` H. Peter Anvin
  0 siblings, 1 reply; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-24  3:41 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: grub-devel, David Shaw, syslinux

В Thu, 23 Apr 2015 20:30:09 -0700
"H. Peter Anvin" <hpa@zytor.com> пишет:

> On 04/23/2015 08:14 PM, Andrei Borzenkov wrote:
> > В Thu, 23 Apr 2015 15:17:02 -0700
> > "H. Peter Anvin" <hpa@zytor.com> пишет:
> > 
> >> On 04/23/2015 07:23 AM, David Shaw via Syslinux wrote:
> >>>
> >>> I'm not sure if this is related to the problem, but note the length
> >>> in the "Ramdisk" line from grub legacy is one byte shorter than the
> >>> length from grub2.  Also note that the length given in the "gzip
> >>> image" line is shifted one byte to the left in the grub legacy
> >>> version (i.e. it's exactly 0x100 times larger).
> >>>
> >>
> >> That would definitely break using a compressed ramdisk.
> > 
> > It does not break for kernel. What makes is so different in case of
> > syslinux?
> > 
> 
> Padding the image doesn't matter for the kernel, but memdisk relies on
> being able to see where the end of the compressed image is, because that
> is where the length is located.

CRC+length (in case of gzip) are located immediately after compressed
stream. After stream is decompressed you get location where it ends and
automatically where length is located.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-24  3:41       ` Andrei Borzenkov
@ 2015-04-24  4:39         ` H. Peter Anvin
  2015-04-24  6:25           ` Andrei Borzenkov
  0 siblings, 1 reply; 16+ messages in thread
From: H. Peter Anvin @ 2015-04-24  4:39 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: grub-devel, David Shaw, syslinux

On 04/23/2015 08:41 PM, Andrei Borzenkov wrote:
> 
> CRC+length (in case of gzip) are located immediately after compressed
> stream. After stream is decompressed you get location where it ends and
> automatically where length is located.
> 

I know.  One could decompress it twice, but why?

	-hpa



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-24  4:39         ` H. Peter Anvin
@ 2015-04-24  6:25           ` Andrei Borzenkov
  2015-04-27 22:20             ` H. Peter Anvin
  0 siblings, 1 reply; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-24  6:25 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: The development of GNU GRUB, David Shaw, syslinux

On Fri, Apr 24, 2015 at 7:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 04/23/2015 08:41 PM, Andrei Borzenkov wrote:
>>
>> CRC+length (in case of gzip) are located immediately after compressed
>> stream. After stream is decompressed you get location where it ends and
>> automatically where length is located.
>>
>
> I know.  One could decompress it twice,

Cannot you incrementally reallocate uncompressed buffer?

> but why?
>
>         -hpa
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-24  6:25           ` Andrei Borzenkov
@ 2015-04-27 22:20             ` H. Peter Anvin
  2015-04-29 13:55               ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 16+ messages in thread
From: H. Peter Anvin @ 2015-04-27 22:20 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: The development of GNU GRUB, syslinux

On 04/23/2015 11:25 PM, Andrei Borzenkov via Syslinux wrote:
> On Fri, Apr 24, 2015 at 7:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 04/23/2015 08:41 PM, Andrei Borzenkov wrote:
>>>
>>> CRC+length (in case of gzip) are located immediately after compressed
>>> stream. After stream is decompressed you get location where it ends and
>>> automatically where length is located.
>>>
>>
>> I know.  One could decompress it twice,
> 
> Cannot you incrementally reallocate uncompressed buffer?
> 

That is very hard because of how memory is managed in MEMDISK.

	-hpa




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-27 22:20             ` H. Peter Anvin
@ 2015-04-29 13:55               ` Vladimir 'φ-coder/phcoder' Serbinenko
  2015-04-29 16:28                 ` Andrei Borzenkov
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-04-29 13:55 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On 28.04.2015 00:20, H. Peter Anvin wrote:
> On 04/23/2015 11:25 PM, Andrei Borzenkov via Syslinux wrote:
>> On Fri, Apr 24, 2015 at 7:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 04/23/2015 08:41 PM, Andrei Borzenkov wrote:
>>>>
>>>> CRC+length (in case of gzip) are located immediately after compressed
>>>> stream. After stream is decompressed you get location where it ends and
>>>> automatically where length is located.
>>>>
>>>
>>> I know.  One could decompress it twice,
>>
>> Cannot you incrementally reallocate uncompressed buffer?
>>
> 
> That is very hard because of how memory is managed in MEMDISK.
> 
It's fine to skip padding if only one file is supplied on command line.
What about:
diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index 117232f..a63a11a 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -205,7 +205,8 @@ grub_initrd_init (int argc, char *argv[],
       initrd_ctx->nfiles++;
       initrd_ctx->components[i].size
        = grub_file_size (initrd_ctx->components[i].file);
-      initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
+      if (argc != 1)
+       initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
     }

   if (newc)



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [syslinux] Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-29 13:55               ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2015-04-29 16:28                 ` Andrei Borzenkov
  0 siblings, 0 replies; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-29 16:28 UTC (permalink / raw)
  To: Vladimir 'φ-coder/phcoder' Serbinenko
  Cc: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

В Wed, 29 Apr 2015 15:55:48 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 28.04.2015 00:20, H. Peter Anvin wrote:
> > On 04/23/2015 11:25 PM, Andrei Borzenkov via Syslinux wrote:
> >> On Fri, Apr 24, 2015 at 7:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> >>> On 04/23/2015 08:41 PM, Andrei Borzenkov wrote:
> >>>>
> >>>> CRC+length (in case of gzip) are located immediately after compressed
> >>>> stream. After stream is decompressed you get location where it ends and
> >>>> automatically where length is located.
> >>>>
> >>>
> >>> I know.  One could decompress it twice,
> >>
> >> Cannot you incrementally reallocate uncompressed buffer?
> >>
> > 
> > That is very hard because of how memory is managed in MEMDISK.
> > 
> It's fine to skip padding if only one file is supplied on command line.

The only reason I can think of to use trailing padding at all is this
line in Documentation/early-userspace/buffer-format.txt:

        cpio_trailer := ALGN(4) + cpio_header + "TRAILER!!!\0" + ALGN(4)

which mandates final alignment. Because grub does not really know
whether file is initrd or initramfs and whether content is compressed
or uncompressed cpio, it is much easier to simply pad everything. If
we accept this as valid reason, number of files does not really matter.

In practice kernel never required trailing alignment, at least since
earliest git version in 2005. So I guess we can remove it. 

> What about:
> diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
> index 117232f..a63a11a 100644
> --- a/grub-core/loader/linux.c
> +++ b/grub-core/loader/linux.c
> @@ -205,7 +205,8 @@ grub_initrd_init (int argc, char *argv[],
>        initrd_ctx->nfiles++;
>        initrd_ctx->components[i].size
>         = grub_file_size (initrd_ctx->components[i].file);
> -      initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
> +      if (argc != 1)
> +       initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
>      }
> 

This results in out-of-bound access in grub_initrd_load. The right fix
is to pad before next file instead which automatically gives correct
length in case of single file. I'll commit a patch.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-23 14:23 Using memdisk with grub2 and a gzip-compressed ISO David Shaw
  2015-04-23 16:59 ` Andrei Borzenkov
  2015-04-23 22:17 ` [syslinux] " H. Peter Anvin
@ 2015-04-29 17:45 ` Andrei Borzenkov
  2015-04-29 18:35   ` Vladimir 'phcoder' Serbinenko
                     ` (2 more replies)
  2 siblings, 3 replies; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-29 17:45 UTC (permalink / raw)
  To: David Shaw; +Cc: grub-devel, syslinux

В Thu, 23 Apr 2015 10:23:09 -0400
David Shaw <dshaw@jabberwocky.com> пишет:

> 
> When booting the box with grub2 2.02, it does not work.  The error is:
> 
>   Ramdisk at 0x37979000, length 0x0033b290
>   gzip image: decompressed addr 0xbfff7000, len 0x00008f58: failed
>   Decompression error: output buffer overrun
> 

Could you test attached patch? While I'm past this message using your
archive, in qemu-kvm it hangs after "... booting ..." and in
qemu-system-i386 I get something that looks like memory corruption with
occasional crash when initrd is uncompressed.

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] loader/linux: do not pad initrd with zeroes at the end

Syslinux memdisk is using initrd image and needs to know uncompressed
size in advance. For gzip uncompressed size is at the end of compressed
stream. Grub padded each input file to 4 bytes at the end, which means
syslinux got wrong size.

Linux initramfs loader apparently does not care about trailing alignment.
So change code to align beginning of each file instead which
automatically gives us the correct size for single file.

Reported-By: David Shaw <dshaw@jabberwocky.com>

---
 grub-core/loader/linux.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
index 117232f..35b858e 100644
--- a/grub-core/loader/linux.c
+++ b/grub-core/loader/linux.c
@@ -161,6 +161,9 @@ grub_initrd_init (int argc, char *argv[],
   for (i = 0; i < argc; i++)
     {
       const char *fname = argv[i];
+
+      initrd_ctx->size = ALIGN_UP (initrd_ctx->size, 4);
+
       if (grub_memcmp (argv[i], "newc:", 5) == 0)
 	{
 	  const char *ptr, *eptr;
@@ -205,7 +208,7 @@ grub_initrd_init (int argc, char *argv[],
       initrd_ctx->nfiles++;
       initrd_ctx->components[i].size
 	= grub_file_size (initrd_ctx->components[i].file);
-      initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
+      initrd_ctx->size += initrd_ctx->components[i].size;
     }
 
   if (newc)
@@ -253,6 +256,9 @@ grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
     {
       grub_ssize_t cursize;
 
+      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4));
+      ptr += ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4);
+
       if (initrd_ctx->components[i].newc_name)
 	{
 	  ptr += insert_dir (initrd_ctx->components[i].newc_name,
@@ -283,8 +289,6 @@ grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
 	  return grub_errno;
 	}
       ptr += cursize;
-      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD (cursize, 4));
-      ptr += ALIGN_UP_OVERHEAD (cursize, 4);
     }
   if (newc)
     ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!") - 1, 0, 0);
-- 
tg: (104dff3..) u/initrd-pad (depends on: master)


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-29 17:45 ` Andrei Borzenkov
@ 2015-04-29 18:35   ` Vladimir 'phcoder' Serbinenko
  2015-04-30  3:36     ` Andrei Borzenkov
  2015-04-29 18:42   ` David Shaw
  2015-05-07 14:23   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 1 reply; 16+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2015-04-29 18:35 UTC (permalink / raw)
  To: The development of GRUB 2

[-- Attachment #1: Type: text/plain, Size: 3515 bytes --]

On Apr 29, 2015 7:45 PM, "Andrei Borzenkov" <arvidjaar@gmail.com> wrote:
>
> В Thu, 23 Apr 2015 10:23:09 -0400
> David Shaw <dshaw@jabberwocky.com> пишет:
>
> >
> > When booting the box with grub2 2.02, it does not work.  The error is:
> >
> >   Ramdisk at 0x37979000, length 0x0033b290
> >   gzip image: decompressed addr 0xbfff7000, len 0x00008f58: failed
> >   Decompression error: output buffer overrun
> >
>
> Could you test attached patch? While I'm past this message using your
> archive, in qemu-kvm it hangs after "... booting ..." and in
> qemu-system-i386 I get something that looks like memory corruption with
> occasional crash when initrd is uncompressed.
>
> From: Andrei Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] loader/linux: do not pad initrd with zeroes at the end
>
> Syslinux memdisk is using initrd image and needs to know uncompressed
> size in advance. For gzip uncompressed size is at the end of compressed
> stream. Grub padded each input file to 4 bytes at the end, which means
> syslinux got wrong size.
>
> Linux initramfs loader apparently does not care about trailing alignment.
> So change code to align beginning of each file instead which
> automatically gives us the correct size for single file.
>
> Reported-By: David Shaw <dshaw@jabberwocky.com>
>
> ---
>  grub-core/loader/linux.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
> index 117232f..35b858e 100644
> --- a/grub-core/loader/linux.c
> +++ b/grub-core/loader/linux.c
> @@ -161,6 +161,9 @@ grub_initrd_init (int argc, char *argv[],
>    for (i = 0; i < argc; i++)
>      {
>        const char *fname = argv[i];
> +
> +      initrd_ctx->size = ALIGN_UP (initrd_ctx->size, 4);
> +
>        if (grub_memcmp (argv[i], "newc:", 5) == 0)
>         {
>           const char *ptr, *eptr;
> @@ -205,7 +208,7 @@ grub_initrd_init (int argc, char *argv[],
>        initrd_ctx->nfiles++;
>        initrd_ctx->components[i].size
>         = grub_file_size (initrd_ctx->components[i].file);
> -      initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
> +      initrd_ctx->size += initrd_ctx->components[i].size;
>      }
>
>    if (newc)
> @@ -253,6 +256,9 @@ grub_initrd_load (struct grub_linux_initrd_context
*initrd_ctx,
>      {
>        grub_ssize_t cursize;
>
> +      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4));
> +      ptr += ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4);
> +
Please don't abuse the pointer this way.
Also thinking about it, I think that the allocation for initrd should be
page-aligned and have integer number of pages if it's not the case already.
I'll check when I get home
>        if (initrd_ctx->components[i].newc_name)
>         {
>           ptr += insert_dir (initrd_ctx->components[i].newc_name,
> @@ -283,8 +289,6 @@ grub_initrd_load (struct grub_linux_initrd_context
*initrd_ctx,
>           return grub_errno;
>         }
>        ptr += cursize;
> -      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD (cursize, 4));
> -      ptr += ALIGN_UP_OVERHEAD (cursize, 4);
>      }
>    if (newc)
>      ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!") - 1, 0,
0);
> --
> tg: (104dff3..) u/initrd-pad (depends on: master)
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #2: Type: text/html, Size: 4664 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-29 17:45 ` Andrei Borzenkov
  2015-04-29 18:35   ` Vladimir 'phcoder' Serbinenko
@ 2015-04-29 18:42   ` David Shaw
  2015-05-07 14:23   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 0 replies; 16+ messages in thread
From: David Shaw @ 2015-04-29 18:42 UTC (permalink / raw)
  To: Andrei Borzenkov; +Cc: grub-devel, syslinux

On Apr 29, 2015, at 1:45 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> 
> В Thu, 23 Apr 2015 10:23:09 -0400
> David Shaw <dshaw@jabberwocky.com> пишет:
> 
>> 
>> When booting the box with grub2 2.02, it does not work.  The error is:
>> 
>>  Ramdisk at 0x37979000, length 0x0033b290
>>  gzip image: decompressed addr 0xbfff7000, len 0x00008f58: failed
>>  Decompression error: output buffer overrun
>> 
> 
> Could you test attached patch? While I'm past this message using your
> archive, in qemu-kvm it hangs after "... booting ..." and in
> qemu-system-i386 I get something that looks like memory corruption with
> occasional crash when initrd is uncompressed.

Thanks for the patch - this boots successfully for me.

David



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-29 18:35   ` Vladimir 'phcoder' Serbinenko
@ 2015-04-30  3:36     ` Andrei Borzenkov
  0 siblings, 0 replies; 16+ messages in thread
From: Andrei Borzenkov @ 2015-04-30  3:36 UTC (permalink / raw)
  To: Vladimir 'phcoder' Serbinenko; +Cc: The development of GRUB 2

В Wed, 29 Apr 2015 20:35:10 +0200
"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> пишет:

> > @@ -253,6 +256,9 @@ grub_initrd_load (struct grub_linux_initrd_context
> *initrd_ctx,
> >      {
> >        grub_ssize_t cursize;
> >
> > +      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4));
> > +      ptr += ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4);
> > +
> Please don't abuse the pointer this way.

Is it better?

grub_memset (ptr, 0, ALIGN_UP_OVERHEAD (ptr - (grub_uint8_t *)target, 4));
ptr += ALIGN_UP_OVERHEAD (ptr - (grub_uint8_t *)target, 4);


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: Using memdisk with grub2 and a gzip-compressed ISO
  2015-04-29 17:45 ` Andrei Borzenkov
  2015-04-29 18:35   ` Vladimir 'phcoder' Serbinenko
  2015-04-29 18:42   ` David Shaw
@ 2015-05-07 14:23   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2 siblings, 0 replies; 16+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2015-05-07 14:23 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 3026 bytes --]

Go ahead
On 29.04.2015 19:45, Andrei Borzenkov wrote:
> В Thu, 23 Apr 2015 10:23:09 -0400
> David Shaw <dshaw@jabberwocky.com> пишет:
> 
>>
>> When booting the box with grub2 2.02, it does not work.  The error is:
>>
>>   Ramdisk at 0x37979000, length 0x0033b290
>>   gzip image: decompressed addr 0xbfff7000, len 0x00008f58: failed
>>   Decompression error: output buffer overrun
>>
> 
> Could you test attached patch? While I'm past this message using your
> archive, in qemu-kvm it hangs after "... booting ..." and in
> qemu-system-i386 I get something that looks like memory corruption with
> occasional crash when initrd is uncompressed.
> 
> From: Andrei Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] loader/linux: do not pad initrd with zeroes at the end
> 
> Syslinux memdisk is using initrd image and needs to know uncompressed
> size in advance. For gzip uncompressed size is at the end of compressed
> stream. Grub padded each input file to 4 bytes at the end, which means
> syslinux got wrong size.
> 
> Linux initramfs loader apparently does not care about trailing alignment.
> So change code to align beginning of each file instead which
> automatically gives us the correct size for single file.
> 
> Reported-By: David Shaw <dshaw@jabberwocky.com>
> 
> ---
>  grub-core/loader/linux.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/loader/linux.c b/grub-core/loader/linux.c
> index 117232f..35b858e 100644
> --- a/grub-core/loader/linux.c
> +++ b/grub-core/loader/linux.c
> @@ -161,6 +161,9 @@ grub_initrd_init (int argc, char *argv[],
>    for (i = 0; i < argc; i++)
>      {
>        const char *fname = argv[i];
> +
> +      initrd_ctx->size = ALIGN_UP (initrd_ctx->size, 4);
> +
>        if (grub_memcmp (argv[i], "newc:", 5) == 0)
>  	{
>  	  const char *ptr, *eptr;
> @@ -205,7 +208,7 @@ grub_initrd_init (int argc, char *argv[],
>        initrd_ctx->nfiles++;
>        initrd_ctx->components[i].size
>  	= grub_file_size (initrd_ctx->components[i].file);
> -      initrd_ctx->size += ALIGN_UP (initrd_ctx->components[i].size, 4);
> +      initrd_ctx->size += initrd_ctx->components[i].size;
>      }
>  
>    if (newc)
> @@ -253,6 +256,9 @@ grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
>      {
>        grub_ssize_t cursize;
>  
> +      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4));
> +      ptr += ALIGN_UP_OVERHEAD ((grub_addr_t)ptr, 4);
> +
>        if (initrd_ctx->components[i].newc_name)
>  	{
>  	  ptr += insert_dir (initrd_ctx->components[i].newc_name,
> @@ -283,8 +289,6 @@ grub_initrd_load (struct grub_linux_initrd_context *initrd_ctx,
>  	  return grub_errno;
>  	}
>        ptr += cursize;
> -      grub_memset (ptr, 0, ALIGN_UP_OVERHEAD (cursize, 4));
> -      ptr += ALIGN_UP_OVERHEAD (cursize, 4);
>      }
>    if (newc)
>      ptr = make_header (ptr, "TRAILER!!!", sizeof ("TRAILER!!!") - 1, 0, 0);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-05-07 14:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-23 14:23 Using memdisk with grub2 and a gzip-compressed ISO David Shaw
2015-04-23 16:59 ` Andrei Borzenkov
2015-04-23 22:17 ` [syslinux] " H. Peter Anvin
2015-04-24  3:14   ` Andrei Borzenkov
2015-04-24  3:30     ` H. Peter Anvin
2015-04-24  3:41       ` Andrei Borzenkov
2015-04-24  4:39         ` H. Peter Anvin
2015-04-24  6:25           ` Andrei Borzenkov
2015-04-27 22:20             ` H. Peter Anvin
2015-04-29 13:55               ` Vladimir 'φ-coder/phcoder' Serbinenko
2015-04-29 16:28                 ` Andrei Borzenkov
2015-04-29 17:45 ` Andrei Borzenkov
2015-04-29 18:35   ` Vladimir 'phcoder' Serbinenko
2015-04-30  3:36     ` Andrei Borzenkov
2015-04-29 18:42   ` David Shaw
2015-05-07 14:23   ` Vladimir 'φ-coder/phcoder' Serbinenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.