Remove section header insertion from syscall rewriter#636
Open
Remove section header insertion from syscall rewriter#636
Conversation
CvvT
reviewed
Feb 4, 2026
litebox_common_linux/src/loader.rs
Outdated
| // Search backwards from the end of the file at page boundaries for the magic. | ||
| // We limit the search to a reasonable number of pages to avoid scanning the | ||
| // entire file. | ||
| let min_offset = file_size.saturating_sub((MAX_SEARCH_PAGES * PAGE_SIZE) as u64); |
Contributor
There was a problem hiding this comment.
Why not put those entries at the end of the file (end of trampoline code instead of head of it) so that we only need to read the last 32/20 bytes for metadata?
Member
Author
There was a problem hiding this comment.
Great suggestion! Working on it.
wdcui
force-pushed
the
wdcui/copilot-elf
branch
from
5080bd9 to
9416585
Compare
Member
|
I haven't reviewed the code, but quick note: we use a constant like |
Member
Author
|
I thought about if we should bump the version from LITEBOX0 to LITEBOX1 and decided to just keep LITEBOX0. Let me know if anyone thinks we should increase it to LITEBOX1. I'm okay with either way. |
This addresses reviewer feedback to simplify the trampoline parsing. Instead of searching for the magic at page boundaries, loaders can now just read the last 32/20 bytes of the file to get the metadata. File layout: [ELF][padding][trampoline code][header] - Header: magic(8) + entry(8/4) + vaddr(8/4) + code_size(8/4) - Trampoline code: entry_placeholder(8/4) + actual_code... Changes: - syscall rewriter: write code first, then header at end - Rust loader: read header from end of file, not page boundaries - rtld_audit.c: read header from end for shared libs, read entry from offset 0 for main binary (already mapped by loader) - Fix jmp displacement to point to offset 0 (entry) instead of 8
With the header moved to the end of the file, the syscall entry point is now at offset 0 (not offset 8). The 32-bit CALL [EAX + offset] displacement needs to target offset 0 instead of offset 8. The formula changes from -(len - 2) and -(len - 11) to -(len - 3) in all three locations where 32-bit indirect calls are generated.
The header at the end of the file now contains: - magic (8 bytes): "LITEBOX0" - file_offset (8/4 bytes): file offset where trampoline code starts - vaddr (8/4 bytes): virtual address of trampoline - code_size (8/4 bytes): size of trampoline code This is cleaner because: 1. Entry point stays at offset 0 of the code (no duplicate in header) 2. File offset is directly stored (no calculation needed from code_size) 3. Header is pure metadata about the trampoline location
wdcui
force-pushed
the
wdcui/copilot-elf
branch
from
c131be8 to
40b270e
Compare
|
🤖 SemverChecks 🤖 Click for details |
CvvT
reviewed
Feb 6, 2026
Comment on lines
+256
to
+272
| // 64-bit: [0..8] magic, [8..16] file_offset, [16..24] vaddr, [24..32] code_size | ||
| let file_offset: u64 = u64::from_le_bytes(header_buf[8..16].try_into().unwrap()); | ||
| let vaddr: usize = u64::from_le_bytes(header_buf[16..24].try_into().unwrap()) | ||
| .try_into() | ||
| .map_err(|_| ElfParseError::BadTrampoline)?; | ||
| let trampoline_size: usize = u64::from_le_bytes(header_buf[24..32].try_into().unwrap()) | ||
| .try_into() | ||
| .map_err(|_| ElfParseError::BadTrampoline)?; | ||
| (file_offset, vaddr, trampoline_size) | ||
| } else { | ||
| // 32-bit: [0..8] magic, [8..12] file_offset, [12..16] vaddr, [16..20] code_size | ||
| let file_offset = u64::from(u32::from_le_bytes(header_buf[8..12].try_into().unwrap())); | ||
| let vaddr = u32::from_le_bytes(header_buf[12..16].try_into().unwrap()) as usize; | ||
| let trampoline_size = | ||
| u32::from_le_bytes(header_buf[16..20].try_into().unwrap()) as usize; | ||
| (file_offset, vaddr, trampoline_size) | ||
| }; |
Contributor
There was a problem hiding this comment.
This is fine, but I believe a better way is to define a struct for it and then directly converting raw bytes to a struct.
| }; | ||
|
|
||
| // Validate trampoline size bounds | ||
| if trampoline_size == 0 || trampoline_size > MAX_TRAMPOLINE_SIZE { |
Contributor
There was a problem hiding this comment.
Why we need MAX_TRAMPOLINE_SIZE? Looks like a small number to me.
Comment on lines
+30
to
+33
| // Maximum number of pages to search for trampoline | ||
| #define MAX_SEARCH_PAGES 16 | ||
| // Maximum allowed trampoline size (must match Rust loader) | ||
| #define MAX_TRAMP_SIZE (MAX_SEARCH_PAGES * 0x1000) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of inserting a
.trampolineLB0section header into the ELF file, store trampoline metadata directly at the end of the ELF file. This fixes issues when there is no space in sections or symbol tables. A future PR will directly patch the ELF file without invoking any other crates to reassemble the ELF file.