Explicitly error out on host-guest version mismatch#1252
Explicitly error out on host-guest version mismatch#1252ludfjig wants to merge 1 commit intohyperlight-dev:mainfrom
Conversation
dblnz
left a comment
There was a problem hiding this comment.
This looks good, but do we want to match against the patch version also?
I think it would be better to ensure that the guest's major/minor are the same, but the patch version is greater or equal the host's version.
I thought about this, and then decided to be as conservative as possible, because this would limit some things we can update in patch versions. Maybe this is a good start, and then we can consider later whether we should relax it? |
Keeping it restrictive to start then relaxing later is a good option. Its harder to go the other way. |
There was a problem hiding this comment.
Pull request overview
This PR adds explicit version checking to prevent loading guest binaries built with incompatible hyperlight-guest-bin versions. When the host loads a guest binary, it now verifies that the embedded guest-bin version exactly matches the host version, addressing the lack of backwards compatibility guarantees mentioned in issue #845.
Changes:
- Embeds hyperlight-guest-bin version string in a custom ELF section (
.hyperlight_guest_bin_version) in guest binaries - Adds host-side logic to extract and validate the embedded version during guest binary loading
- Introduces a new
GuestBinVersionMismatcherror that clearly indicates version incompatibility
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_guest_bin/src/lib.rs | Embeds version string in custom ELF section using global_asm! |
| src/hyperlight_common/src/lib.rs | Defines macro and constant for ELF section name |
| src/hyperlight_host/src/mem/elf.rs | Adds read_section_as_string() to extract version from ELF and stores it in ElfInfo |
| src/hyperlight_host/src/mem/exe.rs | Exposes guest_bin_version() method and adds comprehensive tests for version checking |
| src/hyperlight_host/src/sandbox/snapshot.rs | Validates version match at snapshot creation time, rejecting mismatches |
| src/hyperlight_host/src/error.rs | Adds GuestBinVersionMismatch error variant with descriptive message |
| docs/how-to-build-a-hyperlight-guest-binary.md | Documents version compatibility requirements and how to resolve mismatches |
| /// Load an unpatched simpleguest through `Snapshot::from_env` and verify | ||
| /// that it succeeds when the embedded version matches the host version. | ||
| #[test] | ||
| fn from_env_accepts_matching_version() { |
There was a problem hiding this comment.
do we have an accepts_guest_not_built_with_higihterlight guest?
src/hyperlight_guest_bin/src/lib.rs
Outdated
| // Embed the hyperlight-guest-bin crate version in a dedicated ELF section so | ||
| // the host can verify ABI compatibility at load time. The section name is | ||
| // defined by [`hyperlight_common::hyperlight_guest_bin_version_section!`]. | ||
| core::arch::global_asm!(concat!( |
There was a problem hiding this comment.
We do a similar thing in hyperlight-wasm's build.rs, which uses .note_hyperlight_metadata and does it in Rust via #[link_section]---is there a reason to do it differently here/there?
I think SHT_PROGBITS is not the right section type for this. I went to look at what we are doing in hlwasm, and it looks like it is automagically getting SHT_NOTE, probably because the section name starts with .note. I dion't think that that's strictly correct either---it looks like in the System V gABI there is an explicit structure that SHT_NOTE sections are meant to follow: https://www.sco.com/developers/gabi/2003-12-17/ch5.pheader.html#note_section. However, the intent of the note section does seem like the best fit for this information, and the way we are using it seems not totally out of line with e.g. .note.gnu.build-id and .note.package, so I feel like perhaps we should just go with name = .note.whatever, sh_type = SHT_NOTE, and put the little note header structure in.
One difference here is that we may not actually need our sections to be SHF_ALLOC / covered by any segment, since we are checking them at load time, and we probably will do our own format for conveying the same information in snapshots. However, the waste of memory is very small, so if it isn't easy to avoid that (it might be easy with some assembler directives? not sure), it might not be worth doing anything about.
We could also use a custom section type from the SHT_LOUSER to SHT_HIUSER range, but I think using SHT_NOTE makes more sense.
If we do this just with the link_section attribute, we should check where that is getting turned into an SHT_NOTE section type---it looks like there are incompatibilities between ld.bfd and ld.lld if that's what's being relied on (https://reviews.llvm.org/D118490)
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
911dab9 to
9a416af
Compare
Hyperlight currently provides no backwards compatibility guarantees for guests. We should therefore error out explicitly if someone loads an old guest with new host, as they ABI might no longer match.
This PR enforces that hyperlight-host version must match the guest's hyperlight-guest-bin version.
Question: If the elf section is not found, we still load it. This is good for running guests that don't use hyperlight-guest-bin. However, should we enforce that the section must exist? Does anyone run guests without depending on hyperlight-guest-bin? In addition, stripping the binary will remove the new elf section, which is also something to consider
Closes #845