rust-osdev/multiboot2

[Bug] Assertion failed when parsing EFI Multiboot memory region header

junyang-zh opened this issue · 4 comments

junyang-zh commented

Known versions

The bug can be reproduced with version 0.19.0 and 0.20.0. While 0.18.1 is functioning.

Bug behavior

Panicked with

panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/multiboot2-0.19.0/src/memory_map.rs:335:9:
assertion `left == right` failed
  left: 24
 right: 0

at

multiboot2/multiboot2/src/memory_map.rs

Lines 328 to 337 in 3077c3b

impl TagTrait for EFIMemoryMapTag {
const ID: TagType = TagType::EfiMmap;
fn dst_size(base_tag: &Tag) -> usize {
assert!(base_tag.size as usize >= EFI_METADATA_SIZE);
let size = base_tag.size as usize - EFI_METADATA_SIZE;
assert_eq!(size % mem::size_of::<EFIMemoryDesc>(), 0);
size / mem::size_of::<EFIMemoryDesc>()
}
}

Context

I used let mb2_info = BootInformation::load(addr) to load the header, and try to debug print it with "{:#?}", mb2_info.

The following information is printed:

Multiboot2BootInformation {
    start_address: 589824,
    end_address: 599200,
    total_size: 9376,
    basic_memory_info: Some(
        BasicMemoryInfoTag {
            typ: BasicMeminfo,
            size: 16,
            memory_lower: 640,
            memory_upper: 7168,
        },
    ),
    boot_loader_name: Some(
        BootLoaderNameTag {
            typ: BootLoaderName,
            size: 18,
            name: Ok(
                "GRUB 2.12",
            ),
        },
    ),
    command_line: Some(
        CommandLineTag {
            typ: Cmdline,
            size: 104,
            cmdline: Ok(
                "SHELL=/bin/sh LOGNAME=root HOME=/ USER=root PATH=/bin:/benchmark init=/usr/bin/busybox -- sh -l",
            ),
        },
    ),
    efi_bs_not_exited: None,

And it stops printing with a panic here.

Environment

  • OVMF built with EDK2 version edk2-stable202402 ;
  • QEMU built from source version 8.2.1.

I don't have time build a minimal reproduction demo. But one can do it under our kernel project https://github.com/asterinas/asterinas. There's a development image for the exact environment https://hub.docker.com/r/asterinas/asterinas.

Plans to fix it

We may use 0.18.1 for our project currently. I am happy to provide assistance for anyone taking it. I can also take it but I need guidance.

phip1611 commented

Yep, my fault. Sorry! UEFI doesn't want you to ever rely on size_of::<MemoryDescriptor> - which I did.

In the spec, this is not really that clear, but in the implementation, it is. https://github.com/tianocore/edk2/blob/7142e648416ff5d3eac6c6d607874805f5de0ca8/MdeModulePkg/Core/PiSmmCore/Page.c#L1059

  • 🚀1
phip1611 commented

@junyang-zh Can you please check if #216 fixes the problems for you?

  • ❤️1
junyang-zh commented

@junyang-zh Can you please check if #216 fixes the problems for you?

Tried! #216 is fully functioning in our specific use case. Thanks very much for fixing it such quickly.

  • 👍1
phip1611 commented

Released as multiboot2@0.20.1 at crates.io. I hope it works smoothly now - nice project you're working on!

  • ❤️1