diff mbox series

[v2,1/8] linux/arm: fix ARM Linux header layout

Message ID 20201025134941.4805-2-ard.biesheuvel@arm.com
State New
Headers show
Series linux: implement LoadFile2 initrd loading | expand

Commit Message

Ard Biesheuvel Oct. 25, 2020, 1:49 p.m. UTC
The hdr_offset member of the ARM Linux image header appears at
offset 0x3c, matching the PE/COFF spec's placement of the COFF
header offset in the MS-DOS header. We're currently off by four,
so fix that.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 include/grub/arm/linux.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Leif Lindholm Nov. 4, 2020, 12:11 p.m. UTC | #1
On Sun, Oct 25, 2020 at 14:49:34 +0100, Ard Biesheuvel wrote:
> The hdr_offset member of the ARM Linux image header appears at
> offset 0x3c, matching the PE/COFF spec's placement of the COFF
> header offset in the MS-DOS header. We're currently off by four,
> so fix that.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  include/grub/arm/linux.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
> index 2e98a6689696..bcd5a7eb186e 100644
> --- a/include/grub/arm/linux.h
> +++ b/include/grub/arm/linux.h
> @@ -30,7 +30,7 @@ struct linux_arm_kernel_header {
>    grub_uint32_t magic;
>    grub_uint32_t start; /* _start */
>    grub_uint32_t end;   /* _edata */
> -  grub_uint32_t reserved2[4];
> +  grub_uint32_t reserved2[3];
>    grub_uint32_t hdr_offset;

How did this ever work?

/
    Leif

>  };
>  
> -- 
> 2.17.1
>
Ard Biesheuvel Nov. 4, 2020, 12:19 p.m. UTC | #2
On Wed, 4 Nov 2020 at 13:11, Leif Lindholm <leif@nuviainc.com> wrote:
>
> On Sun, Oct 25, 2020 at 14:49:34 +0100, Ard Biesheuvel wrote:
> > The hdr_offset member of the ARM Linux image header appears at
> > offset 0x3c, matching the PE/COFF spec's placement of the COFF
> > header offset in the MS-DOS header. We're currently off by four,
> > so fix that.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > ---
> >  include/grub/arm/linux.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
> > index 2e98a6689696..bcd5a7eb186e 100644
> > --- a/include/grub/arm/linux.h
> > +++ b/include/grub/arm/linux.h
> > @@ -30,7 +30,7 @@ struct linux_arm_kernel_header {
> >    grub_uint32_t magic;
> >    grub_uint32_t start; /* _start */
> >    grub_uint32_t end;   /* _edata */
> > -  grub_uint32_t reserved2[4];
> > +  grub_uint32_t reserved2[3];
> >    grub_uint32_t hdr_offset;
>
> How did this ever work?
>

By ignoring the value of hdr_offset entirely everywhere else
Leif Lindholm Nov. 4, 2020, 12:31 p.m. UTC | #3
On Wed, Nov 04, 2020 at 13:19:47 +0100, Ard Biesheuvel wrote:
> On Wed, 4 Nov 2020 at 13:11, Leif Lindholm <leif@nuviainc.com> wrote:
> >
> > On Sun, Oct 25, 2020 at 14:49:34 +0100, Ard Biesheuvel wrote:
> > > The hdr_offset member of the ARM Linux image header appears at
> > > offset 0x3c, matching the PE/COFF spec's placement of the COFF
> > > header offset in the MS-DOS header. We're currently off by four,
> > > so fix that.
> > >
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > ---
> > >  include/grub/arm/linux.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
> > > index 2e98a6689696..bcd5a7eb186e 100644
> > > --- a/include/grub/arm/linux.h
> > > +++ b/include/grub/arm/linux.h
> > > @@ -30,7 +30,7 @@ struct linux_arm_kernel_header {
> > >    grub_uint32_t magic;
> > >    grub_uint32_t start; /* _start */
> > >    grub_uint32_t end;   /* _edata */
> > > -  grub_uint32_t reserved2[4];
> > > +  grub_uint32_t reserved2[3];
> > >    grub_uint32_t hdr_offset;
> >
> > How did this ever work?
> >
> 
> By ignoring the value of hdr_offset entirely everywhere else

Oh, right - we only bother checking magic, doh!

/
    Leif
Daniel Kiper March 11, 2021, 4:18 p.m. UTC | #4
On Sun, Oct 25, 2020 at 02:49:34PM +0100, Ard Biesheuvel wrote:
> The hdr_offset member of the ARM Linux image header appears at

> offset 0x3c, matching the PE/COFF spec's placement of the COFF

> header offset in the MS-DOS header. We're currently off by four,

> so fix that.

>

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>


I was asked by Dimitri to take this patch. I double checked it. It
looks that it impacts only one grub_file_read() and one grub_dprintf().
So, there is no or minimal risk at this point and no big advantage
for upstream. Though, due to low risk, I will take it for downstream
advantage.

So, Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Until I hear any objections in an hour or so...

Daniel

> ---

>  include/grub/arm/linux.h | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h

> index 2e98a6689696..bcd5a7eb186e 100644

> --- a/include/grub/arm/linux.h

> +++ b/include/grub/arm/linux.h

> @@ -30,7 +30,7 @@ struct linux_arm_kernel_header {

>    grub_uint32_t magic;

>    grub_uint32_t start; /* _start */

>    grub_uint32_t end;   /* _edata */

> -  grub_uint32_t reserved2[4];

> +  grub_uint32_t reserved2[3];

>    grub_uint32_t hdr_offset;

>  };

>

> --

> 2.17.1
diff mbox series

Patch

diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
index 2e98a6689696..bcd5a7eb186e 100644
--- a/include/grub/arm/linux.h
+++ b/include/grub/arm/linux.h
@@ -30,7 +30,7 @@  struct linux_arm_kernel_header {
   grub_uint32_t magic;
   grub_uint32_t start; /* _start */
   grub_uint32_t end;   /* _edata */
-  grub_uint32_t reserved2[4];
+  grub_uint32_t reserved2[3];
   grub_uint32_t hdr_offset;
 };