From c25292ae21318fb226dba037c139ab06c906c921 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 31 Dec 2024 15:25:40 +1100 Subject: [PATCH 1/6] misc(ld.so): clippy fixes Signed-off-by: Anhad Singh --- src/ld_so/debug.rs | 9 +++---- src/ld_so/dso.rs | 63 ++++++++++++++++++++++++--------------------- src/ld_so/linker.rs | 16 ++++++------ src/ld_so/start.rs | 6 +++-- src/ld_so/tcb.rs | 4 +-- 5 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/ld_so/debug.rs b/src/ld_so/debug.rs index d6838e08a7..a2231cf25a 100644 --- a/src/ld_so/debug.rs +++ b/src/ld_so/debug.rs @@ -1,5 +1,6 @@ use crate::{c_str::CString, platform::types::*}; use alloc::boxed::Box; +use core::ptr; #[repr(C)] pub enum RTLDState { @@ -50,7 +51,6 @@ impl RTLDDebug { } else { unsafe { (*self.r_map).add_object(l_addr, name, l_ld) }; } - return; } pub fn insert_first(&mut self, l_addr: usize, name: &str, l_ld: usize) { if self.r_map.is_null() { @@ -60,7 +60,6 @@ impl RTLDDebug { self.r_map = LinkMap::new_with_args(l_addr, name, l_ld); unsafe { (*self.r_map).link(&mut *tmp) }; } - return; } } @@ -83,10 +82,10 @@ impl LinkMap { fn new() -> *mut Self { let map = Box::new(LinkMap { l_addr: 0, - l_name: 0 as *const c_char, + l_name: ptr::null(), l_ld: 0, - l_next: 0 as *mut LinkMap, - l_prev: 0 as *mut LinkMap, + l_next: ptr::null_mut(), + l_prev: ptr::null_mut(), }); Box::into_raw(map) } diff --git a/src/ld_so/dso.rs b/src/ld_so/dso.rs index c2cc36790a..030fd834d8 100644 --- a/src/ld_so/dso.rs +++ b/src/ld_so/dso.rs @@ -93,7 +93,7 @@ pub struct DSO { impl DSO { pub fn new( path: &str, - data: &Vec, + data: &[u8], base_addr: Option, dlopened: bool, id: usize, @@ -101,13 +101,13 @@ impl DSO { tls_offset: usize, ) -> Result<(DSO, Option)> { let elf = Elf::parse(data)?; - let (mmap, tcb_master) = DSO::mmap_and_copy(&path, &elf, &data, base_addr, tls_offset)?; - let (global_syms, weak_syms) = DSO::collect_syms(&elf, &mmap)?; + let (mmap, tcb_master) = DSO::mmap_and_copy(path, &elf, data, base_addr, tls_offset)?; + let (global_syms, weak_syms) = DSO::collect_syms(&elf, mmap)?; let (init_array, fini_array) = DSO::init_fini_arrays(&elf, mmap.as_ptr() as usize); let name = match elf.soname { Some(soname) => soname.to_string(), - _ => basename(&path), + _ => basename(path), }; let tls_offset = match tcb_master { Some(ref master) => master.offset, @@ -123,7 +123,7 @@ impl DSO { id, dlopened, entry_point, - runpath: DSO::get_runpath(&path, &elf)?, + runpath: DSO::get_runpath(path, &elf)?, mmap, global_syms, weak_syms, @@ -138,7 +138,7 @@ impl DSO { tls_offset, pie: is_pie_enabled(&elf), - dynamic: elf.dynamic.map(|dynamic| dynamic), + dynamic: elf.dynamic, dynsyms: elf.dynsyms.iter().collect(), scope: Scope::local(), @@ -149,10 +149,7 @@ impl DSO { /// Global Offset Table pub(super) fn got(&self) -> Option> { - let Some(dynamic) = self.dynamic.as_ref() else { - return None; - }; - + let dynamic = self.dynamic.as_ref()?; let object_base_addr = self.mmap.as_ptr() as u64; let got = if let Some(ptr) = { @@ -178,10 +175,10 @@ impl DSO { pub fn get_sym(&self, name: &str) -> Option<(Symbol, SymbolBinding)> { if let Some(value) = self.global_syms.get(name) { Some((*value, SymbolBinding::Global)) - } else if let Some(value) = self.weak_syms.get(name) { - Some((*value, SymbolBinding::Weak)) } else { - None + self.weak_syms + .get(name) + .map(|value| (*value, SymbolBinding::Weak)) } } @@ -190,7 +187,9 @@ impl DSO { let (addr, size) = self.init_array; for i in (0..size).step_by(8) { let func = transmute::>(addr + i); - (*func).map(|x| x()); + if let Some(f) = *func { + f(); + } } } } @@ -200,7 +199,9 @@ impl DSO { let (addr, size) = self.fini_array; for i in (0..size).step_by(8).rev() { let func = transmute::>(addr + i); - (*func).map(|x| x()); + if let Some(f) = *func { + f(); + } } } } @@ -220,13 +221,14 @@ impl DSO { _ => return Ok(None), } } - return Ok(None); + + Ok(None) } fn mmap_and_copy( path: &str, elf: &Elf, - data: &Vec, + data: &[u8], base_addr: Option, tls_offset: usize, ) -> Result<(&'static mut [u8], Option)> { @@ -269,12 +271,12 @@ impl DSO { // Allocate memory let mmap = unsafe { if let Some(addr) = base_addr { - let size = if is_pie_enabled(&elf) { + let size = if is_pie_enabled(elf) { bounds.1 } else { bounds.1 - bounds.0 }; - _r_debug.insert_first(addr as usize, path, addr + l_ld as usize); + _r_debug.insert_first(addr, path, addr + l_ld as usize); slice::from_raw_parts_mut(addr as *mut u8, size) } else { let (start, end) = bounds; @@ -371,7 +373,7 @@ impl DSO { } }; tcb_master = Some(Master { - ptr: ptr, + ptr, len: ph.p_filesz as usize, offset: tls_offset + vsize, }); @@ -384,13 +386,11 @@ impl DSO { let mut debug_start = None; // next we identify the location of DT_DEBUG in .dynamic section if let Some(dynamic) = elf.dynamic.as_ref() { - let mut i = 0; - for entry in &dynamic.dyns { + for (i, entry) in dynamic.dyns.iter().enumerate() { if entry.d_tag == DT_DEBUG { - debug_start = Some(i as usize); + debug_start = Some(i); break; } - i += 1; } } if let Some(i) = debug_start { @@ -408,7 +408,8 @@ impl DSO { _ => (), } } - return Ok((mmap, tcb_master)); + + Ok((mmap, tcb_master)) } fn collect_syms( @@ -458,7 +459,8 @@ impl DSO { _ => unreachable!(), } } - return Ok((globals, weak_syms)); + + Ok((globals, weak_syms)) } fn init_fini_arrays(elf: &Elf, mmap_addr: usize) -> ((usize, usize), (usize, usize)) { @@ -480,7 +482,8 @@ impl DSO { fini_array = (addr, section.sh_size as usize); } } - return (init_array, fini_array); + + (init_array, fini_array) } } @@ -492,15 +495,15 @@ impl Drop for DSO { } pub fn is_pie_enabled(elf: &Elf) -> bool { - return elf.header.e_type == ET_DYN; + elf.header.e_type == ET_DYN } fn basename(path: &str) -> String { - return path.split("/").last().unwrap_or(path).to_string(); + path.split("/").last().unwrap_or(path).to_string() } fn dirname(path: &str) -> String { let mut parts: Vec<&str> = path.split("/").collect(); parts.truncate(parts.len() - 1); - return parts.join("/"); + parts.join("/") } diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index f704751b3b..e7a43b4139 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -139,9 +139,9 @@ impl Scope { fn add(&mut self, target: &Arc) { match self { Self::Global { objs } => { - let target = Arc::downgrade(&target); + let target = Arc::downgrade(target); for obj in objs.iter() { - if Weak::ptr_eq(&obj, &target) { + if Weak::ptr_eq(obj, &target) { return; } } @@ -151,7 +151,7 @@ impl Scope { Self::Local { objs, .. } => { for obj in objs.iter() { - if Arc::ptr_eq(obj, &target) { + if Arc::ptr_eq(obj, target) { return; } } @@ -190,7 +190,7 @@ impl Scope { .find_map(get_sym) } } - .or_else(|| res) + .or(res) } fn move_into(&self, other: &mut Self) { @@ -199,7 +199,7 @@ impl Scope { (Self::Local { owner, objs }, Self::Global { objs: other_objs }) => { let owner = owner.as_ref().expect("local scope without owner"); other_objs.push(owner.clone()); - other_objs.extend(objs.iter().map(|o| Arc::downgrade(o))); + other_objs.extend(objs.iter().map(Arc::downgrade)); } _ => unreachable!(), @@ -220,7 +220,7 @@ impl ObjectHandle { } #[inline] - fn into_inner(&self) -> Arc { + fn into_inner(self) -> Arc { unsafe { Arc::from_raw(self.0) } } @@ -1036,7 +1036,7 @@ extern "C" fn __plt_resolve_inner(obj: *const DSO, relocation_index: c_uint) -> let obj = unsafe { &*obj }; let obj_base = obj.mmap.as_ptr() as usize; let dynamic = obj.dynamic.as_ref().unwrap(); - let jmprel = dynamic.info.jmprel as usize; + let jmprel = dynamic.info.jmprel; let rela = unsafe { &*((obj_base + jmprel) as *const reloc::reloc64::Rela).add(relocation_index as usize) @@ -1068,7 +1068,7 @@ extern "C" fn __plt_resolve_inner(obj: *const DSO, relocation_index: c_uint) -> }; let resolved = resolve_sym(name.to_str().unwrap(), &[&GLOBAL_SCOPE.read(), &obj.scope]) - .expect(&format!("symbol '{}' not found", name.to_str().unwrap())) + .unwrap_or_else(|| panic!("symbol '{}' not found", name.to_str().unwrap())) .as_ptr(); trace!( diff --git a/src/ld_so/start.rs b/src/ld_so/start.rs index 661e4273dd..96c3a2b296 100644 --- a/src/ld_so/start.rs +++ b/src/ld_so/start.rs @@ -42,7 +42,8 @@ unsafe fn get_argv(mut ptr: *const usize) -> (Vec, *const usize) { } ptr = ptr.add(1); } - return (argv, ptr); + + (argv, ptr) } unsafe fn get_env(mut ptr: *const usize) -> (BTreeMap, *const usize) { @@ -60,7 +61,8 @@ unsafe fn get_env(mut ptr: *const usize) -> (BTreeMap, *const us } ptr = ptr.add(1); } - return (envs, ptr); + + (envs, ptr) } unsafe fn adjust_stack(sp: &'static mut Stack) { diff --git a/src/ld_so/tcb.rs b/src/ld_so/tcb.rs index 48fd756a0e..2fc7205788 100644 --- a/src/ld_so/tcb.rs +++ b/src/ld_so/tcb.rs @@ -1,4 +1,4 @@ -use alloc::vec::Vec; +use alloc::{string::ToString, vec::Vec}; use core::{ cell::UnsafeCell, mem, @@ -247,7 +247,7 @@ impl Tcb { -1, 0, ) - .map_err(|_| Error::Malformed(format!("failed to map tls")))?; + .map_err(|_| Error::Malformed("failed to map tls".to_string()))?; ptr::write_bytes(ptr as *mut u8, 0, size); Ok(slice::from_raw_parts_mut(ptr as *mut u8, size)) From 60f700fb501fd3b9d5b59fece44e9a69f8011807 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 31 Dec 2024 15:26:23 +1100 Subject: [PATCH 2/6] chore(linker): remove todo Signed-off-by: Anhad Singh --- src/ld_so/linker.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index e7a43b4139..3292724509 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -48,7 +48,6 @@ use super::{ // by the dynamic linker's and was freed by the program. // do better errors than just goblin::Error::Malformed -// TODO: rwlock? static GLOBAL_SCOPE: RwLock = RwLock::new(Scope::global()); /// Same as [`crate::fs::File`], but does not touch [`crate::platform::ERRNO`] as the dynamic From 9cf72fd0b2ebf6010e0955e7b658051e97f0add6 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 31 Dec 2024 18:00:45 +1100 Subject: [PATCH 3/6] feat(ld.so): add `all` option for LD_DEBUG Signed-off-by: Anhad Singh --- src/ld_so/linker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index 3292724509..5a2809ee80 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -272,6 +272,7 @@ impl Config { "baseaddr" => DebugFlags::BASE_ADDRESS, "search" => DebugFlags::SEARCH, "scopes" => DebugFlags::SCOPES, + "all" => DebugFlags::all(), _ => { eprintln!("[ld.so]: unknown debug flag '{}'", opt); DebugFlags::empty() From 42d884c6a38b321c0d4805253959b278c5637cc2 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 31 Dec 2024 18:35:20 +1100 Subject: [PATCH 4/6] fix(ld.so): errors UB Currently, how ld.so errors are handled by functions in dlfcn is wrong. dlopen calls `linker.load_library` which returns a `Result`. Now `goblin::Error` may have variants that are heap allocated. For example: `Error::Malformed(format!("invalid path: '{}': {}", path, err))`. The error string would be allocated by ld.so's allocator but will be dropped inside libc. This is UB. After this patch, we now return a custom `DlError` instead. To get more information about the error, `LD_DEBUG=all` can be set. Signed-off-by: Anhad Singh --- src/header/dlfcn/mod.rs | 2 +- src/ld_so/callbacks.rs | 3 +- src/ld_so/linker.rs | 160 +++++++++++++++++++++++++++------------- src/ld_so/start.rs | 3 +- 4 files changed, 114 insertions(+), 54 deletions(-) diff --git a/src/header/dlfcn/mod.rs b/src/header/dlfcn/mod.rs index e1230c7dcd..06e62f8a36 100644 --- a/src/header/dlfcn/mod.rs +++ b/src/header/dlfcn/mod.rs @@ -98,7 +98,7 @@ pub unsafe extern "C" fn dlopen(cfilename: *const c_char, flags: c_int) -> *mut match (cbs.load_library)(&mut linker, filename, resolve, scope, noload) { Ok(handle) => handle.as_ptr().cast_mut(), Err(error) => { - ERROR.store(ERROR_NOT_SUPPORTED.as_ptr() as usize, Ordering::SeqCst); + ERROR.store(error.repr().as_ptr() as usize, Ordering::SeqCst); ptr::null_mut() } } diff --git a/src/ld_so/callbacks.rs b/src/ld_so/callbacks.rs index 64dc41dc08..c853f7ab36 100644 --- a/src/ld_so/callbacks.rs +++ b/src/ld_so/callbacks.rs @@ -1,7 +1,6 @@ -use super::linker::{Linker, ObjectHandle, ObjectScope, Resolve}; +use super::linker::{Linker, ObjectHandle, ObjectScope, Resolve, Result}; use crate::platform::types::c_void; use alloc::boxed::Box; -use goblin::error::Result; pub struct LinkerCallbacks { pub unload: Box, diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index 5a2809ee80..6e3b8c746b 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -10,10 +10,7 @@ use core::{ mem::transmute, ptr::{self, NonNull}, }; -use goblin::{ - elf::{dynamic::DT_STRTAB, program_header, reloc, sym::STT_TLS, Elf}, - error::{Error, Result}, -}; +use goblin::elf::{dynamic::DT_STRTAB, program_header, reloc, sym::STT_TLS, Elf}; use crate::{ c_str::{CStr, CString}, @@ -41,12 +38,30 @@ use super::{ PATH_SEP, }; -// FIXME: ERROR HANDLING IS WRONG! -// load_library may return an error which may have some variants heap allocated. -// eg Error::Malformed(String::from("failed to locate 'libfoo.so'")) -// therefore dropping it in dlopen would cause heap corruption as it was allocated -// by the dynamic linker's and was freed by the program. -// do better errors than just goblin::Error::Malformed +#[derive(Debug, Copy, Clone)] +pub enum DlError { + /// Failed to locate the requested DSO. + NotFound, + /// The DSO is malformed somehow. + Malformed, +} + +impl DlError { + /// Returns a human-readable, null-terminated C string describing the error. + pub fn repr(&self) -> CStr<'static> { + match self { + DlError::NotFound => c_str!( + "Failed to locate the requested DSO. Set `LD_DEBUG=all` for more information." + ), + + DlError::Malformed => { + c_str!("The DSO is malformed somehow. Set `LD_DEBUG=all` for more information.") + } + } + } +} + +pub type Result = core::result::Result; static GLOBAL_SCOPE: RwLock = RwLock::new(Scope::global()); @@ -245,7 +260,7 @@ bitflags::bitflags! { #[derive(Debug, Default)] pub struct DebugFlags: u32 { /// Display what objects and where they are being loaded. - const BASE_ADDRESS = 1 << 1; + const LOAD = 1 << 1; /// Display library search paths. const SEARCH = 1 << 2; /// Display scope information. @@ -269,7 +284,7 @@ impl Config { let mut flags = DebugFlags::empty(); for opt in value.split(',') { flags |= match opt { - "baseaddr" => DebugFlags::BASE_ADDRESS, + "load" => DebugFlags::LOAD, "search" => DebugFlags::SEARCH, "scopes" => DebugFlags::SCOPES, "all" => DebugFlags::all(), @@ -567,7 +582,7 @@ impl Linker { }; #[cfg(not(target_os = "redox"))] - let tcb = Tcb::new(self.tls_size)?; + let tcb = Tcb::new(self.tls_size).map_err(|_| DlError::Malformed)?; // We are now loading the main program or its dependencies. The TLS for all initially // loaded objects reside in the static TLS block. Depending on the architecture, the @@ -601,7 +616,7 @@ impl Linker { tcb.append_masters(tcb_masters); // Copy the master data into the static TLS block. - tcb.copy_masters()?; + tcb.copy_masters().map_err(|_| DlError::Malformed)?; tcb.activate(); #[cfg(target_os = "redox")] @@ -657,8 +672,10 @@ impl Linker { return Ok(obj.clone()); } + let debug = self.config.debug_flags.contains(DebugFlags::LOAD); + let path = self.search_object(name, parent_runpath)?; - let data = Linker::read_file(&path)?; + let data = self.read_file(&path)?; let (mut obj, tcb_master) = DSO::new( &path, &data, @@ -667,9 +684,16 @@ impl Linker { self.next_object_id, self.next_tls_module_id, self.tls_size, - )?; + ) + .map_err(|err| { + if debug { + eprintln!("[ld.so]: failed to load '{}': {}", name, err) + } - if self.config.debug_flags.contains(DebugFlags::BASE_ADDRESS) { + DlError::Malformed + })?; + + if debug { eprintln!( "[ld.so]: loading object: {} at {:#x}", name, @@ -760,18 +784,41 @@ impl Linker { } } - Err(Error::Malformed(format!("failed to locate '{}'", name))) + if debug { + eprintln!("[ld.so]: failed to locate '{}'", name); + } + + Err(DlError::NotFound) } - fn read_file(path: &str) -> Result> { + fn read_file(&self, path: &str) -> Result> { + let debug = self.config.debug_flags.contains(DebugFlags::SEARCH); + let mut data = Vec::new(); - let path_c = CString::new(path) - .map_err(|err| Error::Malformed(format!("invalid path '{}': {}", path, err)))?; + let path_c = CString::new(path).map_err(|err| { + if debug { + eprintln!("[ld.so]: invalid path '{}': {}", path, err) + } + + DlError::NotFound + })?; + let flags = fcntl::O_RDONLY | fcntl::O_CLOEXEC; - let mut file = File::open(CStr::borrow(&path_c), flags) - .map_err(|err| Error::Malformed(format!("failed to open '{}': {}", path, err)))?; - file.read_to_end(&mut data) - .map_err(|err| Error::Malformed(format!("failed to read '{}': {}", path, err)))?; + let mut file = File::open(CStr::borrow(&path_c), flags).map_err(|err| { + if debug { + eprintln!("[ld.so]: failed to open '{}': {}", path, err) + } + + DlError::NotFound + })?; + + file.read_to_end(&mut data).map_err(|err| { + if debug { + eprintln!("[ld.so]: failed to read '{}': {}", path, err) + } + + DlError::NotFound + })?; Ok(data) } @@ -780,6 +827,7 @@ impl Linker { fn lazy_relocate(&self, obj: &Arc, elf: &Elf, resolve: Resolve) -> Result<()> { let Some(got) = obj.got() else { return Ok(()) }; let object_base_addr = obj.mmap.as_ptr() as u64; + let debug = self.config.debug_flags.contains(DebugFlags::LOAD); unsafe { got.add(1).write(Arc::as_ptr(obj) as usize); @@ -803,18 +851,21 @@ impl Linker { } (reloc::R_X86_64_JUMP_SLOT, Resolve::Now) => { - let sym = elf.dynsyms.get(rel.r_sym).ok_or(Error::Malformed(format!( - "missing symbol for relocation {:?}", - rel - )))?; + let sym = elf.dynsyms.get(rel.r_sym).ok_or_else(|| { + if debug { + eprintln!("missing symbol for relocation {:?}", rel) + } - let name = - elf.dynstrtab - .get_at(sym.st_name) - .ok_or(Error::Malformed(format!( - "missing name for symbol {:?}", - sym - )))?; + DlError::Malformed + })?; + + let name = elf.dynstrtab.get_at(sym.st_name).ok_or_else(|| { + if debug { + eprintln!("missing name for symbol {:?}", sym) + } + + DlError::Malformed + })?; let resolved = GLOBAL_SCOPE .read() @@ -839,7 +890,8 @@ impl Linker { fn relocate(&self, obj: &Arc, object_data: &[u8], resolve: Resolve) -> Result<()> { // Perform static relocations. - let elf = Elf::parse(object_data)?; + let elf = Elf::parse(object_data).or(Err(DlError::Malformed))?; + let debug = self.config.debug_flags.contains(DebugFlags::LOAD); // FIXME trace!("link {}", obj.name); @@ -855,18 +907,21 @@ impl Linker { ); let symbol = if rel.r_sym > 0 { - let sym = elf.dynsyms.get(rel.r_sym).ok_or(Error::Malformed(format!( - "missing symbol for relocation {:?}", - rel - )))?; + let sym = elf.dynsyms.get(rel.r_sym).ok_or_else(|| { + if debug { + eprintln!("[ld.so]: missing symbol for relocation {:?}", rel) + } - let name = elf - .dynstrtab - .get_at(sym.st_name) - .ok_or(Error::Malformed(format!( - "missing name for symbol {:?}", - sym - )))?; + DlError::Malformed + })?; + + let name = elf.dynstrtab.get_at(sym.st_name).ok_or_else(|| { + if debug { + eprintln!("[ld.so]: missing name for symbol {:?}", sym) + } + + DlError::Malformed + })?; let symbol = GLOBAL_SCOPE .read() @@ -992,8 +1047,13 @@ impl Linker { vaddr as *const u8 }; trace!(" prot {:#x}, {:#x}: {:p}, {:#x}", vaddr, vsize, ptr, prot); - Sys::mprotect(ptr as *mut c_void, vsize, prot) - .map_err(|_| Error::Malformed(format!("failed to mprotect {}", obj.name)))?; + Sys::mprotect(ptr as *mut c_void, vsize, prot).map_err(|err| { + if debug { + eprintln!("[ld.so]: failed to mprotect: {}", err) + } + + DlError::Malformed + })?; } } diff --git a/src/ld_so/start.rs b/src/ld_so/start.rs index 96c3a2b296..19fdbdcfd1 100644 --- a/src/ld_so/start.rs +++ b/src/ld_so/start.rs @@ -238,7 +238,8 @@ pub extern "C" fn relibc_ld_so_start(sp: &'static mut Stack, ld_entry: usize) -> let entry = match linker.load_program(&path, base_addr) { Ok(entry) => entry, Err(err) => { - eprintln!("ld.so: failed to link '{}': {}", path, err); + eprintln!("ld.so: failed to link '{}': {:?}", path, err); + eprintln!("ld.so: enable debug output with `LD_DEBUG=all` for more information"); unistd::_exit(1); } }; From 01e6eda258e5b035a71b691bdcdaeb1540628845 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 31 Dec 2024 19:03:05 +1100 Subject: [PATCH 5/6] misc(dlfcn): cleanup Signed-off-by: Anhad Singh --- src/header/dlfcn/mod.rs | 29 ++++++++++++++++++----------- src/ld_so/linker.rs | 6 ++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/header/dlfcn/mod.rs b/src/header/dlfcn/mod.rs index 06e62f8a36..1c23f6ec85 100644 --- a/src/header/dlfcn/mod.rs +++ b/src/header/dlfcn/mod.rs @@ -1,19 +1,19 @@ //! dlfcn implementation for Redox, following http://pubs.opengroup.org/onlinepubs/7908799/xsh/dlfcn.h.html #![deny(unsafe_op_in_unsafe_fn)] +// FIXME(andypython): remove this when #![allow(warnings, unused_variables)] is +// dropped from src/lib.rs. +#![warn(warnings, unused_variables)] use core::{ - ptr::{self, NonNull}, - str, + ptr, str, sync::atomic::{AtomicUsize, Ordering}, }; -use alloc::sync::Arc; - use crate::{ c_str::CStr, ld_so::{ - linker::{ObjectHandle, ObjectScope, Resolve}, + linker::{DlError, ObjectHandle, ObjectScope, Resolve}, tcb::Tcb, }, platform::types::*, @@ -32,7 +32,12 @@ static ERROR_NOT_SUPPORTED: CStr = c_str!("dlfcn not supported"); #[thread_local] static ERROR: AtomicUsize = AtomicUsize::new(0); +fn set_last_error(error: DlError) { + ERROR.store(error.repr().as_ptr() as usize, Ordering::SeqCst); +} + #[repr(C)] +#[allow(non_camel_case_types)] pub struct Dl_info { dli_fname: *const c_char, dli_fbase: *mut c_void, @@ -41,7 +46,7 @@ pub struct Dl_info { } #[no_mangle] -pub unsafe extern "C" fn dladdr(addr: *mut c_void, info: *mut Dl_info) -> c_int { +pub unsafe extern "C" fn dladdr(_addr: *mut c_void, info: *mut Dl_info) -> c_int { //TODO unsafe { (*info).dli_fname = ptr::null(); @@ -86,11 +91,13 @@ pub unsafe extern "C" fn dlopen(cfilename: *const c_char, flags: c_int) -> *mut return ptr::null_mut(); } }; + if tcb.linker_ptr.is_null() { ERROR.store(ERROR_NOT_SUPPORTED.as_ptr() as usize, Ordering::SeqCst); return ptr::null_mut(); } - let mut linker = unsafe { (&*tcb.linker_ptr).lock() }; + + let mut linker = unsafe { (*tcb.linker_ptr).lock() }; let cbs_c = linker.cbs.clone(); let cbs = cbs_c.borrow(); @@ -98,7 +105,7 @@ pub unsafe extern "C" fn dlopen(cfilename: *const c_char, flags: c_int) -> *mut match (cbs.load_library)(&mut linker, filename, resolve, scope, noload) { Ok(handle) => handle.as_ptr().cast_mut(), Err(error) => { - ERROR.store(error.repr().as_ptr() as usize, Ordering::SeqCst); + set_last_error(error); ptr::null_mut() } } @@ -131,7 +138,7 @@ pub unsafe extern "C" fn dlsym(handle: *mut c_void, symbol: *const c_char) -> *m return ptr::null_mut(); } - let linker = unsafe { (&*tcb.linker_ptr).lock() }; + let linker = unsafe { (*tcb.linker_ptr).lock() }; let cbs_c = linker.cbs.clone(); let cbs = cbs_c.borrow(); match (cbs.get_sym)(&linker, handle, symbol_str) { @@ -159,11 +166,11 @@ pub unsafe extern "C" fn dlclose(handle: *mut c_void) -> c_int { }; let Some(handle) = ObjectHandle::from_ptr(handle) else { - ERROR.store(ERROR_NOT_SUPPORTED.as_ptr() as usize, Ordering::SeqCst); + set_last_error(DlError::InvalidHandle); return -1; }; - let mut linker = unsafe { (&*tcb.linker_ptr).lock() }; + let mut linker = unsafe { (*tcb.linker_ptr).lock() }; let cbs_c = linker.cbs.clone(); let cbs = cbs_c.borrow(); (cbs.unload)(&mut linker, handle); diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index 6e3b8c746b..8146b1e795 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -44,6 +44,8 @@ pub enum DlError { NotFound, /// The DSO is malformed somehow. Malformed, + /// Invalid DSO handle. + InvalidHandle, } impl DlError { @@ -57,6 +59,10 @@ impl DlError { DlError::Malformed => { c_str!("The DSO is malformed somehow. Set `LD_DEBUG=all` for more information.") } + + DlError::InvalidHandle => { + c_str!("Invalid DSO handle. Set `LD_DEBUG=all` for more information.") + } } } } From bfe1c0b545ed3fb45506d0e86d2fbdf6bf54581d Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 31 Dec 2024 19:30:37 +1100 Subject: [PATCH 6/6] fix(ld.so): redox build Signed-off-by: Anhad Singh --- src/ld_so/linker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index 8146b1e795..26985f2e4e 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -547,7 +547,7 @@ impl Linker { use redox_rt::signal::tmp_disable_signals; let old_tcb = Tcb::current().expect("failed to get bootstrap TCB"); - let new_tcb = Tcb::new(self.tls_size)?; // This actually allocates TCB, TLS and ABI page. + let new_tcb = Tcb::new(self.tls_size).map_err(|_| DlError::Malformed)?; // This actually allocates TCB, TLS and ABI page. // Stash let new_tls_end = new_tcb.generic.tls_end;