Safer memory-mapped structures
This article is part of the Making our own executable packer series.
Welcome back to the "Making our own executable packer" series, where digressions are our bread and butter.
Last time, we implemented indirect functions in a no-libc C program. Of course, we got lost on the way and accidentally implemented a couple of useful elk-powered GDB functions - with only the minimal required amount of Python code.
The article got pretty long, and we could use a nice distraction. And I have just the thing! A little while ago, a member of the Rust compiler team stumbled upon this series and gave me some feedback.
The first piece of feedback is easily addressed - there were a few "useless use of transmute", like here for example:
// in `delf/src/lib.rs` impl Addr { pub unsafe fn as_ptr<T>(&self) -> *const T { std::mem::transmute(self.0 as usize) } pub unsafe fn as_mut_ptr<T>(&self) -> *mut T { std::mem::transmute(self.0 as usize) } }
While playing around with loading executables, I had gotten used to stuff
being wildly unsafe, but it turns out - this is perfectly safe! Raw pointers
are basically typed usize
, so this is perfectly fine:
// in `delf/src/lib.rs` impl Addr { pub fn as_ptr<T>(&self) -> *const T { self.0 as *const T } pub fn as_mut_ptr<T>(&self) -> *mut T { self.0 as *mut T } }
Dereferencing those pointers is the unsafe thing to do, so this won't compile:
// hypothetical code, not committed impl Addr { pub fn example(&self) { let x = *self.as_ptr::<u64>(); } }
$ cargo b error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block --> /home/amos/ftl/delf/src/lib.rs:371:17 | 371 | let x = *self.as_ptr::<u64>(); | ^^^^^^^^^^^^^^^^^^^^^ dereference of raw pointer | = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
Also, in our call to MemoryMap::new
, we can remove that unsafe block - it's
not actually unsafe!
// in `elk/src/process.rs` let map = MemoryMap::new( filesz.into(), &[ MapOption::MapReadable, MapOption::MapWritable, MapOption::MapExecutable, MapOption::MapFd(fs_file.as_raw_fd()), MapOption::MapOffset(offset.into()), // before: MapOption::MapAddr(unsafe { (base + vaddr).as_ptr() }), // after: MapOption::MapAddr((base + vaddr).as_ptr()), ], )?;
Similarly, we don't need it when printing the contents of msg
from
libmsg.so
(not sure why we're still carrying that code around but oh well):
// in `elk/src/process.rs` // in `impl Process`, `fn load_object` if path.to_str().unwrap().ends_with("libmsg.so") { // before: let msg_addr: *const u8 = unsafe { (base + delf::Addr(0x2000)).as_ptr() }; // after: let msg_addr: *const u8 = (base + delf::Addr(0x2000)).as_ptr(); dbg!(msg_addr); let msg_slice = unsafe { std::slice::from_raw_parts(msg_addr, 0x26) }; let msg = std::str::from_utf8(msg_slice).unwrap(); dbg!(msg); }
Ah. I feel lighter already. Not that unsafe
has any performance penalty -
it just weighs heavy on my shoulders.
So where do we need transmute?
We still need it when processing indirect functions:
// in `elk/src/process.rs` // in `match reltype` RT::IRelative => unsafe { let selector: extern "C" fn() -> delf::Addr = transmute(obj.base + addend); objrel.addr().set(selector()); },
As a tiny nitpick, we should probably mark selector
as unsafe - it is
definitely not safe to call:
// in `elk/src/process.rs` // in `match reltype` RT::IRelative => unsafe { // spread across two lines for readability type Selector = unsafe extern "C" fn() -> delf::Addr; let selector: Selector = transmute(obj.base + addend); objrel.addr().set(selector()); },
We also still need transmute
when jumping to the entry point - and we can
improve the function signature here too.
// in `elk/src/main.rs` unsafe fn jmp(addr: *const u8) { let fn_ptr: fn() = std::mem::transmute(addr); fn_ptr(); }
Let's make a quick list. fn_ptr
:
- is not safe to call
- is a C function
- never returns
I had no idea you could annotate Rust functions with "never returns" but it's actually super easy:
What?
// in `elk/src/main.rs` unsafe fn jmp(addr: *const u8) -> ! { // spread across two lines for readability type EntryPoint = unsafe extern "C" fn() -> !; let entry_point: EntryPoint = std::mem::transmute(addr); entry_point(); }
And in cmd_run
, we can remove the final Ok(())
, because, well,
jmp
never returns:
// in` elk/src/main.rs` fn cmd_run(args: RunArgs) -> Result<(), AnyError> { let mut proc = process::Process::new(); let exec_index = proc.load_object_and_dependencies(args.exec_path)?; proc.apply_relocations()?; proc.adjust_protections()?; let exec_obj = &proc.objects[exec_index]; let entry_point = exec_obj.file.entry_point + exec_obj.base; unsafe { jmp(entry_point.as_ptr()) }; // (deleted line was here) }
Now! Onto the meat of the article. One of the naughtiest things
we did back in Dynamic linker speed and
correctness is
introduce the Name
enum:
// in `elk/src/name.rs` #[derive(Clone)] pub enum Name { FromAddr { addr: delf::Addr, len: usize }, Owned(Vec<u8>), }
So far it has been... wildly unsafe.
The constructor for the FromAddr
variant was unsafe:
pub unsafe fn from_addr(addr: delf::Addr) -> Self { let len = addr .as_slice::<u8>(2048) .iter() .position(|&c| c == 0) .expect("scanned 2048 bytes without finding null-terminator for name"); Self::FromAddr { addr, len } }
It's not that it's looking for a null terminator - that's just how ELF files (and C in general) work! The problem is that we're reading memory at an arbitrary address.
If we did the maths right, the delf::Addr
in question points to a valid
memory-mapped region, and if our ELF file is valid, and a bunch of other ifs,
then we read a symbol name, and everything is fine.
But... if somehow a Name
outlives the MemoryMap
, then the region is
unmapped, and we end up with a SIGBUS or SIGSEGV instead - if we're lucky.
If we're not lucky, we end up reading arbitrary data, and that would be
very hard to debug.
So the problem here is that Name
is easy to mis-use and it can result in
unsafe memory access. And it's not just a theoretical problem too. I ran into
it the minute I introduced Name
into the codebase.
See, originally, I had RelocationError
like this:
// in `elk/src/process.rs` (originally) #[derive(Error, Debug)] pub enum RelocationError { // omitted: other cariants #[error("undefined symbol: {0}")] UndefinedSymbol(NamedSym), }
(Note that NamedSym
contains Name
)
And it was used like so:
// in `elk/src/process.rs` (originally) // in `fn apply_relocations` let found = match rel.sym { 0 => ResolvedSym::Undefined, _ => match self.lookup_symbol(&wanted, ignore_self) { undef @ ResolvedSym::Undefined => match wanted.sym.sym.bind { delf::SymBind::Weak => undef, // here: _ => return Err(RelocationError::UndefinedSymbol(wanted.sym.clone())), }, x => x, }, };
Seems all good, right? Our error type has rich information (a NamedSym
),
which is cloned so we can return it without worrying about ownership.
It should work fine?
Except apply_relocations
is called from cmd_run
, like so:
// in `elk/src/main.rs` fn cmd_run(args: RunArgs) -> Result<(), Box<dyn Error>> { let mut proc = process::Process::new(); let exec_index = proc.load_object_and_dependencies(args.exec_path)?; // right here proc.apply_relocations()?; proc.adjust_protections()?; let exec_obj = &proc.objects[exec_index]; let entry_point = exec_obj.file.entry_point + exec_obj.base; unsafe { jmp(entry_point.as_ptr()) }; Ok(()) }
So, within cmd_run
, everything is fine - Process
is in scope, it owns
Object
, which owns NamedSym
and Segment
, which in turns owns the
MemoryMap
:
This article is part 10 of the Making our own executable packer series.
If you liked what you saw, please support my work!