Skip to content

Conversation

@agustingianni
Copy link
Contributor

Hyperkit's multiboot loader incorrectly loads the firmware into guest memory.

This patch fixes the offset used to load the firmware image by using the offset of the first (smallest offset) loaded program segment in an elf32 firmware.

In essence what was happening before this patch was that hyperkit would load the entire ELF32 binary into memory from the start of the image. In some cases this works due to how some firmware image are built, but the correct behavior I think should be to find the offset instead of having to apply some link time tricks.

@djs55
Copy link
Collaborator

djs55 commented Jan 7, 2021

The CI failure looks unrelated:

undef: _bigstringaf_blit_to_bytes
undef: _bigstringaf_memcmp_bigstring
undef: _bigstringaf_memcmp_string
undef: _bigstringaf_blit_from_bytes
undef: _bigstringaf_blit_to_bigstring
Undefined symbols for architecture x86_64:
  "_bigstringaf_blit_to_bytes", referenced from:
      _camlBigstringaf__fun_1264 in mirage_block_ocaml.o
      _camlBigstringaf__substring_313 in mirage_block_ocaml.o
      _camlBigstringaf__to_string_320 in mirage_block_ocaml.o
      _camlBigstringaf__blit_to_bytes_407 in mirage_block_ocaml.o
      _camlBigstringaf__1 in mirage_block_ocaml.o

-- these were fixed in #305, now merged. I'll see if I have permission to force push a rebase.

The multiboot loader incorrectly loads the firmware into guest memory.
This patch fixes the offset used to load the firmware image by using
the offset of the first loaded program segment in an elf32 firmware.

Signed-off-by: Agustin Gianni <agustingianni@gmail.com>
Copy link
Collaborator

@djs55 djs55 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@djs55 djs55 merged commit 40cbd5c into moby:master Jan 7, 2021
@agustingianni
Copy link
Contributor Author

Thank you very much for merging this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants