Skip to content
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.

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

Handle multiple LOAD program headers with different biases #107

Merged
merged 2 commits into from
Oct 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
128 changes: 74 additions & 54 deletions src/elf/dyn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ if_alloc! {
use core::result;
use container::{Ctx, Container};
use strtab::Strtab;
use self::dyn32::{DynamicInfo};
use alloc::vec::Vec;

#[derive(Default, PartialEq, Clone)]
Expand Down Expand Up @@ -362,7 +361,7 @@ if_alloc! {
impl Dynamic {
#[cfg(feature = "endian_fd")]
/// Returns a vector of dynamic entries from the underlying byte `bytes`, with `endianness`, using the provided `phdrs`
pub fn parse(bytes: &[u8], phdrs: &[::elf::program_header::ProgramHeader], bias: usize, ctx: Ctx) -> ::error::Result<Option<Self>> {
pub fn parse(bytes: &[u8], phdrs: &[::elf::program_header::ProgramHeader], ctx: Ctx) -> ::error::Result<Option<Self>> {
use scroll::ctx::SizeWith;
use scroll::Pread;
use elf::program_header;
Expand All @@ -381,8 +380,7 @@ if_alloc! {
}
let mut info = DynamicInfo::default();
for dyn in &dyns {
let dyn: dyn32::Dyn = dyn.clone().into();
info.update(bias, &dyn);
info.update(phdrs, dyn);
}
let count = dyns.len();
return Ok(Some(Dynamic { dyns: dyns, info: info, count: count }));
Expand Down Expand Up @@ -463,36 +461,6 @@ macro_rules! elf_dyn_std_impl {
}
}

impl fmt::Debug for DynamicInfo {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let gnu_hash = if let Some(addr) = self.gnu_hash { addr } else { 0 };
let hash = if let Some(addr) = self.hash { addr } else { 0 };
let pltgot = if let Some(addr) = self.pltgot { addr } else { 0 };
write!(f, "rela: 0x{:x} relasz: {} relaent: {} relacount: {} gnu_hash: 0x{:x} hash: 0x{:x} strtab: 0x{:x} strsz: {} symtab: 0x{:x} syment: {} pltgot: 0x{:x} pltrelsz: {} pltrel: {} jmprel: 0x{:x} verneed: 0x{:x} verneednum: {} versym: 0x{:x} init: 0x{:x} fini: 0x{:x} needed_count: {}",
self.rela,
self.relasz,
self.relaent,
self.relacount,
gnu_hash,
hash,
self.strtab,
self.strsz,
self.symtab,
self.syment,
pltgot,
self.pltrelsz,
self.pltrel,
self.jmprel,
self.verneed,
self.verneednum,
self.versym,
self.init,
self.fini,
self.needed_count,
)
}
}

/// Returns a vector of dynamic entries from the given fd and program headers
#[cfg(feature = "std")]
pub fn from_fd(mut fd: &File, phdrs: &[$phdr]) -> Result<Option<Vec<Dyn>>> {
Expand Down Expand Up @@ -547,6 +515,23 @@ macro_rules! elf_dyn_std_impl {
needed
}
}
};
}

macro_rules! elf_dynamic_info_std_impl {
($size:ident, $phdr:ty) => {
/// Convert a virtual memory address to a file offset
fn vm_to_offset(phdrs: &[$phdr], address: $size) -> Option<$size> {
Copy link
Owner

Choose a reason for hiding this comment

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

I have some reservations about looping over the phdrs everytime an address lookup is requested; we really need to create an interval tree (and we could also use this in PE, which has very similar lookups).

I know you're trying to avoid allocation, but core dumps, for example, can have hundreds of phdrs, but then that might not ever have any bias calculations? I guess if this only occurs in the single pass over the dynamic info we're ok. I dunno, just some thoughts here, as opposed to allocating an associative list or a proper interval tree (which rust afaics doesn't have a robust one with a nice api unfortunately...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it would be overkill to do that for the single pass over the dynamic info.

for ph in phdrs {
if address >= ph.p_vaddr {
let offset = address - ph.p_vaddr;
if offset < ph.p_memsz {
return ph.p_offset.checked_add(offset );
}
}
}
None
}

/// Important dynamic linking info generated via a single pass through the `_DYNAMIC` array
#[derive(Default)]
Expand Down Expand Up @@ -587,34 +572,34 @@ macro_rules! elf_dyn_std_impl {

impl DynamicInfo {
#[inline]
pub fn update(&mut self, bias: usize, dyn: &Dyn) {
pub fn update(&mut self, phdrs: &[$phdr], dyn: &Dyn) {
match dyn.d_tag as u64 {
DT_RELA => self.rela = dyn.d_val.wrapping_add(bias as _) as usize, // .rela.dyn
DT_RELA => self.rela = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0) as usize, // .rela.dyn
DT_RELASZ => self.relasz = dyn.d_val as usize,
DT_RELAENT => self.relaent = dyn.d_val as _,
DT_RELACOUNT => self.relacount = dyn.d_val as usize,
DT_REL => self.rel = dyn.d_val.wrapping_add(bias as _) as usize, // .rel.dyn
DT_REL => self.rel = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0) as usize, // .rel.dyn
DT_RELSZ => self.relsz = dyn.d_val as usize,
DT_RELENT => self.relent = dyn.d_val as _,
DT_RELCOUNT => self.relcount = dyn.d_val as usize,
DT_GNU_HASH => self.gnu_hash = Some(dyn.d_val.wrapping_add(bias as _)),
DT_HASH => self.hash = Some(dyn.d_val.wrapping_add(bias as _)) as _,
DT_STRTAB => self.strtab = dyn.d_val.wrapping_add(bias as _) as usize,
DT_GNU_HASH => self.gnu_hash = vm_to_offset(phdrs, dyn.d_val),
DT_HASH => self.hash = vm_to_offset(phdrs, dyn.d_val),
DT_STRTAB => self.strtab = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0) as usize,
DT_STRSZ => self.strsz = dyn.d_val as usize,
DT_SYMTAB => self.symtab = dyn.d_val.wrapping_add(bias as _) as usize,
DT_SYMTAB => self.symtab = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0) as usize,
DT_SYMENT => self.syment = dyn.d_val as usize,
DT_PLTGOT => self.pltgot = Some(dyn.d_val.wrapping_add(bias as _)) as _,
DT_PLTGOT => self.pltgot = vm_to_offset(phdrs, dyn.d_val),
DT_PLTRELSZ => self.pltrelsz = dyn.d_val as usize,
DT_PLTREL => self.pltrel = dyn.d_val as _,
DT_JMPREL => self.jmprel = dyn.d_val.wrapping_add(bias as _) as usize, // .rela.plt
DT_VERNEED => self.verneed = dyn.d_val.wrapping_add(bias as _) as _,
DT_JMPREL => self.jmprel = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0) as usize, // .rela.plt
DT_VERNEED => self.verneed = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0),
DT_VERNEEDNUM => self.verneednum = dyn.d_val as _,
DT_VERSYM => self.versym = dyn.d_val.wrapping_add(bias as _) as _,
DT_INIT => self.init = dyn.d_val.wrapping_add(bias as _) as _,
DT_FINI => self.fini = dyn.d_val.wrapping_add(bias as _) as _,
DT_INIT_ARRAY => self.init_array = dyn.d_val.wrapping_add(bias as _) as _,
DT_VERSYM => self.versym = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0),
DT_INIT => self.init = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0),
DT_FINI => self.fini = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0),
DT_INIT_ARRAY => self.init_array = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0),
DT_INIT_ARRAYSZ => self.init_arraysz = dyn.d_val as _,
DT_FINI_ARRAY => self.fini_array = dyn.d_val.wrapping_add(bias as _) as _,
DT_FINI_ARRAY => self.fini_array = vm_to_offset(phdrs, dyn.d_val).unwrap_or(0),
DT_FINI_ARRAYSZ => self.fini_arraysz = dyn.d_val as _,
DT_NEEDED => self.needed_count += 1,
DT_FLAGS => self.flags = dyn.d_val as _,
Expand All @@ -624,17 +609,52 @@ macro_rules! elf_dyn_std_impl {
_ => (),
}
}
pub fn new(dynamic: &[Dyn], bias: usize) -> DynamicInfo {
pub fn new(dynamic: &[Dyn], phdrs: &[$phdr]) -> DynamicInfo {
let mut info = DynamicInfo::default();
for dyn in dynamic {
info.update(bias, &dyn);
info.update(phdrs, &dyn);
}
info
}
} // end if_std
}

if_alloc! {
Copy link
Owner

Choose a reason for hiding this comment

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

Just curious: how come this got moved to alloc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was always in alloc. I just moved it into another macro and kept it in alloc without thinking about whether it should be. I can move it out as part of this PR if you want, but it's probably better as another PR.

Copy link
Owner

Choose a reason for hiding this comment

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

Nah just leave

impl fmt::Debug for DynamicInfo {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let gnu_hash = if let Some(addr) = self.gnu_hash { addr } else { 0 };
let hash = if let Some(addr) = self.hash { addr } else { 0 };
let pltgot = if let Some(addr) = self.pltgot { addr } else { 0 };
write!(f, "rela: 0x{:x} relasz: {} relaent: {} relacount: {} gnu_hash: 0x{:x} hash: 0x{:x} strtab: 0x{:x} strsz: {} symtab: 0x{:x} syment: {} pltgot: 0x{:x} pltrelsz: {} pltrel: {} jmprel: 0x{:x} verneed: 0x{:x} verneednum: {} versym: 0x{:x} init: 0x{:x} fini: 0x{:x} needed_count: {}",
self.rela,
self.relasz,
self.relaent,
self.relacount,
gnu_hash,
hash,
self.strtab,
self.strsz,
self.symtab,
self.syment,
pltgot,
self.pltrelsz,
self.pltrel,
self.jmprel,
self.verneed,
self.verneednum,
self.versym,
self.init,
self.fini,
self.needed_count,
)
}
}
}
};
}

if_alloc! {
elf_dynamic_info_std_impl!(u64, ::elf::program_header::ProgramHeader);
}

pub mod dyn32 {
pub use elf::dyn::*;
Expand All @@ -644,10 +664,9 @@ pub mod dyn32 {
pub const SIZEOF_DYN: usize = 8;

elf_dyn_std_impl!(u32, ::elf32::program_header::ProgramHeader);

elf_dynamic_info_std_impl!(u32, ::elf::program_header::program_header32::ProgramHeader);
}


pub mod dyn64 {
pub use elf::dyn::*;

Expand All @@ -656,4 +675,5 @@ pub mod dyn64 {
pub const SIZEOF_DYN: usize = 16;

elf_dyn_std_impl!(u64, ::elf64::program_header::ProgramHeader);
elf_dynamic_info_std_impl!(u64, ::elf::program_header::program_header64::ProgramHeader);
}
28 changes: 1 addition & 27 deletions src/elf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ if_sylvan! {
pub is_lib: bool,
/// The binaries entry point address, if it has one
pub entry: u64,
/// The bias used to overflow virtual memory addresses into physical byte offsets into the binary
pub bias: u64,
/// Whether the binary is little endian or not
pub little_endian: bool,
ctx: Ctx,
Expand Down Expand Up @@ -220,27 +218,6 @@ if_sylvan! {

let program_headers = ProgramHeader::parse(bytes, header.e_phoff as usize, header.e_phnum as usize, ctx)?;

let mut bias: usize = 0;
for ph in &program_headers {
if ph.p_type == program_header::PT_LOAD {
// NB this _only_ works on the first load address, and the GOT values (usually at base + 2000) will be incorrect binary offsets...
// this is an overflow hack that allows us to use virtual memory addresses
// as though they're in the file by generating a fake load bias which is then
// used to overflow the values in the dynamic array, and in a few other places
// (see Dyn::DynamicInfo), to generate actual file offsets; you may have to
// marinate a bit on why this works. i am unsure whether it works in every
// conceivable case. i learned this trick from reading too much dynamic linker
// C code (a whole other class of C code) and having to deal with broken older
// kernels on VMs. enjoi
bias = match container {
Container::Little => (::core::u32::MAX - (ph.p_vaddr as u32)).wrapping_add(1) as usize,
Container::Big => (::core::u64::MAX - ph.p_vaddr).wrapping_add(1) as usize,
};
// we must grab only the first one, otherwise the bias will be incorrect
break;
}
}

let mut interpreter = None;
for ph in &program_headers {
if ph.p_type == program_header::PT_INTERP && ph.p_filesz != 0 {
Expand Down Expand Up @@ -284,7 +261,7 @@ if_sylvan! {
let mut dynrels = RelocSection::default();
let mut pltrelocs = RelocSection::default();
let mut dynstrtab = Strtab::default();
let dynamic = Dynamic::parse(bytes, &program_headers, bias, ctx)?;
let dynamic = Dynamic::parse(bytes, &program_headers, ctx)?;
if let Some(ref dynamic) = dynamic {
let dyn_info = &dynamic.info;
dynstrtab = Strtab::parse(bytes,
Expand Down Expand Up @@ -347,7 +324,6 @@ if_sylvan! {
is_64: is_64,
is_lib: is_lib,
entry: entry as u64,
bias: bias as u64,
Copy link
Owner

Choose a reason for hiding this comment

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

hmmm; I don't like to see this field go, but given now we're supporting multiple biases we have to. Or maybe I'm wrong and its useless? Still might be nice to expose the iteration macro as a function hanging off the Elf type for users to pass an unbiased value in and get biased value out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since there's no single bias, this field seems wrong or at least misleading to me. Exposing the iteration might be ok, but we can wait until someone needs it. The current iteration can't be part of the Elf type because we need it during construction of the Elf type, but any exposed version would be best as part of the Elf type.

Copy link
Owner

Choose a reason for hiding this comment

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

Got it, makes sense

little_endian: is_lsb,
ctx,
})
Expand Down Expand Up @@ -376,7 +352,6 @@ mod tests {
assert!(binary.is_64);
assert!(!binary.is_lib);
assert_eq!(binary.entry, 0);
assert_eq!(binary.bias, 0);
assert!(binary.syms.get(1000).is_none());
assert!(binary.syms.get(5).is_some());
let syms = binary.syms.to_vec();
Expand Down Expand Up @@ -408,7 +383,6 @@ mod tests {
assert!(!binary.is_64);
assert!(!binary.is_lib);
assert_eq!(binary.entry, 0);
assert_eq!(binary.bias, 0);
assert!(binary.syms.get(1000).is_none());
assert!(binary.syms.get(5).is_some());
let syms = binary.syms.to_vec();
Expand Down
1 change: 1 addition & 0 deletions src/pe/section_table.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use alloc::string::{String, ToString};
use scroll::{self, Pread};
use error::{self, Error};

Expand Down
2 changes: 1 addition & 1 deletion src/pe/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use error;

use super::section_table;

use std::cmp;
use core::cmp;

pub fn is_in_range (rva: usize, r1: usize, r2: usize) -> bool {
r1 <= rva && rva < r2
Expand Down
1 change: 0 additions & 1 deletion tests/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ fn parse_self() {
match goblin::Object::parse(&bytes).expect("parse object") {
goblin::Object::Elf(elf) => {
assert!(elf.entry == 0);
assert!(elf.bias == 0);
}
goblin::Object::Mach(goblin::mach::Mach::Binary(macho)) => {
assert_eq!(macho.header.filetype, goblin::mach::header::MH_OBJECT);
Expand Down