On Wed, Mar 8, 2017, 22:28 Andrei Borzenkov <arvidj...@gmail.com> wrote:
> On Thu, Mar 9, 2017 at 1:17 AM, Ahmed, Safayet (GE Global Research, > US) <safayet.ah...@ge.com> 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
