Skip to content

fix: zero-fill NOBITS sections in ELF loader #1379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ludfjig wants to merge 1 commit into from
+33 −4
Closed

fix: zero-fill NOBITS sections in ELF loader #1379

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions src/hyperlight_host/src/mem/elf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ use std::sync::Arc;
use goblin::elf::reloc::{R_AARCH64_NONE, R_AARCH64_RELATIVE};
#[cfg(target_arch = "x86_64")]
use goblin::elf::reloc::{R_X86_64_NONE, R_X86_64_RELATIVE};
use goblin::elf::{Elf, ProgramHeaders, Reloc};
#[cfg(feature = "nanvix-unstable")]
use goblin::elf32::program_header::PT_LOAD;
#[cfg(not(feature = "nanvix-unstable"))]
use goblin::elf::{Elf, ProgramHeaders, Reloc, section_header};
use goblin::elf64::program_header::PT_LOAD;

use super::exe::LoadInfo;
Expand All @@ -45,6 +42,8 @@ pub(crate) struct ElfInfo {
shdrs: Vec<ResolvedSectionHeader>,
entry: u64,
relocs: Vec<Reloc>,
/// (addr, size) of NOBITS sections that need zero-filling (excludes .tbss).
nobits_ranges: Vec<(u64, u64)>,
/// The hyperlight version string embedded by `hyperlight-guest-bin`, if
/// present. Used to detect version/ABI mismatches between guest and host.
guest_bin_version: Option<String>,
Expand Down Expand Up @@ -128,6 +127,20 @@ impl ElfInfo {
// hyperlight-guest-bin.
let guest_bin_version = Self::read_version_note(&elf, bytes);

// Collect NOBITS sections (e.g. .bss) that need zero-filling.
// Skip .tbss (SHF_TLS) since thread-local BSS is allocated per-thread.
let nobits_ranges: Vec<(u64, u64)> = {
elf.section_headers
.iter()
.filter(|sh| {
sh.sh_type == section_header::SHT_NOBITS
&& sh.sh_size > 0
&& (sh.sh_flags & u64::from(section_header::SHF_TLS)) == 0
})
.map(|sh| (sh.sh_addr, sh.sh_size))
.collect()
Comment on lines +133 to +141
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

When collecting NOBITS ranges, consider filtering to only sections that are actually mapped into memory (e.g. SHF_ALLOC) in addition to excluding TLS. As-is, non-ALLOC NOBITS sections (often with sh_addr == 0) can cause spurious warnings during load, and in some layouts could lead to zero-filling unintended parts of the image if base_va is also 0.

Copilot uses AI. Check for mistakes.
};

Ok(ElfInfo {
payload: bytes.to_vec(),
phdrs: elf.program_headers,
Expand All @@ -146,6 +159,7 @@ impl ElfInfo {
.collect(),
entry: elf.entry,
relocs,
nobits_ranges,
guest_bin_version,
})
}
Expand Down Expand Up @@ -206,6 +220,21 @@ impl ElfInfo {
.copy_from_slice(&self.payload[payload_offset..payload_offset + payload_len]);
target[start_va + payload_len..start_va + phdr.p_memsz as usize].fill(0);
}
// Zero-fill NOBITS sections (e.g. .bss) that were not already
// covered by the filesz < memsz zeroing above.
for &(addr, size) in &self.nobits_ranges {
let sh_start = (addr - base_va) as usize;
Copy link
Copy Markdown
Contributor

@dblnz dblnz Apr 15, 2026

Choose a reason for hiding this comment

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

Nit: I am not sure if it can happen, but is this guaranteed to not fail? Maybe a checked sub is better here

let sh_end = sh_start + size as usize;
Comment on lines +226 to +227
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

sh_start is computed with (addr - base_va) as usize and sh_end with sh_start + size as usize. If addr < base_va (or if the addition overflows usize), this can wrap and potentially make sh_end <= target.len() true, leading to a panic on the subsequent slice. Use checked arithmetic (checked_sub, checked_add, and usize::try_from) and skip/return an error when the range is not representable or falls before base_va.

Suggested change
let sh_start = (addr - base_va) as usize;
let sh_end = sh_start + size as usize;
let Some(sh_offset) = addr.checked_sub(base_va) else {
tracing::warn!(
"NOBITS section at VA {:#x} (size {:#x}) falls before base VA {:#x}, skipping zero-fill",
addr,
size,
base_va
);
continue;
};
let Ok(sh_start) = usize::try_from(sh_offset) else {
tracing::warn!(
"NOBITS section at VA {:#x} (size {:#x}) offset from base VA {:#x} is not representable, skipping zero-fill",
addr,
size,
base_va
);
continue;
};
let Ok(sh_size) = usize::try_from(size) else {
tracing::warn!(
"NOBITS section at VA {:#x} has size {:#x} that is not representable, skipping zero-fill",
addr,
size
);
continue;
};
let Some(sh_end) = sh_start.checked_add(sh_size) else {
tracing::warn!(
"NOBITS section at VA {:#x} (size {:#x}) overflows target range calculation, skipping zero-fill",
addr,
size
);
continue;
};

Copilot uses AI. Check for mistakes.
if sh_end <= target.len() {
target[sh_start..sh_end].fill(0);
Comment on lines +223 to +229
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

This change introduces new loader behavior (zero-filling NOBITS sections even when p_filesz == p_memsz). There doesn’t appear to be a test asserting this correctness fix; adding a unit/integration test that reproduces the Unikraft-style layout (NOBITS VMA overlapping file bytes) would help prevent regressions.

Copilot generated this review using guidance from repository custom instructions.
} else {
tracing::warn!(
"NOBITS section at VA {:#x} (size {:#x}) extends past loaded image, skipping zero-fill",
addr,
size
);
}
}
let get_addend = |name, r: &Reloc| {
r.r_addend
.ok_or_else(|| new_error!("{} missing addend", name))
Expand Down
Loading