Re: Tag-alignment in multiboot2 image headers

[Vladimir 'phcoder' Serbinenko]

On Wed, Mar 8, 2017, 22:28 Andrei Borzenkov  wrote:

> On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research,
> US)  wrote:
> > Hello,
> >
> > I'm seeing an inconsistency in the multiboot2 specification and the
> implementation of the multiboot2 loader code in GRUB. It may be my
> understanding that's incorrect. A clarification would be appreciated.
> >
> > This concerns the alignment requirements for tags in OS image headers.
> The specification states 4 bytes, but the code uses 8 bytes.
> >
> > The specification states (Section 3.1.3) that "Tags constitutes a buffer
> of structures following each other padded on 'u32' size."
> >
>
> This is ambiguous and needs better wording as well (it is not clear
> whether "padded" here applies to individual tag or all tags block).
>
> > The "for" loop for parsing tags uses the following "increment" statement
> (grub/grub_core/loader/multiboot_mbi2.c: line 148):
> > tag = (struct multiboot_header_tag *) ((grub_uint32_t *) tag +
> ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) / 4))
> >
> > The macro MULTIBOOT_TAG_ALIGN is defined in (include/multiboot2.h) as 8.
> This alignment value is consistent with the specification for tags in the
> multiboot2 information structure, but not for tags in an OS image header.
> >
>
> Yes, it sure looks wrong. Thanks for making us aware!
>
> @Vladimir, @Daniel - I think this is 2.02 material, we do not want
> release with such inconsistency. The question is what needs fixing
> though - about half of all tags are not multiple of 8 bytes, so I
> expect people to hit it in real life. What is current implementation
> in MB2 compliant kernels?
>
The intention was 8, so that we can have u64 naturally aligned in the
header even on non-x86. I believe all current kernels are compatible with
grub, so I think we should update the docs. Daniel, your opinion?

>
___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub

Re: Tag-alignment in multiboot2 image headers

[Andrei Borzenkov]

On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research,
US)  wrote:
> Hello,
>
> I'm seeing an inconsistency in the multiboot2 specification and the 
> implementation of the multiboot2 loader code in GRUB. It may be my 
> understanding that's incorrect. A clarification would be appreciated.
>
> This concerns the alignment requirements for tags in OS image headers. The 
> specification states 4 bytes, but the code uses 8 bytes.
>
> The specification states (Section 3.1.3) that "Tags constitutes a buffer of 
> structures following each other padded on 'u32' size."
>

This is ambiguous and needs better wording as well (it is not clear
whether "padded" here applies to individual tag or all tags block).

> The "for" loop for parsing tags uses the following "increment" statement 
> (grub/grub_core/loader/multiboot_mbi2.c: line 148):
> tag = (struct multiboot_header_tag *) ((grub_uint32_t *) tag + ALIGN_UP 
> (tag->size, MULTIBOOT_TAG_ALIGN) / 4))
>
> The macro MULTIBOOT_TAG_ALIGN is defined in (include/multiboot2.h) as 8. This 
> alignment value is consistent with the specification for tags in the 
> multiboot2 information structure, but not for tags in an OS image header.
>

Yes, it sure looks wrong. Thanks for making us aware!

@Vladimir, @Daniel - I think this is 2.02 material, we do not want
release with such inconsistency. The question is what needs fixing
though - about half of all tags are not multiple of 8 bytes, so I
expect people to hit it in real life. What is current implementation
in MB2 compliant kernels?

___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub

Tag-alignment in multiboot2 image headers

[Ahmed, Safayet (GE Global Research, US)]

Hello,

I'm seeing an inconsistency in the multiboot2 specification and the 
implementation of the multiboot2 loader code in GRUB. It may be my 
understanding that's incorrect. A clarification would be appreciated.

This concerns the alignment requirements for tags in OS image headers. The 
specification states 4 bytes, but the code uses 8 bytes.

The specification states (Section 3.1.3) that "Tags constitutes a buffer of 
structures following each other padded on 'u32' size."

The "for" loop for parsing tags uses the following "increment" statement 
(grub/grub_core/loader/multiboot_mbi2.c: line 148):
tag = (struct multiboot_header_tag *) ((grub_uint32_t *) tag + ALIGN_UP 
(tag->size, MULTIBOOT_TAG_ALIGN) / 4))

The macro MULTIBOOT_TAG_ALIGN is defined in (include/multiboot2.h) as 8. This 
alignment value is consistent with the specification for tags in the multiboot2 
information structure, but not for tags in an OS image header.

thank you,

Safayet N Ahmed Ph.D

Researcher, Computer Engineer
GE Global Research Center
One Research Circle,
Niskayuna, NY 12309  United States
General Electric Company, GE Global Research

GE imagination at work



___
Bug-grub mailing list
Bug-grub@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-grub