Re: Tag-alignment in multiboot2 image headers

[Andrei Borzenkov]

On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research,
US) <address@hidden> 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?