From ff085572239ea5e5eeaa7d0c62192b70f4305278 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Wed, 29 Jul 2026 12:53:28 +0000 Subject: [PATCH 01/14] feat(got-hook): add DT_HASH fallback, relocation type guard, and hook_symbol - DT_HASH (sysv) fallback: from_phdr no longer requires DT_GNU_HASH. Objects linked with --hash-style=sysv are now parsed by reading nchain from the sysv hash header. - elf64_r_type + is_got_pointer_reloc: only patch GLOB_DAT and JUMP_SLOT relocations. Skips non-pointer relocation types that would corrupt adjacent code/data if overwritten as 8-byte pointer slots. - hook_symbol: single-symbol convenience wrapper that composes dlsym, iterate_libraries, and patch_got_entries into one call. --- libdd-gotter/src/elf.rs | 475 +++++++++++++++++++++++-- libdd-gotter/src/lib.rs | 4 +- libdd-profiling-heap-gotter/src/elf.rs | 8 +- 3 files changed, 450 insertions(+), 37 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index c3fb336620..513cc6c4e0 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -5,8 +5,7 @@ //! //! Scope: //! * 64-bit Linux ELF only (`Elf64_*`). -//! * GNU hash tables only (`DT_GNU_HASH`). `DT_HASH` is not parsed; objects without a GNU hash -//! table are skipped. +//! * Supports `DT_GNU_HASH` and falls back to `DT_HASH` (sysv) for determining dynsym entry count. //! * REL / RELA / JMPREL relocation arrays. use core::ffi::{c_char, c_int, c_void, CStr}; @@ -27,6 +26,7 @@ struct Elf64_Dyn { d_un: u64, // d_val / d_ptr union; we only ever read it as u64 } const DT_NULL: i64 = 0; +const DT_HASH: i64 = 4; const DT_STRTAB: i64 = 5; const DT_SYMTAB: i64 = 6; const DT_RELA: i64 = 7; @@ -47,6 +47,8 @@ pub struct DynamicInfo { strtab_size: usize, symtab: *const Elf64_Sym, sym_count: u32, + /// Pointer and word-count for the `.gnu.hash` table, if present. + /// Used by [`gnu_hash_lookup`] for symbol resolution. gnu_hash: *const u32, gnu_hash_words: usize, rels: *const Elf64_Rel, @@ -65,6 +67,10 @@ impl DynamicInfo { /// addresses in DT entries while musl stores load-relative offsets; /// we use the `addr > base ? addr : base + addr` heuristic. /// + /// Supports both `DT_GNU_HASH` and `DT_HASH` (sysv) for determining + /// the dynsym entry count. Objects with neither hash table fall back + /// to a symtab/strtab distance heuristic. + /// /// # Safety /// `info` must point to a valid `dl_phdr_info` from `dl_iterate_phdr`. pub unsafe fn from_phdr(info: &dl_phdr_info) -> Option { @@ -98,6 +104,7 @@ impl DynamicInfo { let mut jmprels: *const Elf64_Rela = core::ptr::null(); let mut jmprels_size: usize = 0; let mut gnu_hash: *const u32 = core::ptr::null(); + let mut sysv_hash: *const u32 = core::ptr::null(); let mut pltrel_type: i64 = 0; let mut it = dyn_begin; @@ -112,6 +119,7 @@ impl DynamicInfo { DT_STRSZ => strtab_size = v as usize, DT_SYMTAB => symtab = correct(v) as *const Elf64_Sym, DT_GNU_HASH => gnu_hash = correct(v) as *const u32, + DT_HASH => sysv_hash = correct(v) as *const u32, DT_REL => rels = correct(v) as *const Elf64_Rel, DT_RELA => relas = correct(v) as *const Elf64_Rela, DT_JMPREL => jmprels = correct(v) as *const Elf64_Rela, @@ -130,32 +138,28 @@ impl DynamicInfo { jmprels_size = 0; } - if strtab.is_null() || symtab.is_null() || gnu_hash.is_null() { + // Need at minimum strtab + symtab to resolve relocation symbol names. + if strtab.is_null() || symtab.is_null() { return None; } - let gnu_hash_addr = gnu_hash as usize; - let end = containing_load_segment_end(gnu_hash_addr)?; - let bytes = end.checked_sub(gnu_hash_addr)?; - let gnu_hash_words = bytes / core::mem::size_of::(); - let sym_count = gnu_hash_symbol_count(gnu_hash, gnu_hash_words).unwrap_or_else(|| { - // Fallback for degenerate .gnu.hash (e.g. executables with only - // undefined imports): estimate dynsym entry count from the common - // .dynsym-before-.dynstr layout. This is a heuristic, not an ELF - // guarantee. If it underestimates we may skip patching some - // relocations; valid relocation indexes should still keep an - // overestimate from faulting on normal loaded objects. - let symtab_addr = symtab as usize; - let strtab_addr = strtab as usize; - if strtab_addr > symtab_addr { - let bytes = strtab_addr - symtab_addr; - (bytes / core::mem::size_of::()) as u32 + // Determine sym_count and gnu_hash metadata. + let (sym_count, gnu_hash_words) = if !gnu_hash.is_null() { + let gnu_hash_addr = gnu_hash as usize; + if let Some(end) = containing_load_segment_end(gnu_hash_addr) { + let bytes = end.saturating_sub(gnu_hash_addr); + let words = bytes / core::mem::size_of::(); + if let Some(count) = gnu_hash_symbol_count(gnu_hash, words) { + (count, words) + } else { + (sym_count_fallback(symtab, strtab, sysv_hash), words) + } } else { - // Can't estimate; allow any index and rely on strtab - // bounds checking in sym_name to catch bad accesses. - u32::MAX + (sym_count_fallback(symtab, strtab, sysv_hash), 0) } - }); + } else { + (sym_count_fallback(symtab, strtab, sysv_hash), 0) + }; Some(Self { strtab, @@ -234,6 +238,63 @@ impl DynamicInfo { unsafe { core::slice::from_raw_parts(self.jmprels, self.jmprels_count) } } } + + /// Linear scan of the dynsym table for a symbol by name. + /// + /// This is the fallback for objects that have `DT_HASH` but no + /// `DT_GNU_HASH`. The scan is bounded by `sym_count` (this is calc from + /// `DT_HASH` nchain or the symtab/strtab distance heuristic). + /// + /// # Safety + /// The `DynamicInfo` must have been produced by [`DynamicInfo::from_phdr`] + /// for a currently-loaded ELF object. + pub unsafe fn linear_sym_lookup(&self, name: &[u8]) -> Option { + for idx in 0..self.sym_count { + let sym = &*self.symtab.add(idx as usize); + let off = sym.st_name as usize; + if off >= self.strtab_size { + continue; + } + let sname = CStr::from_ptr(self.strtab.add(off)); + if sname.to_bytes() == name && check_sym(sym) { + return Some(*sym); + } + } + None + } + + /// Whether this object has a usable GNU hash table. + pub fn has_gnu_hash(&self) -> bool { + !self.gnu_hash.is_null() && self.gnu_hash_words >= 4 + } +} + +/// Fallback sym_count determination: try sysv DT_HASH, then +/// symtab/strtab distance heuristic. +unsafe fn sym_count_fallback( + symtab: *const Elf64_Sym, + strtab: *const c_char, + sysv_hash: *const u32, +) -> u32 { + // DT_HASH (sysv): header is [nbucket, nchain]. nchain == dynsym count. + if !sysv_hash.is_null() { + let nchain = *sysv_hash.add(1); + if nchain > 0 { + return nchain; + } + } + + // Last resort: estimate from the common .dynsym-before-.dynstr layout. + let symtab_addr = symtab as usize; + let strtab_addr = strtab as usize; + if strtab_addr > symtab_addr { + let bytes = strtab_addr - symtab_addr; + (bytes / core::mem::size_of::()) as u32 + } else { + // Can't estimate; allow any index and rely on strtab bounds + // checking in sym_name to catch bad accesses. + u32::MAX + } } /// Compute the GNU symbol hash used by `DT_GNU_HASH` tables. @@ -480,7 +541,6 @@ pub fn read_proc_maps() -> Vec { pub struct PageProtGuard { page_size: usize, maps: Vec, - // Aligned page base -> original prot flags read from /proc/self/maps. touched: HashMap, } @@ -513,10 +573,6 @@ impl PageProtGuard { pub unsafe fn override_entry(&mut self, addr: usize, new_value: usize) -> bool { let aligned = addr & !(self.page_size - 1); if !self.touched.contains_key(&aligned) { - // If /proc/self/maps isn't available (or the page isn't in - // it, which shouldn't happen for a mapped GOT page) fall - // back to PROT_READ - the RELRO'd default. That's tighter - // than the previous behavior of leaving pages RW. let orig = self.original_prot(aligned).unwrap_or(PROT_READ); if mprotect( aligned as *mut c_void, @@ -557,15 +613,48 @@ pub fn elf64_r_sym(info: u64) -> u64 { info >> 32 } -/// Result of a symbol lookup. -#[derive(Clone, Copy)] -pub struct LookupResult { - pub address: usize, +/// Extract the relocation type from an ELF64 relocation's `r_info` field. +pub fn elf64_r_type(info: u64) -> u32 { + (info & 0xffff_ffff) as u32 +} + +/// Return whether the relocation type represents a pointer-width slot +/// that is safe to overwrite with a function pointer. +/// +/// Accepted types: +/// - `GLOB_DAT` / `JUMP_SLOT` -- GOT entries filled by the dynamic linker. +/// - `R_X86_64_64` / `R_AARCH64_ABS64` -- absolute pointer-width relocations used for data-section +/// function pointers (`void *(*fn)(size_t) = malloc;`). +/// +/// Narrow or PC-relative types (`R_X86_64_PC32`, `R_AARCH64_TLSDESC`, etc.) +/// are excluded since they have different widths and addend semantics +pub fn is_got_pointer_reloc(r_type: u32) -> bool { + // x86_64 + const R_X86_64_64: u32 = 1; + const R_X86_64_GLOB_DAT: u32 = 6; + const R_X86_64_JUMP_SLOT: u32 = 7; + // aarch64 + const R_AARCH64_ABS64: u32 = 257; + const R_AARCH64_GLOB_DAT: u32 = 1025; + const R_AARCH64_JUMP_SLOT: u32 = 1026; + + matches!( + r_type, + R_X86_64_64 + | R_X86_64_GLOB_DAT + | R_X86_64_JUMP_SLOT + | R_AARCH64_ABS64 + | R_AARCH64_GLOB_DAT + | R_AARCH64_JUMP_SLOT + ) } /// Look up a symbol across loaded objects, returning the first /// non-zero-sized definition whose address is not `not_this_symbol`. -/// Null-sized symbols are ignored so hooks resolve to callable definitions. +/// +/// Uses `gnu_hash_lookup` for objects with `DT_GNU_HASH`, and falls +/// back to a bounded linear dynsym scan for objects that only have +/// `DT_HASH` (sysv). pub fn lookup_symbol(name: &str, not_this_symbol: usize) -> Option { let needle = name.as_bytes(); let mut found: Option = None; @@ -581,7 +670,14 @@ pub fn lookup_symbol(name: &str, not_this_symbol: usize) -> Option let Some(dyn_info) = DynamicInfo::from_phdr(info) else { return false; }; - if let Some(sym) = gnu_hash_lookup(&dyn_info, needle) { + // Try GNU hash first (O(1) average), fall back to linear scan + // for sysv-hash-only objects. + let sym = if dyn_info.has_gnu_hash() { + gnu_hash_lookup(&dyn_info, needle) + } else { + dyn_info.linear_sym_lookup(needle) + }; + if let Some(sym) = sym { if sym.st_size > 0 { let addr = sym.st_value as usize + dyn_info.base_address(); if addr != not_this_symbol { @@ -595,6 +691,126 @@ pub fn lookup_symbol(name: &str, not_this_symbol: usize) -> Option found } +/// Result of a symbol lookup. +#[derive(Clone, Copy)] +pub struct LookupResult { + pub address: usize, +} + +/// Hook a single symbol across all loaded ELF objects by patching their +/// GOT entries. +/// +/// - `symbol_name`: the symbol to hook (`c"__assert_fail"`) +/// - `hook_fn`: address of the replacement function +/// - `orig_out`: on success, receives the address of the original symbol +/// +/// Returns `true` if at least one GOT entry was patched. +/// +/// # Safety +/// +/// `hook_fn` must point to a function with the same calling convention +/// and signature as the symbol being hooked. The patching is permanent. +pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usize) -> bool { + let symbol_name_bytes = symbol_name.to_bytes(); + let name_str = match symbol_name.to_str() { + Ok(s) => s, + Err(_) => return false, + }; + + let Some(result) = lookup_symbol(name_str, hook_fn) else { + return false; + }; + *orig_out = result.address; + + let mut patched_any = false; + let mut guard = PageProtGuard::new(); + + let guard_ptr = &mut guard as *mut PageProtGuard; + let patched_ptr = &mut patched_any as *mut bool; + + iterate_libraries(|info, _is_exe| { + let lib_name = if info.dlpi_name.is_null() { + "" + } else { + unsafe { CStr::from_ptr(info.dlpi_name) } + .to_str() + .unwrap_or("") + }; + if lib_name.contains("linux-vdso") || lib_name.contains("/ld-linux") { + return false; + } + let Some(dyn_info) = (unsafe { DynamicInfo::from_phdr(info) }) else { + return false; + }; + unsafe { + patch_got_entries( + &dyn_info, + symbol_name_bytes, + hook_fn, + &mut *guard_ptr, + &mut *patched_ptr, + ); + } + false + }); + + patched_any +} + +/// Patch GOT entries in one library for the target symbol. +/// +/// Only patches relocations of type `GLOB_DAT` or `JUMP_SLOT` — the +/// pointer-sized GOT slots that the dynamic linker fills with resolved +/// symbol addresses. Other relocation types (e.g. `R_X86_64_PC32`) have +/// different widths or addend semantics and are skipped. +unsafe fn patch_got_entries( + dyn_info: &DynamicInfo, + symbol_name: &[u8], + hook_fn: usize, + guard: &mut PageProtGuard, + patched: &mut bool, +) { + let (rels_ptr, rels_count) = dyn_info.rels(); + if !rels_ptr.is_null() { + let relocs = core::slice::from_raw_parts(rels_ptr, rels_count); + for reloc in relocs { + if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { + continue; + } + let sym_idx = elf64_r_sym(reloc.r_info) as u32; + if let Some(cstr) = dyn_info.sym_name(sym_idx) { + if cstr.to_bytes() == symbol_name { + let addr = reloc.r_offset as usize + dyn_info.base_address(); + if guard.override_entry(addr, hook_fn) { + *patched = true; + } + } + } + } + } + + for (ptr, count) in [dyn_info.relas(), dyn_info.jmprels()] { + if ptr.is_null() { + continue; + } + let relocs = core::slice::from_raw_parts(ptr, count); + for reloc in relocs { + if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { + continue; + } + let sym_idx = elf64_r_sym(reloc.r_info) as u32; + if let Some(cstr) = dyn_info.sym_name(sym_idx) { + if cstr.to_bytes() == symbol_name { + let addr = reloc.r_offset as usize + dyn_info.base_address(); + if guard.override_entry(addr, hook_fn) { + *patched = true; + } + } + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -624,6 +840,83 @@ mod tests { assert_eq!(guard.original_prot(0x3000), None); } + #[test] + fn test_gnu_hash_symbol_count_too_small() { + let data: [u32; 3] = [0; 3]; + assert_eq!( + unsafe { gnu_hash_symbol_count(data.as_ptr(), data.len()) }, + None + ); + } + + #[test] + fn test_gnu_hash_symbol_count_zero_buckets() { + let data: [u32; 6] = [0, 0, 1, 0, 0, 0]; + assert_eq!( + unsafe { gnu_hash_symbol_count(data.as_ptr(), data.len()) }, + None, + ); + } + + #[test] + fn test_gnu_hash_symbol_count_valid_single_chain() { + // nbuckets=1, symbias=1, bloom_size=1 (2 u32 words), bloom_shift=0 + // bloom: [0, 0], bucket: [1], chain: [1 (LSB set = end)] + // → sym_count = 1 + 1 = 2 + let data: [u32; 8] = [1, 1, 1, 0, 0, 0, 1, 1]; + assert_eq!( + unsafe { gnu_hash_symbol_count(data.as_ptr(), data.len()) }, + Some(2), + ); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_iterate_libraries_finds_loaded_objects() { + let mut count = 0usize; + iterate_libraries(|_info, _is_exe| { + count += 1; + false + }); + assert!(count > 0); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_dynamic_info_parses_loaded_library() { + let mut found = false; + iterate_libraries(|info, _| { + if let Some(_dyn_info) = unsafe { DynamicInfo::from_phdr(info) } { + found = true; + return true; + } + false + }); + assert!(found, "should parse at least one loaded library"); + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_all_loaded_libraries_have_valid_sym_count() { + iterate_libraries(|info, _| { + let Some(dyn_info) = (unsafe { DynamicInfo::from_phdr(info) }) else { + return false; + }; + assert!( + dyn_info.sym_count > 0, + "sym_count should be > 0 (base=0x{:x})", + dyn_info.base_address + ); + let name = unsafe { dyn_info.sym_name(0) }; + assert!( + name.is_some(), + "sym_name(0) should succeed (base=0x{:x})", + dyn_info.base_address + ); + false + }); + } + #[test] #[cfg_attr(miri, ignore)] // miri doesn't support dl_iterate_phdr fn test_can_lookup_known_symbol() { @@ -671,4 +964,118 @@ mod tests { "expected at least one readable mapping" ); } + + /// Compile a tiny shared library with `--hash-style=sysv` (no + /// DT_GNU_HASH), dlopen it, and verify that `DynamicInfo::from_phdr` + /// parses it with a valid sym_count calculated from DT_HASH nchain. + #[test] + #[cfg_attr(miri, ignore)] + fn test_sysv_hash_library_parsed_correctly() { + use std::io::Write; + use std::process::Command; + + let dir = std::env::temp_dir().join("libdd_got_hook_test_sysv"); + let _ = std::fs::create_dir_all(&dir); + let c_path = dir.join("sysv_test.c"); + let so_path = dir.join("libsysv_test.so"); + + { + let mut f = std::fs::File::create(&c_path).expect("create .c"); + f.write_all(b"int sysv_test_symbol(void) { return 42; }\n") + .expect("write .c"); + } + + // Compile with --hash-style=sysv so the .so has DT_HASH but no + // DT_GNU_HASH. + let status = Command::new("cc") + .args(["-shared", "-fPIC", "-Wl,--hash-style=sysv", "-o"]) + .arg(&so_path) + .arg(&c_path) + .status(); + + let status = match status { + Ok(s) => s, + Err(e) => { + eprintln!("note: cc not available ({e}), skipping sysv hash test"); + let _ = std::fs::remove_dir_all(&dir); + return; + } + }; + if !status.success() { + eprintln!("note: cc --hash-style=sysv failed, skipping"); + let _ = std::fs::remove_dir_all(&dir); + return; + } + + // dlopen the library. + let so_cstr = + std::ffi::CString::new(so_path.to_str().expect("path is utf8")).expect("CString"); + let handle = unsafe { libc::dlopen(so_cstr.as_ptr(), libc::RTLD_NOW) }; + assert!(!handle.is_null(), "dlopen failed: {:?}", unsafe { + CStr::from_ptr(libc::dlerror()) + },); + + // Walk loaded libraries and find our .so. + let mut found = false; + iterate_libraries(|info, _| { + let lib_name = if info.dlpi_name.is_null() { + return false; + } else { + unsafe { CStr::from_ptr(info.dlpi_name) } + .to_str() + .unwrap_or("") + }; + if !lib_name.contains("libsysv_test") { + return false; + } + + let dyn_info = unsafe { DynamicInfo::from_phdr(info) }; + assert!( + dyn_info.is_some(), + "from_phdr should succeed for sysv-hash library at {lib_name}" + ); + let dyn_info = dyn_info.unwrap(); + assert!( + dyn_info.sym_count > 0, + "sym_count should be > 0 for sysv-hash library" + ); + + // Verify we can look up our exported symbol by walking + // relocations isn't needed + let mut found_sym = false; + for idx in 0..dyn_info.sym_count { + if let Some(name) = unsafe { dyn_info.sym_name(idx) } { + if name.to_bytes() == b"sysv_test_symbol" { + found_sym = true; + break; + } + } + } + assert!( + found_sym, + "should find sysv_test_symbol in dynsym (sym_count={})", + dyn_info.sym_count + ); + + found = true; + true + }); + + assert!( + found, + "should have found libsysv_test.so in loaded libraries" + ); + + // Verify that lookup_symbol (which uses the linear fallback for + // sysv-hash-only objects) can resolve the exported symbol. + let result = lookup_symbol("sysv_test_symbol", 0); + assert!( + result.is_some(), + "lookup_symbol should find sysv_test_symbol via linear dynsym scan" + ); + assert!(result.unwrap().address != 0); + + unsafe { libc::dlclose(handle) }; + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/libdd-gotter/src/lib.rs b/libdd-gotter/src/lib.rs index a7e8637260..4e2ed00996 100644 --- a/libdd-gotter/src/lib.rs +++ b/libdd-gotter/src/lib.rs @@ -9,8 +9,8 @@ //! //! Scope: //! * 64-bit Linux ELF only (`Elf64_*`). Other targets are compile-time gated. -//! * `DT_GNU_HASH` for determining dynsym entry count, with a symtab/strtab distance heuristic -//! fallback. +//! * Supports both `DT_GNU_HASH` and `DT_HASH` (sysv) for determining dynsym entry count. Objects +//! with neither fall back to a symtab/strtab distance heuristic. //! * REL / RELA / JMPREL relocation arrays. #[cfg(all(target_os = "linux", target_pointer_width = "64"))] diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index 1e6d9291f9..8bc4af131d 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -12,7 +12,9 @@ use std::ffi::CStr; use std::sync::atomic::{AtomicUsize, Ordering}; pub use libdd_gotter::lookup_symbol; -use libdd_gotter::{elf64_r_sym, iterate_libraries, DynamicInfo, PageProtGuard}; +use libdd_gotter::{ + elf64_r_sym, elf64_r_type, is_got_pointer_reloc, iterate_libraries, DynamicInfo, PageProtGuard, +}; /// Per-library bookkeeping for the GOT re-scan. We never un-patch (see /// the crate docs on why un-installing can't be done safely), so this @@ -144,6 +146,7 @@ impl SymbolOverrides { // /proc/self/maps via PageProtGuard even if only one new object needs // patching. Track already-processed libraries and lazily create the // page-protection guard to avoid repeated heavy work. + let mut guard = PageProtGuard::new(); // SAFETY: closure runs synchronously inside dl_iterate_phdr. @@ -233,6 +236,9 @@ impl SymbolOverrides { } for relocs in [dyn_info.relas(), dyn_info.jmprels()] { for reloc in relocs { + if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { + continue; + } Self::process_relocation( &self.overrides, dyn_info, From a6c8dac3b93eda56000feb2af297cd9d3310e3c5 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Fri, 31 Jul 2026 19:05:02 +0000 Subject: [PATCH 02/14] Restore/improve comments, cleaner type casting, unsafe comments --- libdd-gotter/src/elf.rs | 184 ++++++++++++++++--------- libdd-profiling-heap-gotter/src/elf.rs | 4 +- 2 files changed, 124 insertions(+), 64 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 513cc6c4e0..dca52f1f68 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -9,6 +9,7 @@ //! * REL / RELA / JMPREL relocation arrays. use core::ffi::{c_char, c_int, c_void, CStr}; +use core::slice; use std::collections::HashMap; use std::io::{BufRead, BufReader}; @@ -51,6 +52,9 @@ pub struct DynamicInfo { /// Used by [`gnu_hash_lookup`] for symbol resolution. gnu_hash: *const u32, gnu_hash_words: usize, + /// Pointer to the `DT_HASH` (sysv) table, if present. + /// Used by [`sysv_hash_lookup`] as a fallback when `DT_GNU_HASH` is absent. + sysv_hash: *const u32, rels: *const Elf64_Rel, rels_count: usize, relas: *const Elf64_Rela, @@ -74,7 +78,7 @@ impl DynamicInfo { /// # Safety /// `info` must point to a valid `dl_phdr_info` from `dl_iterate_phdr`. pub unsafe fn from_phdr(info: &dl_phdr_info) -> Option { - let phdrs = core::slice::from_raw_parts(info.dlpi_phdr, info.dlpi_phnum as usize); + let phdrs = slice::from_raw_parts(info.dlpi_phdr, info.dlpi_phnum as usize); let dyn_phdr = phdrs.iter().find(|p| p.p_type == PT_DYNAMIC)?; let dyn_begin = (info.dlpi_addr as usize + dyn_phdr.p_vaddr as usize) as *const Elf64_Dyn; let base = info.dlpi_addr as usize; @@ -168,6 +172,7 @@ impl DynamicInfo { sym_count, gnu_hash, gnu_hash_words, + sysv_hash, rels, rels_count: rels_size / core::mem::size_of::(), relas, @@ -213,7 +218,7 @@ impl DynamicInfo { // entries of a mapped ELF object; the array is valid for the // object's lifetime (held by dl_iterate_phdr's loader lock or by // the caller's dlopen handle). - unsafe { core::slice::from_raw_parts(self.rels, self.rels_count) } + unsafe { slice::from_raw_parts(self.rels, self.rels_count) } } } @@ -239,34 +244,15 @@ impl DynamicInfo { } } - /// Linear scan of the dynsym table for a symbol by name. - /// - /// This is the fallback for objects that have `DT_HASH` but no - /// `DT_GNU_HASH`. The scan is bounded by `sym_count` (this is calc from - /// `DT_HASH` nchain or the symtab/strtab distance heuristic). - /// - /// # Safety - /// The `DynamicInfo` must have been produced by [`DynamicInfo::from_phdr`] - /// for a currently-loaded ELF object. - pub unsafe fn linear_sym_lookup(&self, name: &[u8]) -> Option { - for idx in 0..self.sym_count { - let sym = &*self.symtab.add(idx as usize); - let off = sym.st_name as usize; - if off >= self.strtab_size { - continue; - } - let sname = CStr::from_ptr(self.strtab.add(off)); - if sname.to_bytes() == name && check_sym(sym) { - return Some(*sym); - } - } - None - } - /// Whether this object has a usable GNU hash table. pub fn has_gnu_hash(&self) -> bool { !self.gnu_hash.is_null() && self.gnu_hash_words >= 4 } + + /// Whether this object has a usable sysv (`DT_HASH`) hash table. + pub fn has_sysv_hash(&self) -> bool { + !self.sysv_hash.is_null() + } } /// Fallback sym_count determination: try sysv DT_HASH, then @@ -297,6 +283,62 @@ unsafe fn sym_count_fallback( } } +/// Compute the ELF sysv hash used by `DT_HASH` tables. +/// From +pub fn sysv_hash(name: &[u8]) -> u32 { + let mut h: u32 = 0; + for &c in name { + h = (h << 4).wrapping_add(c as u32); + let g = h & 0xf000_0000; + if g != 0 { + h ^= g >> 24; + } + h &= !g; + } + h +} + +/// Look up a symbol by name in an object's `DT_HASH` (sysv) table. +/// +/// Returns the `Elf64_Sym` entry if found and valid (per [`check_sym`]). +/// +/// # Safety +/// `info` must have been produced by [`DynamicInfo::from_phdr`] for a +/// currently-loaded ELF object. +pub unsafe fn sysv_hash_lookup(info: &DynamicInfo, name: &[u8]) -> Option { + let hashtab = info.sysv_hash; + if hashtab.is_null() { + return None; + } + + let nbucket = *hashtab; + // nchain at hashtab[1] == sym_count, already validated by from_phdr + if nbucket == 0 { + return None; + } + + let buckets = hashtab.add(2); + let chains = buckets.add(nbucket as usize); + + let h = sysv_hash(name); + let mut idx = *buckets.add((h % nbucket) as usize); + + // Walk the chain. Guard with sym_count to avoid infinite loops on + // malformed tables + let mut steps = 0u32; + while idx != STN_UNDEF && steps < info.sym_count { + if let Some(sname) = info.sym_name(idx) { + let sym = &*info.symtab.add(idx as usize); + if sname.to_bytes() == name && check_sym(sym) { + return Some(*sym); + } + } + idx = *chains.add(idx as usize); + steps += 1; + } + None +} + /// Compute the GNU symbol hash used by `DT_GNU_HASH` tables. /// See . pub fn gnu_hash(name: &[u8]) -> u32 { @@ -335,7 +377,7 @@ pub unsafe fn gnu_hash_symbol_count(hashtab: *const u32, hashtab_words: usize) - return None; } - let buckets = core::slice::from_raw_parts(hashtab.add(buckets_start), nbuckets as usize); + let buckets = slice::from_raw_parts(hashtab.add(buckets_start), nbuckets as usize); let mut idx = *buckets.iter().max()?; // All buckets empty: hash covers zero defined symbols, but the // symtab may still have undefined imports. Signal the caller to @@ -573,6 +615,10 @@ impl PageProtGuard { pub unsafe fn override_entry(&mut self, addr: usize, new_value: usize) -> bool { let aligned = addr & !(self.page_size - 1); if !self.touched.contains_key(&aligned) { + // If /proc/self/maps isn't available (or the page isn't in + // it, which shouldn't happen for a mapped GOT page) fall + // back to PROT_READ - the RELRO'd default. That's tighter + // than the previous behavior of leaving pages RW. let orig = self.original_prot(aligned).unwrap_or(PROT_READ); if mprotect( aligned as *mut c_void, @@ -609,8 +655,8 @@ impl Drop for PageProtGuard { } /// Extract the symbol index from an ELF64 relocation's `r_info` field. -pub fn elf64_r_sym(info: u64) -> u64 { - info >> 32 +pub fn elf64_r_sym(info: u64) -> u32 { + (info >> 32) as u32 } /// Extract the relocation type from an ELF64 relocation's `r_info` field. @@ -670,12 +716,12 @@ pub fn lookup_symbol(name: &str, not_this_symbol: usize) -> Option let Some(dyn_info) = DynamicInfo::from_phdr(info) else { return false; }; - // Try GNU hash first (O(1) average), fall back to linear scan - // for sysv-hash-only objects. let sym = if dyn_info.has_gnu_hash() { gnu_hash_lookup(&dyn_info, needle) + } else if dyn_info.has_sysv_hash() { + sysv_hash_lookup(&dyn_info, needle) } else { - dyn_info.linear_sym_lookup(needle) + None }; if let Some(sym) = sym { if sym.st_size > 0 { @@ -712,9 +758,8 @@ pub struct LookupResult { /// and signature as the symbol being hooked. The patching is permanent. pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usize) -> bool { let symbol_name_bytes = symbol_name.to_bytes(); - let name_str = match symbol_name.to_str() { - Ok(s) => s, - Err(_) => return false, + let Ok(name_str) = symbol_name.to_str() else { + return false; }; let Some(result) = lookup_symbol(name_str, hook_fn) else { @@ -739,6 +784,8 @@ pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usi if lib_name.contains("linux-vdso") || lib_name.contains("/ld-linux") { return false; } + // SAFETY: `info` points to a valid `dl_phdr_info` provided by + // `dl_iterate_phdr` let Some(dyn_info) = (unsafe { DynamicInfo::from_phdr(info) }) else { return false; }; @@ -759,10 +806,14 @@ pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usi /// Patch GOT entries in one library for the target symbol. /// -/// Only patches relocations of type `GLOB_DAT` or `JUMP_SLOT` — the -/// pointer-sized GOT slots that the dynamic linker fills with resolved -/// symbol addresses. Other relocation types (e.g. `R_X86_64_PC32`) have -/// different widths or addend semantics and are skipped. +/// Only patches relocations of type `GLOB_DAT`, `JUMP_SLOT`, or +/// pointer-width absolute (`R_X86_64_64` / `R_AARCH64_ABS64`). +/// Narrow or PC-relative relocation types are skipped. +/// +/// # Safety +/// `dyn_info` must have been produced by [`DynamicInfo::from_phdr`] for a +/// currently-loaded ELF object. `guard` must belong to the current +/// patching pass. unsafe fn patch_got_entries( dyn_info: &DynamicInfo, symbol_name: &[u8], @@ -770,35 +821,27 @@ unsafe fn patch_got_entries( guard: &mut PageProtGuard, patched: &mut bool, ) { - let (rels_ptr, rels_count) = dyn_info.rels(); - if !rels_ptr.is_null() { - let relocs = core::slice::from_raw_parts(rels_ptr, rels_count); - for reloc in relocs { - if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { - continue; - } - let sym_idx = elf64_r_sym(reloc.r_info) as u32; - if let Some(cstr) = dyn_info.sym_name(sym_idx) { - if cstr.to_bytes() == symbol_name { - let addr = reloc.r_offset as usize + dyn_info.base_address(); - if guard.override_entry(addr, hook_fn) { - *patched = true; - } + for reloc in dyn_info.rels() { + if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { + continue; + } + let sym_idx = elf64_r_sym(reloc.r_info); + if let Some(cstr) = dyn_info.sym_name(sym_idx) { + if cstr.to_bytes() == symbol_name { + let addr = reloc.r_offset as usize + dyn_info.base_address(); + if guard.override_entry(addr, hook_fn) { + *patched = true; } } } } - for (ptr, count) in [dyn_info.relas(), dyn_info.jmprels()] { - if ptr.is_null() { - continue; - } - let relocs = core::slice::from_raw_parts(ptr, count); + for relocs in [dyn_info.relas(), dyn_info.jmprels()] { for reloc in relocs { if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { continue; } - let sym_idx = elf64_r_sym(reloc.r_info) as u32; + let sym_idx = elf64_r_sym(reloc.r_info); if let Some(cstr) = dyn_info.sym_name(sym_idx) { if cstr.to_bytes() == symbol_name { let addr = reloc.r_offset as usize + dyn_info.base_address(); @@ -870,6 +913,22 @@ mod tests { ); } + #[test] + fn test_sysv_hash_known_values() { + // Reference values from the ELF spec and glibc's dl-hash.h. + // Empty string hashes to 0. + assert_eq!(sysv_hash(b""), 0); + // Verify a few known symbol names produce non-zero, distinct hashes. + let h1 = sysv_hash(b"malloc"); + let h2 = sysv_hash(b"free"); + let h3 = sysv_hash(b"__assert_fail"); + assert!(h1 != 0); + assert!(h2 != 0); + assert!(h3 != 0); + assert!(h1 != h2); + assert!(h1 != h3); + } + #[test] #[cfg_attr(miri, ignore)] fn test_iterate_libraries_finds_loaded_objects() { @@ -899,6 +958,7 @@ mod tests { #[cfg_attr(miri, ignore)] fn test_all_loaded_libraries_have_valid_sym_count() { iterate_libraries(|info, _| { + // SAFETY: `info` is a valid `dl_phdr_info` from `dl_iterate_phdr`. let Some(dyn_info) = (unsafe { DynamicInfo::from_phdr(info) }) else { return false; }; @@ -1066,12 +1126,12 @@ mod tests { "should have found libsysv_test.so in loaded libraries" ); - // Verify that lookup_symbol (which uses the linear fallback for - // sysv-hash-only objects) can resolve the exported symbol. + // Verify that lookup_symbol (which uses sysv_hash_lookup for + // DT_HASH-only objects) can resolve the exported symbol. let result = lookup_symbol("sysv_test_symbol", 0); assert!( result.is_some(), - "lookup_symbol should find sysv_test_symbol via linear dynsym scan" + "lookup_symbol should find sysv_test_symbol via sysv hash lookup" ); assert!(result.unwrap().address != 0); diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index 8bc4af131d..7e77b58de3 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -229,7 +229,7 @@ impl SymbolOverrides { Self::process_relocation( &self.overrides, dyn_info, - elf64_r_sym(reloc.r_info) as u32, + elf64_r_sym(reloc.r_info), reloc.r_offset as usize, guard, ); @@ -242,7 +242,7 @@ impl SymbolOverrides { Self::process_relocation( &self.overrides, dyn_info, - elf64_r_sym(reloc.r_info) as u32, + elf64_r_sym(reloc.r_info), reloc.r_offset as usize, guard, ); From 11650a6461e56c45a6a2365651a9f83ea9f9c583 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Mon, 3 Aug 2026 19:25:34 +0000 Subject: [PATCH 03/14] Safety comments --- libdd-gotter/src/elf.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index dca52f1f68..96637dc6ca 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -78,7 +78,8 @@ impl DynamicInfo { /// # Safety /// `info` must point to a valid `dl_phdr_info` from `dl_iterate_phdr`. pub unsafe fn from_phdr(info: &dl_phdr_info) -> Option { - let phdrs = slice::from_raw_parts(info.dlpi_phdr, info.dlpi_phnum as usize); + // SAFETY: info is valid for the lifetime of the program. + let phdrs = unsafe { slice::from_raw_parts(info.dlpi_phdr, info.dlpi_phnum as usize) }; let dyn_phdr = phdrs.iter().find(|p| p.p_type == PT_DYNAMIC)?; let dyn_begin = (info.dlpi_addr as usize + dyn_phdr.p_vaddr as usize) as *const Elf64_Dyn; let base = info.dlpi_addr as usize; @@ -229,7 +230,7 @@ impl DynamicInfo { } else { // SAFETY: same as rels(); from_phdr set relas/relas_count // from DT_RELA/DT_RELASZ of a mapped ELF object. - unsafe { core::slice::from_raw_parts(self.relas, self.relas_count) } + unsafe { slice::from_raw_parts(self.relas, self.relas_count) } } } @@ -240,7 +241,7 @@ impl DynamicInfo { } else { // SAFETY: same as rels(); from_phdr set jmprels/jmprels_count // from DT_JMPREL/DT_PLTRELSZ of a mapped ELF object. - unsafe { core::slice::from_raw_parts(self.jmprels, self.jmprels_count) } + unsafe { slice::from_raw_parts(self.jmprels, self.jmprels_count) } } } From 9f66fc7f181c96213207ce6e068058bc84bf8c8a Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Mon, 3 Aug 2026 19:33:52 +0000 Subject: [PATCH 04/14] Add process reloc guard and add comment about REL processing --- libdd-gotter/src/elf.rs | 4 ++++ libdd-profiling-heap-gotter/src/elf.rs | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 96637dc6ca..21fde1c225 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -822,6 +822,10 @@ unsafe fn patch_got_entries( guard: &mut PageProtGuard, patched: &mut bool, ) { + // NOTE: the SysV x86-64 ABI specifies that only RELA entries are + // used on AMD64 (spec page 64). ARM64 appears similar. REL + // processing is kept for defensive completeness but may be + // dead code on both architectures. We should revisit this. for reloc in dyn_info.rels() { if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { continue; diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index 7e77b58de3..44d1e5bdac 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -225,7 +225,15 @@ impl SymbolOverrides { return; } + // NOTE: the SysV x86-64 ABI: + // + // specifies that only RELA entries are used on AMD64 (spec page 64). + // ARM64 appears similar. REL processing is kept for defensive completeness + // but may be dead code on both architectures. We should revisit this. for reloc in dyn_info.rels() { + if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { + continue; + } Self::process_relocation( &self.overrides, dyn_info, From 860adc917d1374b0fd0ef5f3ae9928327ac7afd2 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Mon, 3 Aug 2026 20:06:01 +0000 Subject: [PATCH 05/14] Bounds checking --- libdd-gotter/src/elf.rs | 52 ++++++++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 21fde1c225..b74f623987 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -52,9 +52,10 @@ pub struct DynamicInfo { /// Used by [`gnu_hash_lookup`] for symbol resolution. gnu_hash: *const u32, gnu_hash_words: usize, - /// Pointer to the `DT_HASH` (sysv) table, if present. + /// Pointer and word-count for the `DT_HASH` (sysv) table, if present. /// Used by [`sysv_hash_lookup`] as a fallback when `DT_GNU_HASH` is absent. sysv_hash: *const u32, + sysv_hash_words: usize, rels: *const Elf64_Rel, rels_count: usize, relas: *const Elf64_Rela, @@ -148,6 +149,17 @@ impl DynamicInfo { return None; } + // Compute sysv_hash_words from the containing load segment. + let sysv_hash_words = if !sysv_hash.is_null() { + let addr = sysv_hash as usize; + containing_load_segment_end(addr) + .and_then(|end| end.checked_sub(addr)) + .map(|bytes| bytes / core::mem::size_of::()) + .unwrap_or(0) + } else { + 0 + }; + // Determine sym_count and gnu_hash metadata. let (sym_count, gnu_hash_words) = if !gnu_hash.is_null() { let gnu_hash_addr = gnu_hash as usize; @@ -174,6 +186,7 @@ impl DynamicInfo { gnu_hash, gnu_hash_words, sysv_hash, + sysv_hash_words, rels, rels_count: rels_size / core::mem::size_of::(), relas, @@ -301,6 +314,15 @@ pub fn sysv_hash(name: &[u8]) -> u32 { /// Look up a symbol by name in an object's `DT_HASH` (sysv) table. /// +/// [nbucket] [nchain] [bucket[0..nbucket]] [chain[0..nchain]] +/// +/// Each bucket holds the index of the first symbol in that bucket's +/// chain (or `STN_UNDEF` if empty). Each chain entry at position `i` +/// holds the index of the next symbol after symbol `i` in the same +/// bucket (or `STN_UNDEF` to end the chain). `nchain` equals the +/// total number of dynamic symbols, so chain indices double as +/// symbol indices into `.dynsym`. +/// /// Returns the `Elf64_Sym` entry if found and valid (per [`check_sym`]). /// /// # Safety @@ -308,26 +330,40 @@ pub fn sysv_hash(name: &[u8]) -> u32 { /// currently-loaded ELF object. pub unsafe fn sysv_hash_lookup(info: &DynamicInfo, name: &[u8]) -> Option { let hashtab = info.sysv_hash; - if hashtab.is_null() { + if hashtab.is_null() || info.sysv_hash_words < 2 { return None; } + // Read the header: nbucket and nchain. let nbucket = *hashtab; - // nchain at hashtab[1] == sym_count, already validated by from_phdr + let nchain = *hashtab.add(1); if nbucket == 0 { return None; } - let buckets = hashtab.add(2); - let chains = buckets.add(nbucket as usize); + // Validate the table fits within the mapped region before computing + // any pointers into the bucket/chain arrays. + let buckets_start: usize = 2; + let chains_start = buckets_start.checked_add(nbucket as usize)?; + let table_end = chains_start.checked_add(nchain as usize)?; + if table_end > info.sysv_hash_words { + return None; + } + + let buckets = hashtab.add(buckets_start); + let chains = hashtab.add(chains_start); let h = sysv_hash(name); let mut idx = *buckets.add((h % nbucket) as usize); - // Walk the chain. Guard with sym_count to avoid infinite loops on - // malformed tables + // Follow the chain from the bucket's head symbol, comparing names + // at each step. The chain terminates at STN_UNDEF (0). We also + // cap iterations at nchain to guard against malformed cycles. let mut steps = 0u32; - while idx != STN_UNDEF && steps < info.sym_count { + while idx != STN_UNDEF && steps < nchain { + if (idx as usize) >= nchain as usize { + break; + } if let Some(sname) = info.sym_name(idx) { let sym = &*info.symtab.add(idx as usize); if sname.to_bytes() == name && check_sym(sym) { From 3bd03f2923cf923af50f748fe5c7c215ba3b88b9 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Mon, 3 Aug 2026 20:13:09 +0000 Subject: [PATCH 06/14] Try patch helper fn --- libdd-gotter/src/elf.rs | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index b74f623987..85e6300ca3 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -858,39 +858,34 @@ unsafe fn patch_got_entries( guard: &mut PageProtGuard, patched: &mut bool, ) { - // NOTE: the SysV x86-64 ABI specifies that only RELA entries are - // used on AMD64 (spec page 64). ARM64 appears similar. REL - // processing is kept for defensive completeness but may be - // dead code on both architectures. We should revisit this. - for reloc in dyn_info.rels() { - if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { - continue; + // Both REL and RELA relocations carry r_info (symbol + type) and + // r_offset (GOT slot address). RELA has an additional r_addend we + // don't use. This helper processes one relocation by those two fields. + let mut try_patch = |r_info: u64, r_offset: u64| { + if !is_got_pointer_reloc(elf64_r_type(r_info)) { + return; } - let sym_idx = elf64_r_sym(reloc.r_info); + let sym_idx = elf64_r_sym(r_info); if let Some(cstr) = dyn_info.sym_name(sym_idx) { if cstr.to_bytes() == symbol_name { - let addr = reloc.r_offset as usize + dyn_info.base_address(); + let addr = r_offset as usize + dyn_info.base_address(); if guard.override_entry(addr, hook_fn) { *patched = true; } } } - } + }; + // NOTE: the SysV x86-64 ABI specifies that only RELA entries are + // used on AMD64 (spec page 64). ARM64 appears similar. REL + // processing is kept for defensive completeness but may be + // dead code on both architectures. We should revisit this. + for reloc in dyn_info.rels() { + try_patch(reloc.r_info, reloc.r_offset); + } for relocs in [dyn_info.relas(), dyn_info.jmprels()] { for reloc in relocs { - if !is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { - continue; - } - let sym_idx = elf64_r_sym(reloc.r_info); - if let Some(cstr) = dyn_info.sym_name(sym_idx) { - if cstr.to_bytes() == symbol_name { - let addr = reloc.r_offset as usize + dyn_info.base_address(); - if guard.override_entry(addr, hook_fn) { - *patched = true; - } - } - } + try_patch(reloc.r_info, reloc.r_offset); } } } From 69981ff2ffb814926a67c2512416f7b8cee63085 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Tue, 4 Aug 2026 13:15:02 +0000 Subject: [PATCH 07/14] Unit test for checking ptr size --- libdd-gotter/src/elf.rs | 58 ++++++++++++++++++++++++++ libdd-profiling-heap-gotter/src/elf.rs | 2 +- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 85e6300ca3..2164a93008 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -1174,4 +1174,62 @@ mod tests { unsafe { libc::dlclose(handle) }; let _ = std::fs::remove_dir_all(&dir); } + + #[test] + fn test_is_got_pointer_reloc_accepts_pointer_width_types() { + // x86_64 + assert!(is_got_pointer_reloc(1)); // R_X86_64_64 + assert!(is_got_pointer_reloc(6)); // R_X86_64_GLOB_DAT + assert!(is_got_pointer_reloc(7)); // R_X86_64_JUMP_SLOT + // aarch64 + assert!(is_got_pointer_reloc(257)); // R_AARCH64_ABS64 + assert!(is_got_pointer_reloc(1025)); // R_AARCH64_GLOB_DAT + assert!(is_got_pointer_reloc(1026)); // R_AARCH64_JUMP_SLOT + } + + #[test] + fn test_is_got_pointer_reloc_rejects_non_pointer_types() { + assert!(!is_got_pointer_reloc(0)); // R_*_NONE + assert!(!is_got_pointer_reloc(2)); // R_X86_64_PC32 + assert!(!is_got_pointer_reloc(10)); // R_X86_64_32 + assert!(!is_got_pointer_reloc(11)); // R_X86_64_32S + assert!(!is_got_pointer_reloc(258)); // R_AARCH64_ABS32 + assert!(!is_got_pointer_reloc(1029)); // R_AARCH64_TLSDESC + assert!(!is_got_pointer_reloc(u32::MAX)); + } + + /// Sanity check against real loaded libraries: the filter should + /// accept some relocations (GOT entries exist) and reject some + #[test] + #[cfg_attr(miri, ignore)] + fn test_is_got_pointer_reloc_filters_real_relocations() { + let mut accepted = 0usize; + let mut rejected = 0usize; + + iterate_libraries(|info, _| { + // SAFETY: `info` is a valid `dl_phdr_info` from `dl_iterate_phdr`. + let Some(dyn_info) = (unsafe { DynamicInfo::from_phdr(info) }) else { + return false; + }; + for relocs in [dyn_info.relas(), dyn_info.jmprels()] { + for reloc in relocs { + if is_got_pointer_reloc(elf64_r_type(reloc.r_info)) { + accepted += 1; + } else { + rejected += 1; + } + } + } + false + }); + + assert!( + accepted > 0, + "expected at least one GOT relocation across loaded libraries" + ); + assert!( + rejected > 0, + "expected at least one non-GOT relocation to be filtered out" + ); + } } diff --git a/libdd-profiling-heap-gotter/src/elf.rs b/libdd-profiling-heap-gotter/src/elf.rs index 44d1e5bdac..de662df35c 100644 --- a/libdd-profiling-heap-gotter/src/elf.rs +++ b/libdd-profiling-heap-gotter/src/elf.rs @@ -3,7 +3,7 @@ //! GOT-table interposition for heap profiling. //! -//! Uses the shared ELF primitives from `libdd-got-hook` for parsing and +//! Uses the shared ELF primitives from `libdd-gotter` for parsing and //! patching, and adds the multi-symbol `SymbolOverrides` registry with //! per-library dedup and dlopen rescan support on top. From f19c984c13243fcd3175251c94244e4d64d17aa0 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Tue, 4 Aug 2026 14:35:23 +0000 Subject: [PATCH 08/14] Hard fail on compile fail --- Cargo.lock | 1 + libdd-gotter/Cargo.toml | 3 +++ libdd-gotter/src/elf.rs | 27 +++++++++------------------ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c9dd90cd8..aabdd451c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3116,6 +3116,7 @@ name = "libdd-gotter" version = "0.1.0" dependencies = [ "libc", + "tempfile", ] [[package]] diff --git a/libdd-gotter/Cargo.toml b/libdd-gotter/Cargo.toml index 8df14a0352..4bd0f36a8d 100644 --- a/libdd-gotter/Cargo.toml +++ b/libdd-gotter/Cargo.toml @@ -16,3 +16,6 @@ bench = false [dependencies] libc.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 2164a93008..4d2bd26884 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -893,7 +893,7 @@ unsafe fn patch_got_entries( #[cfg(test)] mod tests { use super::*; - + use tempfile::TempDir; #[test] fn page_prot_guard_finds_original_mapping_protection() { let guard = PageProtGuard { @@ -1070,10 +1070,9 @@ mod tests { use std::io::Write; use std::process::Command; - let dir = std::env::temp_dir().join("libdd_got_hook_test_sysv"); - let _ = std::fs::create_dir_all(&dir); - let c_path = dir.join("sysv_test.c"); - let so_path = dir.join("libsysv_test.so"); + let dir = TempDir::new().expect("create temp dir"); + let c_path = dir.path().join("sysv_test.c"); + let so_path = dir.path().join("libsysv_test.so"); { let mut f = std::fs::File::create(&c_path).expect("create .c"); @@ -1089,19 +1088,11 @@ mod tests { .arg(&c_path) .status(); - let status = match status { - Ok(s) => s, - Err(e) => { - eprintln!("note: cc not available ({e}), skipping sysv hash test"); - let _ = std::fs::remove_dir_all(&dir); - return; - } - }; - if !status.success() { - eprintln!("note: cc --hash-style=sysv failed, skipping"); - let _ = std::fs::remove_dir_all(&dir); - return; - } + let status = status.expect("cc should be available"); + assert!( + status.success(), + "cc --hash-style=sysv compilation failed: {status}" + ); // dlopen the library. let so_cstr = From 4b7aa6b3be989e2ce5692c9a927585874ed35f8a Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Tue, 4 Aug 2026 17:42:36 +0000 Subject: [PATCH 09/14] Safety comments --- libdd-gotter/src/elf.rs | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 4d2bd26884..a9bf4eb9d1 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -271,6 +271,10 @@ impl DynamicInfo { /// Fallback sym_count determination: try sysv DT_HASH, then /// symtab/strtab distance heuristic. +/// +/// # Safety +/// All non-null pointers must point into the `PT_DYNAMIC` segment of a +/// currently-loaded ELF object (as produced by [`DynamicInfo::from_phdr`]). unsafe fn sym_count_fallback( symtab: *const Elf64_Sym, strtab: *const c_char, @@ -557,6 +561,10 @@ pub fn iterate_libraries(mut callback: impl FnMut(&dl_phdr_info, bool) -> bool) result.map(i32::from).unwrap_or(1) } + // SAFETY: `trampoline` has the correct signature for dl_iterate_phdr. + // `ctx` is live for the duration of the call; the trampoline casts + // `data` back to `&mut Ctx` and catches panics to prevent unwinding + // through C frames. unsafe { dl_iterate_phdr(Some(trampoline), &mut ctx as *mut _ as *mut c_void); } @@ -620,6 +628,7 @@ pub fn read_proc_maps() -> Vec { pub struct PageProtGuard { page_size: usize, maps: Vec, + // Aligned page base -> original prot flags read from /proc/self/maps. touched: HashMap, } @@ -684,8 +693,9 @@ impl Drop for PageProtGuard { /// are never left weakened even if a patching pass bails out midway. fn drop(&mut self) { for (aligned, orig) in self.touched.drain() { - // Best-effort: nothing sensible to do on failure other than - // leave the page RW, which is the pre-fix behavior. + // SAFETY: `aligned` was a page-aligned address we successfully + // mprotect'd earlier; restoring its original protection is safe. + // Best-effort: nothing sensible to do on failure. unsafe { mprotect(aligned as *mut c_void, self.page_size, orig) }; } } @@ -734,6 +744,7 @@ pub fn is_got_pointer_reloc(r_type: u32) -> bool { /// Look up a symbol across loaded objects, returning the first /// non-zero-sized definition whose address is not `not_this_symbol`. +/// Null-sized symbols are ignored so hooks resolve to callable definitions. /// /// Uses `gnu_hash_lookup` for objects with `DT_GNU_HASH`, and falls /// back to a bounded linear dynsym scan for objects that only have @@ -741,6 +752,10 @@ pub fn is_got_pointer_reloc(r_type: u32) -> bool { pub fn lookup_symbol(name: &str, not_this_symbol: usize) -> Option { let needle = name.as_bytes(); let mut found: Option = None; + // SAFETY: the callback runs synchronously inside dl_iterate_phdr; + // `info` points to a valid dl_phdr_info for a currently-loaded + // library. CStr::from_ptr, DynamicInfo::from_phdr, and the hash + // lookups all operate on pointers from the mapped ELF object. iterate_libraries(|info, _is_exe| unsafe { let lib_name = if info.dlpi_name.is_null() { "" @@ -814,6 +829,8 @@ pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usi let lib_name = if info.dlpi_name.is_null() { "" } else { + // SAFETY: dl_iterate_phdr guarantees dlpi_name is a valid + // NUL-terminated C string for the callback's duration. unsafe { CStr::from_ptr(info.dlpi_name) } .to_str() .unwrap_or("") @@ -822,10 +839,14 @@ pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usi return false; } // SAFETY: `info` points to a valid `dl_phdr_info` provided by - // `dl_iterate_phdr` + // `dl_iterate_phdr`; the library is mapped for the callback's + // duration. let Some(dyn_info) = (unsafe { DynamicInfo::from_phdr(info) }) else { return false; }; + // SAFETY: dyn_info was just produced from a currently-loaded + // library. guard_ptr/patched_ptr are valid for the duration of + // iterate_libraries (they point to locals in the enclosing fn). unsafe { patch_got_entries( &dyn_info, From e9568ee78f2904f96167a5479a1168798913a052 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Wed, 5 Aug 2026 20:25:05 +0000 Subject: [PATCH 10/14] Make DynInfo contain prevalidated slices --- libdd-gotter/src/elf.rs | 161 +++++++++++++++++++++++----------------- 1 file changed, 92 insertions(+), 69 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index a9bf4eb9d1..22f948b9e4 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -43,11 +43,14 @@ const STN_UNDEF: u32 = 0; /// The subset of an ELF object's `PT_DYNAMIC` entries needed to find /// and rewrite GOT entries. -pub struct DynamicInfo { - strtab: *const c_char, - strtab_size: usize, - symtab: *const Elf64_Sym, - sym_count: u32, +/// +/// Slice fields (`strtab`, `symtab`, `rels`, `relas`, `jmprels`) are +/// validated once in [`from_phdr`] so all subsequent access is safe. +/// The hash table pointers remain raw because the hash lookup functions +/// need arithmetic into their variable-layout internal structure. +pub struct DynamicInfo<'a> { + strtab: &'a [u8], + symtab: &'a [Elf64_Sym], /// Pointer and word-count for the `.gnu.hash` table, if present. /// Used by [`gnu_hash_lookup`] for symbol resolution. gnu_hash: *const u32, @@ -56,16 +59,13 @@ pub struct DynamicInfo { /// Used by [`sysv_hash_lookup`] as a fallback when `DT_GNU_HASH` is absent. sysv_hash: *const u32, sysv_hash_words: usize, - rels: *const Elf64_Rel, - rels_count: usize, - relas: *const Elf64_Rela, - relas_count: usize, - jmprels: *const Elf64_Rela, - jmprels_count: usize, + rels: &'a [Elf64_Rel], + relas: &'a [Elf64_Rela], + jmprels: &'a [Elf64_Rela], base_address: usize, } -impl DynamicInfo { +impl<'a> DynamicInfo<'a> { /// Read DT_* entries out of a PT_DYNAMIC array. /// /// Handles the glibc-vs-musl quirk where glibc stores absolute @@ -78,7 +78,7 @@ impl DynamicInfo { /// /// # Safety /// `info` must point to a valid `dl_phdr_info` from `dl_iterate_phdr`. - pub unsafe fn from_phdr(info: &dl_phdr_info) -> Option { + pub unsafe fn from_phdr(info: &'a dl_phdr_info) -> Option { // SAFETY: info is valid for the lifetime of the program. let phdrs = unsafe { slice::from_raw_parts(info.dlpi_phdr, info.dlpi_phnum as usize) }; let dyn_phdr = phdrs.iter().find(|p| p.p_type == PT_DYNAMIC)?; @@ -178,41 +178,85 @@ impl DynamicInfo { (sym_count_fallback(symtab, strtab, sysv_hash), 0) }; + // SAFETY (applies to all `slice::from_raw_parts` calls below): + // + // Each pointer (strtab, symtab, rels, relas, jmprels) was read + // from the PT_DYNAMIC segment of a currently-loaded ELF object + // and corrected for the glibc/musl absolute-vs-relative address + // quirk. The dynamic linker has already validated and mapped + // these sections, so: + // - Non-null: guarded by the `is_null()` check in each branch; the null/zero-length case + // returns `&[]` without calling `from_raw_parts`. + // - Properly aligned: ELF sections are required by the spec to be aligned to their entry + // size (e.g. `Elf64_Sym` is 8-byte aligned, `Elf64_Rela` is 8-byte aligned). The dynamic + // linker enforces this at load time. + // - Valid for `len * size_of::()` reads within a single allocation: the entire section + // is mapped contiguously from the ELF file by the kernel via `mmap`. The size/count + // values come from DT_STRSZ, sym_count (from DT_GNU_HASH or DT_HASH nchain), DT_RELSZ, + // DT_RELASZ, and DT_PLTRELSZ respectively. + // - Not mutated for lifetime `'a`: the ELF object is held mapped by `dl_iterate_phdr`'s + // loader lock (for the callback's duration) or by the caller's `dlopen` handle. + // - Total size does not overflow `isize::MAX`: ELF section sizes are bounded by the + // file/mapping size, which the kernel validated at mmap time. + let strtab_slice = if strtab.is_null() || strtab_size == 0 { + &[] + } else { + slice::from_raw_parts(strtab as *const u8, strtab_size) + }; + + let rels_count = rels_size / core::mem::size_of::(); + let relas_count = relas_size / core::mem::size_of::(); + let jmprels_count = jmprels_size / core::mem::size_of::(); + + let symtab_slice = if symtab.is_null() || sym_count == 0 { + &[] + } else { + slice::from_raw_parts(symtab, sym_count as usize) + }; + let rels_slice = if rels.is_null() || rels_count == 0 { + &[] + } else { + slice::from_raw_parts(rels, rels_count) + }; + let relas_slice = if relas.is_null() || relas_count == 0 { + &[] + } else { + slice::from_raw_parts(relas, relas_count) + }; + let jmprels_slice = if jmprels.is_null() || jmprels_count == 0 { + &[] + } else { + slice::from_raw_parts(jmprels, jmprels_count) + }; + Some(Self { - strtab, - strtab_size, - symtab, - sym_count, + strtab: strtab_slice, + symtab: symtab_slice, gnu_hash, gnu_hash_words, sysv_hash, sysv_hash_words, - rels, - rels_count: rels_size / core::mem::size_of::(), - relas, - relas_count: relas_size / core::mem::size_of::(), - jmprels, - jmprels_count: jmprels_size / core::mem::size_of::(), + rels: rels_slice, + relas: relas_slice, + jmprels: jmprels_slice, base_address: base, }) } /// Look up the name of the symbol at index `idx` in the dynamic /// string table. - /// - /// # Safety - /// The `DynamicInfo` must have been produced by [`DynamicInfo::from_phdr`] - /// for a currently-loaded ELF object whose symtab/strtab are still mapped. - pub unsafe fn sym_name(&self, idx: u32) -> Option<&CStr> { - if (idx as usize) >= self.sym_count as usize { - return None; - } - let sym = &*self.symtab.add(idx as usize); + pub fn sym_name(&self, idx: u32) -> Option<&CStr> { + let sym = self.symtab.get(idx as usize)?; let off = sym.st_name as usize; - if off >= self.strtab_size { + if off >= self.strtab.len() { return None; } - Some(CStr::from_ptr(self.strtab.add(off))) + // Find the NUL terminator within the remaining strtab. + let remaining = &self.strtab[off..]; + let nul_pos = remaining.iter().position(|&b| b == 0)?; + // SAFETY: we found a NUL byte within the validated strtab slice, + // so CStr::from_bytes_with_nul won't fail. + CStr::from_bytes_with_nul(&remaining[..=nul_pos]).ok() } /// The base load address of this ELF object. @@ -221,41 +265,18 @@ impl DynamicInfo { } /// REL relocations for this object, or empty if none. - /// - /// Safe because `from_phdr` validated the pointer and count from the - /// `PT_DYNAMIC` segment of a currently-loaded ELF object. pub fn rels(&self) -> &[Elf64_Rel] { - if self.rels.is_null() || self.rels_count == 0 { - &[] - } else { - // SAFETY: from_phdr set rels/rels_count from the DT_REL/DT_RELSZ - // entries of a mapped ELF object; the array is valid for the - // object's lifetime (held by dl_iterate_phdr's loader lock or by - // the caller's dlopen handle). - unsafe { slice::from_raw_parts(self.rels, self.rels_count) } - } + self.rels } /// RELA relocations for this object, or empty if none. pub fn relas(&self) -> &[Elf64_Rela] { - if self.relas.is_null() || self.relas_count == 0 { - &[] - } else { - // SAFETY: same as rels(); from_phdr set relas/relas_count - // from DT_RELA/DT_RELASZ of a mapped ELF object. - unsafe { slice::from_raw_parts(self.relas, self.relas_count) } - } + self.relas } /// JMPREL (PLT) relocations for this object, or empty if none. pub fn jmprels(&self) -> &[Elf64_Rela] { - if self.jmprels.is_null() || self.jmprels_count == 0 { - &[] - } else { - // SAFETY: same as rels(); from_phdr set jmprels/jmprels_count - // from DT_JMPREL/DT_PLTRELSZ of a mapped ELF object. - unsafe { slice::from_raw_parts(self.jmprels, self.jmprels_count) } - } + self.jmprels } /// Whether this object has a usable GNU hash table. @@ -369,11 +390,13 @@ pub unsafe fn sysv_hash_lookup(info: &DynamicInfo, name: &[u8]) -> Option Option> 1) == 0 { if let Some(sname) = info.sym_name(symidx) { - let sym = info.symtab.add(symidx as usize); - if sname.to_bytes() == name && check_sym(&*sym) { + let sym = &info.symtab[symidx as usize]; + if sname.to_bytes() == name && check_sym(sym) { return Some(*sym); } } @@ -1020,11 +1043,11 @@ mod tests { return false; }; assert!( - dyn_info.sym_count > 0, + !dyn_info.symtab.is_empty(), "sym_count should be > 0 (base=0x{:x})", dyn_info.base_address ); - let name = unsafe { dyn_info.sym_name(0) }; + let name = dyn_info.sym_name(0); assert!( name.is_some(), "sym_name(0) should succeed (base=0x{:x})", @@ -1144,15 +1167,15 @@ mod tests { ); let dyn_info = dyn_info.unwrap(); assert!( - dyn_info.sym_count > 0, + !dyn_info.symtab.is_empty(), "sym_count should be > 0 for sysv-hash library" ); // Verify we can look up our exported symbol by walking // relocations isn't needed let mut found_sym = false; - for idx in 0..dyn_info.sym_count { - if let Some(name) = unsafe { dyn_info.sym_name(idx) } { + for idx in 0..dyn_info.symtab.len() as u32 { + if let Some(name) = dyn_info.sym_name(idx) { if name.to_bytes() == b"sysv_test_symbol" { found_sym = true; break; @@ -1162,7 +1185,7 @@ mod tests { assert!( found_sym, "should find sysv_test_symbol in dynsym (sym_count={})", - dyn_info.sym_count + dyn_info.symtab.len() ); found = true; From f1f6efb6241ead0362fd94daad7a329171f762c5 Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Fri, 7 Aug 2026 13:14:38 +0000 Subject: [PATCH 11/14] Include sym_entry api and clarify docs --- libdd-gotter/src/elf.rs | 59 ++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 22f948b9e4..218a44e931 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -3,7 +3,22 @@ //! GOT-table interposition primitives. //! -//! Scope: +//! # Choosing an API +//! +//! * [`hook_symbol`] -- one-shot, single-symbol hook. Only patches libraries that are loaded at +//! call time. Libraries `dlopen`'d afterwards will **not** have their GOT entries patched, so +//! calls to the hooked symbol from those libraries will bypass the hook. Use this when you know +//! the target symbol is already loaded and all relevant callers are already in memory (e.g. +//! crashtracker hooking `__assert_fail` during `init()`). +//! +//! * [`DynamicInfo`], [`iterate_libraries`], [`PageProtGuard`] -- the lower-level building blocks. +//! Use these when you need to hook multiple symbols, re-scan after `dlopen` for newly loaded +//! libraries, or maintain per-library bookkeeping. Example: `libdd-profiling-heap-gotter`'s +//! `SymbolOverrides` registry, which patches `malloc`/`free`/etc. and re-applies overrides +//! whenever a new library is loaded. +//! +//! # Scope +//! //! * 64-bit Linux ELF only (`Elf64_*`). //! * Supports `DT_GNU_HASH` and falls back to `DT_HASH` (sysv) for determining dynsym entry count. //! * REL / RELA / JMPREL relocation arrays. @@ -243,20 +258,22 @@ impl<'a> DynamicInfo<'a> { }) } - /// Look up the name of the symbol at index `idx` in the dynamic - /// string table. - pub fn sym_name(&self, idx: u32) -> Option<&CStr> { + /// Look up the symbol entry and its name at index `idx`. + /// Returns the `Elf64_Sym` and its name from the string table, + /// or `None` if the index is out of bounds or the name is invalid. + pub fn sym_entry(&self, idx: u32) -> Option<(&Elf64_Sym, &CStr)> { let sym = self.symtab.get(idx as usize)?; let off = sym.st_name as usize; - if off >= self.strtab.len() { - return None; - } - // Find the NUL terminator within the remaining strtab. - let remaining = &self.strtab[off..]; + let remaining = self.strtab.get(off..)?; let nul_pos = remaining.iter().position(|&b| b == 0)?; - // SAFETY: we found a NUL byte within the validated strtab slice, - // so CStr::from_bytes_with_nul won't fail. - CStr::from_bytes_with_nul(&remaining[..=nul_pos]).ok() + let name = CStr::from_bytes_with_nul(&remaining[..=nul_pos]).ok()?; + Some((sym, name)) + } + + /// Look up the name of the symbol at index `idx` in the dynamic + /// string table. + pub fn sym_name(&self, idx: u32) -> Option<&CStr> { + self.sym_entry(idx).map(|(_, name)| name) } /// The base load address of this ELF object. @@ -389,8 +406,7 @@ pub unsafe fn sysv_hash_lookup(info: &DynamicInfo, name: &[u8]) -> Option= nchain as usize { break; } - if let Some(sname) = info.sym_name(idx) { - let sym = &info.symtab[idx as usize]; + if let Some((sym, sname)) = info.sym_entry(idx) { if sname.to_bytes() == name && check_sym(sym) { return Some(*sym); } @@ -530,8 +546,7 @@ pub unsafe fn gnu_hash_lookup(info: &DynamicInfo, name: &[u8]) -> Option> 1) == 0 { - if let Some(sname) = info.sym_name(symidx) { - let sym = &info.symtab[symidx as usize]; + if let Some((sym, sname)) = info.sym_entry(symidx) { if sname.to_bytes() == name && check_sym(sym) { return Some(*sym); } @@ -821,7 +836,15 @@ pub struct LookupResult { /// Hook a single symbol across all loaded ELF objects by patching their /// GOT entries. /// -/// - `symbol_name`: the symbol to hook (`c"__assert_fail"`) +/// This is a one-shot API: it patches every library that is loaded at +/// call time. Libraries `dlopen`'d after this call will **not** be +/// patched. Their calls to the hooked symbol will go directly to the +/// original. For hooks that need to cover dynamically loaded libraries, +/// use the lower-level [`DynamicInfo`] / [`iterate_libraries`] / +/// [`PageProtGuard`] primitives to build a registry that re-scans on +/// `dlopen` (see `libdd-profiling-heap-gotter`'s `SymbolOverrides`). +/// +/// - `symbol_name`: the symbol to hook (e.g. `c"__assert_fail"`) /// - `hook_fn`: address of the replacement function /// - `orig_out`: on success, receives the address of the original symbol /// @@ -1172,7 +1195,7 @@ mod tests { ); // Verify we can look up our exported symbol by walking - // relocations isn't needed + // relocations aren't needed let mut found_sym = false; for idx in 0..dyn_info.symtab.len() as u32 { if let Some(name) = dyn_info.sym_name(idx) { From 740e1b967d0bed60540d64b92600439f3a40eedf Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Fri, 7 Aug 2026 15:24:33 +0000 Subject: [PATCH 12/14] Return Option for hook_symbol --- libdd-gotter/README.md | 11 +++++------ libdd-gotter/src/elf.rs | 34 +++++++++++++++++++++++----------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/libdd-gotter/README.md b/libdd-gotter/README.md index 932bc7272a..1730dd3221 100644 --- a/libdd-gotter/README.md +++ b/libdd-gotter/README.md @@ -21,13 +21,12 @@ unsafe extern "C" fn my_hook(/* same signature as target */) { // forward to original via ORIG_FN } -let mut orig_addr: usize = 0; -unsafe { - hook_symbol(c"__assert_fail", my_hook as *const () as usize, &mut orig_addr); +if let Some(result) = unsafe { hook_symbol(c"__assert_fail", my_hook as *const () as usize) } { + // Release pairs with the Acquire load in my_hook, ensuring the GOT + // patches from hook_symbol are visible before the hook reads orig_addr. + ORIG_FN.store(result.orig_addr, Ordering::Release); + // result.patched tells you whether any GOT entries were rewritten } -// Release pairs with the Acquire load in my_hook, ensuring the GOT -// patches from hook_symbol are visible before the hook reads orig_addr. -ORIG_FN.store(orig_addr, Ordering::Release); ``` ### Multi-symbol registry (heap profiling hooking malloc/free/calloc/realloc) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index 218a44e931..c554e40be7 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -833,6 +833,18 @@ pub struct LookupResult { pub address: usize, } +/// Result of a successful [`hook_symbol`] call. +#[derive(Clone, Copy, Debug)] +pub struct HookResult { + /// Resolved address of the original symbol. Store this so the hook + /// function can forward calls to the real implementation. + pub orig_addr: usize, + /// Whether at least one GOT entry was patched. `false` means the + /// symbol was found but no loaded library had a matching GOT + /// relocation for it (e.g. statically linked libc on musl). + pub patched: bool, +} + /// Hook a single symbol across all loaded ELF objects by patching their /// GOT entries. /// @@ -846,24 +858,21 @@ pub struct LookupResult { /// /// - `symbol_name`: the symbol to hook (e.g. `c"__assert_fail"`) /// - `hook_fn`: address of the replacement function -/// - `orig_out`: on success, receives the address of the original symbol /// -/// Returns `true` if at least one GOT entry was patched. +/// Returns `None` if the symbol could not be found in any loaded +/// library. Returns `Some(HookResult)` if the symbol was resolved, +/// with `orig_addr` set to the original function address and `patched` +/// indicating whether any GOT entries were actually rewritten. /// /// # Safety /// /// `hook_fn` must point to a function with the same calling convention /// and signature as the symbol being hooked. The patching is permanent. -pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usize) -> bool { +pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize) -> Option { let symbol_name_bytes = symbol_name.to_bytes(); - let Ok(name_str) = symbol_name.to_str() else { - return false; - }; + let name_str = symbol_name.to_str().ok()?; - let Some(result) = lookup_symbol(name_str, hook_fn) else { - return false; - }; - *orig_out = result.address; + let result = lookup_symbol(name_str, hook_fn)?; let mut patched_any = false; let mut guard = PageProtGuard::new(); @@ -905,7 +914,10 @@ pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usi false }); - patched_any + Some(HookResult { + orig_addr: result.address, + patched: patched_any, + }) } /// Patch GOT entries in one library for the target symbol. From 4144d50b77aa438c7b687bfdb8171276b1e320bb Mon Sep 17 00:00:00 2001 From: Gyuheon Oh Date: Fri, 7 Aug 2026 18:49:48 +0000 Subject: [PATCH 13/14] Result with error variants --- libdd-gotter/README.md | 16 +++++++---- libdd-gotter/src/elf.rs | 62 ++++++++++++++++++++++++++++------------- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/libdd-gotter/README.md b/libdd-gotter/README.md index 1730dd3221..ce7a2e40c2 100644 --- a/libdd-gotter/README.md +++ b/libdd-gotter/README.md @@ -21,11 +21,17 @@ unsafe extern "C" fn my_hook(/* same signature as target */) { // forward to original via ORIG_FN } -if let Some(result) = unsafe { hook_symbol(c"__assert_fail", my_hook as *const () as usize) } { - // Release pairs with the Acquire load in my_hook, ensuring the GOT - // patches from hook_symbol are visible before the hook reads orig_addr. - ORIG_FN.store(result.orig_addr, Ordering::Release); - // result.patched tells you whether any GOT entries were rewritten +match unsafe { hook_symbol(c"__assert_fail", my_hook as *const () as usize) } { + Ok(result) => { + // Release pairs with the Acquire load in my_hook, ensuring the GOT + // patches from hook_symbol are visible before the hook reads orig_addr. + ORIG_FN.store(result.orig_addr, Ordering::Release); + + // result.entries_patched: number of GOT entries rewritten + // result.entries_failed: matched but mprotect failed + } + Err(HookError::SymbolNotFound) => { /* expected on musl/static libc */ } + Err(HookError::InvalidSymbolName) => { /* bug */ } } ``` diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index c554e40be7..deb5f09052 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -839,10 +839,20 @@ pub struct HookResult { /// Resolved address of the original symbol. Store this so the hook /// function can forward calls to the real implementation. pub orig_addr: usize, - /// Whether at least one GOT entry was patched. `false` means the - /// symbol was found but no loaded library had a matching GOT - /// relocation for it (e.g. statically linked libc on musl). - pub patched: bool, + /// Number of GOT entries successfully rewritten. + pub entries_patched: usize, + /// Number of GOT entries that matched the symbol but could not be + /// patched (`mprotect` failed to make the page writable). + pub entries_failed: usize, +} + +/// Error returned by [`hook_symbol`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum HookError { + /// `symbol_name` was not valid. + InvalidSymbolName, + /// No loaded library exports a definition for this symbol. + SymbolNotFound, } /// Hook a single symbol across all loaded ELF objects by patching their @@ -856,29 +866,37 @@ pub struct HookResult { /// [`PageProtGuard`] primitives to build a registry that re-scans on /// `dlopen` (see `libdd-profiling-heap-gotter`'s `SymbolOverrides`). /// -/// - `symbol_name`: the symbol to hook (e.g. `c"__assert_fail"`) +/// - `symbol_name`: the symbol to hook (`c"__assert_fail"`) /// - `hook_fn`: address of the replacement function /// -/// Returns `None` if the symbol could not be found in any loaded -/// library. Returns `Some(HookResult)` if the symbol was resolved, -/// with `orig_addr` set to the original function address and `patched` +/// Returns `Ok(HookResult)` if the symbol was resolved, with +/// `orig_addr` set to the original function address and `patched` /// indicating whether any GOT entries were actually rewritten. +/// Returns `Err(HookError)` if the symbol name is invalid or the +/// symbol could not be found. /// /// # Safety /// /// `hook_fn` must point to a function with the same calling convention /// and signature as the symbol being hooked. The patching is permanent. -pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize) -> Option { +pub unsafe fn hook_symbol( + symbol_name: &CStr, + hook_fn: usize, +) -> Result { let symbol_name_bytes = symbol_name.to_bytes(); - let name_str = symbol_name.to_str().ok()?; + let name_str = symbol_name + .to_str() + .map_err(|_| HookError::InvalidSymbolName)?; - let result = lookup_symbol(name_str, hook_fn)?; + let result = lookup_symbol(name_str, hook_fn).ok_or(HookError::SymbolNotFound)?; - let mut patched_any = false; + let mut entries_patched: usize = 0; + let mut entries_failed: usize = 0; let mut guard = PageProtGuard::new(); let guard_ptr = &mut guard as *mut PageProtGuard; - let patched_ptr = &mut patched_any as *mut bool; + let patched_ptr = &mut entries_patched as *mut usize; + let failed_ptr = &mut entries_failed as *mut usize; iterate_libraries(|info, _is_exe| { let lib_name = if info.dlpi_name.is_null() { @@ -900,8 +918,9 @@ pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize) -> Option Option Date: Fri, 7 Aug 2026 19:02:57 +0000 Subject: [PATCH 14/14] format --- libdd-gotter/src/elf.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libdd-gotter/src/elf.rs b/libdd-gotter/src/elf.rs index deb5f09052..509fe3678c 100644 --- a/libdd-gotter/src/elf.rs +++ b/libdd-gotter/src/elf.rs @@ -879,10 +879,7 @@ pub enum HookError { /// /// `hook_fn` must point to a function with the same calling convention /// and signature as the symbol being hooked. The patching is permanent. -pub unsafe fn hook_symbol( - symbol_name: &CStr, - hook_fn: usize, -) -> Result { +pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize) -> Result { let symbol_name_bytes = symbol_name.to_bytes(); let name_str = symbol_name .to_str()