Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

Refactor - Separates the text section from read-only sections #553

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Refactor - Separates the text section from read-only sections #553

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
81 changes: 23 additions & 58 deletions src/elf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,18 +213,6 @@ impl BpfRelocationType {
}
}

#[derive(Debug, PartialEq)]
struct SectionInfo {
name: String,
vaddr: u64,
offset_range: Range<usize>,
}
impl SectionInfo {
fn mem_size(&self) -> usize {
mem::size_of::<Self>().saturating_add(self.name.capacity())
}
}

#[derive(Debug, PartialEq)]
pub(crate) enum Section {
/// Owned section data.
Expand All @@ -249,8 +237,8 @@ pub struct Executable<C: ContextObject> {
sbpf_version: SBPFVersion,
/// Read-only section
ro_section: Section,
/// Text section info
text_section_info: SectionInfo,
/// Text section
text_section: Section,
/// Address of the entry point
entry_pc: usize,
/// Call resolution map (hash, pc, name)
Expand All @@ -275,21 +263,16 @@ impl<C: ContextObject> Executable<C> {

/// Get the .text section virtual address and bytes
pub fn get_text_bytes(&self) -> (u64, &[u8]) {
let (ro_offset, ro_section) = match &self.ro_section {
Section::Owned(offset, data) => (*offset, data.as_slice()),
let (offset, section) = match &self.text_section {
Section::Owned(_offset, _data) => unreachable!(),
Section::Borrowed(offset, byte_range) => {
(*offset, &self.elf_bytes.as_slice()[byte_range.clone()])
}
};

let offset = self
.text_section_info
.vaddr
.saturating_sub(ebpf::MM_PROGRAM_START)
.saturating_sub(ro_offset as u64) as usize;
(
self.text_section_info.vaddr,
&ro_section[offset..offset.saturating_add(self.text_section_info.offset_range.len())],
// The offset field is relative to `MM_PROGRAM_START`
ebpf::MM_PROGRAM_START.saturating_add(offset as u64),
Copy link

@LucasSte LucasSte Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm guessing that you are adding to MM_PROGRAM_START because you subtracted it from the offset in the initialization to maintain parallelism between this function and the get_ro_region, right?

Would you mind explaining this in a comment?

Copy link
Author

Choose a reason for hiding this comment

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

rbpf/src/elf.rs

Line 237 in a400aa7

/// The first field is the offset of the section from MM_PROGRAM_START. The

The structs field is defined as being relative to MM_PROGRAM_START, both the text section and all read-only sections live in that MemoryRegion.

Copy link

Choose a reason for hiding this comment

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

In line 469, you're subtracting the values

 text_section_vaddr.saturating_sub(ebpf::MM_PROGRAM_START) as usize,

My concern was that it was a little confusing that you subtract and then you add in get_text_bytes. A comment to explain why this is the case would be helpful.

section,
)
}

Expand All @@ -313,10 +296,13 @@ impl<C: ContextObject> Executable<C> {
self.entry_pc
}

/// Get the text section offset
/// Get the text section offset in the ELF file
#[cfg(feature = "debugger")]
pub fn get_text_section_offset(&self) -> u64 {
self.text_section_info.offset_range.start as u64
match &self.text_section {
Section::Owned(_offset, _data) => unreachable!(),
Section::Borrowed(_offset, byte_range) => byte_range.start as u64,
}
}

/// Get the loader built-in program
Expand Down Expand Up @@ -362,8 +348,6 @@ impl<C: ContextObject> Executable<C> {
mut function_registry: FunctionRegistry<usize>,
) -> Result<Self, ElfError> {
let elf_bytes = AlignedMemory::from_slice(text_bytes);
let config = loader.get_config();
let enable_symbol_and_section_labels = config.enable_symbol_and_section_labels;
let entry_pc = if let Some((_name, pc)) = function_registry.lookup_by_name(b"entrypoint") {
pc
} else {
Expand All @@ -379,15 +363,7 @@ impl<C: ContextObject> Executable<C> {
elf_bytes,
sbpf_version,
ro_section: Section::Borrowed(0, 0..text_bytes.len()),
text_section_info: SectionInfo {
name: if enable_symbol_and_section_labels {
".text".to_string()
} else {
String::default()
},
vaddr: ebpf::MM_PROGRAM_START,
offset_range: 0..text_bytes.len(),
},
text_section: Section::Borrowed(0, 0..text_bytes.len()),
entry_pc,
function_registry,
loader,
Expand Down Expand Up @@ -429,29 +405,16 @@ impl<C: ContextObject> Executable<C> {

// calculate the text section info
let text_section = get_section(elf, b".text")?;
let text_section_info = SectionInfo {
name: if config.enable_symbol_and_section_labels {
elf.section_name(text_section.sh_name)
.ok()
.and_then(|name| std::str::from_utf8(name).ok())
.unwrap_or(".text")
.to_string()
} else {
String::default()
},
vaddr: if sbpf_version.enable_elf_vaddr()
&& text_section.sh_addr >= ebpf::MM_PROGRAM_START
{
let text_section_vaddr =
if sbpf_version.enable_elf_vaddr() && text_section.sh_addr >= ebpf::MM_PROGRAM_START {
text_section.sh_addr
} else {
text_section.sh_addr.saturating_add(ebpf::MM_PROGRAM_START)
},
offset_range: text_section.file_range().unwrap_or_default(),
};
};
let vaddr_end = if sbpf_version.reject_rodata_stack_overlap() {
text_section_info.vaddr.saturating_add(text_section.sh_size)
text_section_vaddr.saturating_add(text_section.sh_size)
} else {
text_section_info.vaddr
text_section_vaddr
};
if (config.reject_broken_elfs
&& !sbpf_version.enable_elf_vaddr()
Expand Down Expand Up @@ -503,7 +466,11 @@ impl<C: ContextObject> Executable<C> {
elf_bytes,
sbpf_version,
ro_section,
text_section_info,
text_section: Section::Borrowed(
// The offset field is relative to `MM_PROGRAM_START`
text_section_vaddr.saturating_sub(ebpf::MM_PROGRAM_START) as usize,
text_section.file_range().unwrap_or_default(),
),
entry_pc,
function_registry,
loader,
Expand All @@ -525,8 +492,6 @@ impl<C: ContextObject> Executable<C> {
Section::Owned(_, data) => data.capacity(),
Section::Borrowed(_, _) => 0,
})
Comment on lines 492 to 494
Copy link

@LucasSte LucasSte Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should we include an arm like this to account for a possible owned data (in the text section)?

Copy link
Author

Choose a reason for hiding this comment

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

The text section is not supposed to ever be owned / copied. Hence why all match expressions against it have an unreachable! arm.

// text section info
.saturating_add(self.text_section_info.mem_size())
// bpf functions
.saturating_add(self.function_registry.mem_size());

Expand Down