From b566f1917a63c2850c9e4307f384f8bc68286009 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 06:17:21 +0200 Subject: [PATCH 1/6] libc: dl_iterate_phdr, from program headers the kernel reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libunwind finds a module's unwind tables through `dl_iterate_phdr`, which M3 of issues/build/toyos-builds-itself.md needs. Nothing told userland where a loaded module's program headers are: a process starts with no auxiliary vector, and `SYS_QUERY_MODULES`'s `ModuleInfo` carried a base, an extent and `.eh_frame_hdr` but no `PT_*` table. ABI: `ModuleInfo` gains `phdr` (the absolute address of the module's program header table), `phnum` and `phentsize`, between `eh_frame_hdr_size` and `path_offset`. It grows from 40 to 56 bytes, still with no padding, which its const assert and the wire-decoding test now say. No auxiliary vector is added. toyos-elf: `Layout::program_headers()` is where the image holds `e_phoff..+e_phnum*56`: the first `PT_LOAD` whose file bytes hold all of it, as an `ImageRange`, or `None`. It is derived from where the loader copies the file, not from `PT_PHDR`, which is only the file's claim; bytes past `p_filesz` are zeroes and hold nothing. The bootloader parses the kernel with the same `Layout`, so this is an answer, not a refusal. Kernel: an executable or library whose table no `PT_LOAD` maps is refused — `spawn` right after the rebase check, `load_shared_lib` right after the vaddr-0 check — rather than mapped or copied in. Refusing keeps the answer true with no new mapping, and every ELF the tree builds already maps its table: `llvm-readobj -h -l` over the 24 guest ELFs `--build-only` makes, the 98 test binaries and four test libraries of the primary's last build, and this branch's C test, found all 127 with `e_phoff` 64 and the table inside one `PT_LOAD`'s file bytes. The crafted ELFs in abuse_elf_loader and abuse_elf_segments that do not map their table (filesz-0 segments at kernel-half and arena vaddrs) are refused by `rebase_base` before this check, so each still exercises the refusal it was written for. `ElfInfo` carries the executable's table as (address, count), `LoadedLib` and the cache snapshot carry a library's, and `sys_query_modules` reports both. toyos-abi: `syscall::modules` decodes one `query_modules` answer into its records and paths, which libc uses; host-tested beside the layout test. libc: `dl_iterate_phdr` and `link.h` (`struct dl_phdr_info` with `dlpi_addr`, `dlpi_name`, `dlpi_phdr`, `dlpi_phnum` — no `dlpi_adds`/`dlpi_subs`), with an `elf.h` holding the program header vocabulary `link.h` needs. The contract is glibc's dl_iterate_phdr(3), which the LSB adopts: the executable first, named "", then each library in load order; the first non-zero callback return stops the walk and is returned, else 0. `dlpi_addr` is the load bias, so `dlpi_addr + p_vaddr` is a runtime address. Test: tests/testcases/tinycc/204_dl_iterate_phdr walks every module and checks each against facts the kernel did not report: its table lies in one of its own `PT_LOAD`s, `PT_PHDR` (where the linker emits one) names the same address, the executable's `main` is in an executable `PT_LOAD` and its `PT_DYNAMIC` is at the linker's `_DYNAMIC`; after `dlopen("/system/lib/libtls_lib.so")` the library is visited by that name with `dlsym`'s `tls_get_label` in one of its executable segments; callbacks returning 1, 7 and -1 stop the walk where they say and are returned. The corpus licence counts two more files of ours. The std fork's unwinder reads `ModuleInfo` through `size_of` and by field name, so it builds unchanged against the larger record; it needed no fork commit. Reading it found it takes the byte count `query_modules` returns as a record count, filed as issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md. Closes issues/build/libc-has-no-dl-iterate-phdr.md, and drops its citation from issues/build/toyos-builds-itself.md. Co-Authored-By: Claude Opus 5.5 --- issues/build/libc-has-no-dl-iterate-phdr.md | 19 --- issues/build/toyos-builds-itself.md | 3 +- ...ry-modules-byte-count-as-a-module-count.md | 24 +++ kernel/src/elf/cache.rs | 5 +- kernel/src/elf/mod.rs | 10 +- kernel/src/loader/mod.rs | 7 + kernel/src/process.rs | 3 + kernel/src/syscall/vm.rs | 7 + tests/testcases/LICENSE | 4 +- tests/testcases/tinycc/204_dl_iterate_phdr.c | 139 ++++++++++++++++++ .../tinycc/204_dl_iterate_phdr.expect | 8 + toyos-abi/src/syscall.rs | 101 ++++++++++++- toyos-elf/src/layout.rs | 58 +++++++- toyos-elf/src/lib.rs | 2 +- toyos-elf/tests/crafted.rs | 46 ++++++ toyos-elf/tests/fuzz.rs | 3 + toyos-elf/tests/real.rs | 5 + userland/libc/include/elf.h | 44 ++++++ userland/libc/include/link.h | 22 +++ userland/libc/src/lib.rs | 1 + userland/libc/src/link.rs | 61 ++++++++ 21 files changed, 535 insertions(+), 37 deletions(-) delete mode 100644 issues/build/libc-has-no-dl-iterate-phdr.md create mode 100644 issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md create mode 100644 tests/testcases/tinycc/204_dl_iterate_phdr.c create mode 100644 tests/testcases/tinycc/204_dl_iterate_phdr.expect create mode 100644 userland/libc/include/elf.h create mode 100644 userland/libc/include/link.h create mode 100644 userland/libc/src/link.rs diff --git a/issues/build/libc-has-no-dl-iterate-phdr.md b/issues/build/libc-has-no-dl-iterate-phdr.md deleted file mode 100644 index 8c7b4d7d744..00000000000 --- a/issues/build/libc-has-no-dl-iterate-phdr.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -status: open -kind: track -opened: 2026-09-28 ---- - -# libc has no `dl_iterate_phdr` - -libunwind finds a module's unwind tables through `dl_iterate_phdr`, so M3 of -`issues/build/toyos-builds-itself.md` needs it in `userland/libc`. It is not a -libc change alone: the process starts with `argc` and `argv` on its stack and no -auxiliary vector, and `SYS_QUERY_MODULES`'s `ModuleInfo` carries a module's -base, extent and `.eh_frame_hdr` but not where its program headers are, so -nothing tells userland a loaded module's `PT_*` table. Reading them off the -module's base assumes a mapped ELF header, which no loader code promises. - -**Exit**: `dl_iterate_phdr` in libc and `link.h` in its headers, visiting the -executable and every loaded library with its program headers, from an answer -the kernel gives rather than one assumed. diff --git a/issues/build/toyos-builds-itself.md b/issues/build/toyos-builds-itself.md index 8bf082ce50d..7311413ceda 100644 --- a/issues/build/toyos-builds-itself.md +++ b/issues/build/toyos-builds-itself.md @@ -46,8 +46,7 @@ and the network stack under it (`issues/hardware/the-lan-is-not-yet-production-g `issues/design-debt/the-internet-clients-work-unchanged.md`), room for about a gigabyte of toolchain, and threads and `mmap` mature enough for LLVM (`issues/kernel/std-and-libc-drop-the-answer-thread-join-gives.md`). M3 needs -locale support or libc++'s no-localization build, and -`dl_iterate_phdr` in libc (`issues/build/libc-has-no-dl-iterate-phdr.md`). M4 needs git in the guest, storage durable and fast +locale support or libc++'s no-localization build. M4 needs git in the guest, storage durable and fast enough for an LLVM build tree (`issues/filesystem/storage-is-layers-and-a-role-is-a-filesystem.md`), and memory beyond what 2 MiB process pages allow diff --git a/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md b/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md new file mode 100644 index 00000000000..597cfafaa2d --- /dev/null +++ b/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-09-29 +--- + +# std reads a `SYS_QUERY_MODULES` byte count as a module count + +The std fork's unwinder (`rust/library/std/src/sys/pal/toyos/mod.rs`, +`eh_frame::load_modules`) takes `query_modules`'s `Ok(n)` as a record count +and reads `size_of::()`-byte records from offset 0 until one would +pass the end of its 4096-byte buffer. `n` counts **bytes** +(`toyos_abi::syscall::query_modules`'s doc), so every "record" after the real +ones is the packed path strings and then the zeroed tail of the buffer, read +as `base`/`text_end`/`eh_frame_hdr`. It also takes `n > buf.len()`, which means +nothing was written, as an answer and reads the zeroes. + +No wrong frame has been seen: a path's ASCII read as a `u64` lies far above the +user half, so no user PC falls in the range it makes, and a zeroed record's +range is empty. That is luck in the bytes, not a check. + +**Exit**: `load_modules` decodes the answer with `toyos_abi::syscall::modules` +over the `n` bytes the call reported, and grows the buffer to `n` when `n` +exceeds it. diff --git a/kernel/src/elf/cache.rs b/kernel/src/elf/cache.rs index 7eb03a5d926..963222950d0 100644 --- a/kernel/src/elf/cache.rs +++ b/kernel/src/elf/cache.rs @@ -22,7 +22,7 @@ use crate::vfs::BackingId; use crate::UserAddr; use toyos_elf::dynamic::InitArray; use toyos_elf::rela::Rules; -use toyos_elf::{ImageRange, Op, RelaCounts, RelocKind, SymIndex, TlsRef}; +use toyos_elf::{ImageRange, Op, ProgramHeaderTable, RelaCounts, RelocKind, SymIndex, TlsRef}; /// A module's non-`RELATIVE` relocations, parsed and extracted once at cache /// time, each as `(r_offset, what it names)`. @@ -82,6 +82,7 @@ struct Snapshot { gnu_hash: Option, rules: Rules, eh_frame_hdr: Option, + phdrs: ProgramHeaderTable, init_array: Option, span: u64, rw_lo: u64, @@ -100,6 +101,7 @@ impl Snapshot { gnu_hash: lib.gnu_hash, rules: lib.rules, eh_frame_hdr: lib.eh_frame_hdr, + phdrs: lib.phdrs, init_array: lib.init_array, span: lib.span, rw_lo: lib.rw_lo, @@ -127,6 +129,7 @@ impl Snapshot { cached_relocs, rules: self.rules, eh_frame_hdr: self.eh_frame_hdr, + phdrs: self.phdrs, init_array: self.init_array, span: self.span, rw_lo: self.rw_lo, diff --git a/kernel/src/elf/mod.rs b/kernel/src/elf/mod.rs index 9df1d73a336..41618d3d993 100644 --- a/kernel/src/elf/mod.rs +++ b/kernel/src/elf/mod.rs @@ -27,7 +27,10 @@ use crate::UserAddr; use toyos_elf::dynamic::{Dynamic, InitArray}; use toyos_elf::section::{SectionTable, SHT_DYNSYM}; use toyos_elf::sym::{Sym, SymTab}; -use toyos_elf::{rela, Extent, GnuHash, ImageRange, Layout, Rela, RelaTable, Reloc, RelocError, SymIndex, TlsSegment}; +use toyos_elf::{ + rela, Extent, GnuHash, ImageRange, Layout, ProgramHeaderTable, Rela, RelaTable, Reloc, RelocError, SymIndex, + TlsSegment, +}; /// `toyos_elf::MAX_TLS_ALIGN` must equal the kernel's largest page. const _: () = assert!(toyos_elf::MAX_TLS_ALIGN == PAGE_2M); @@ -101,6 +104,8 @@ pub struct LoadedLib { rules: rela::Rules, /// `PT_GNU_EH_FRAME`, for DWARF unwinding. pub eh_frame_hdr: Option, + /// The program header table, which `load_shared_lib` refuses a module without. + pub phdrs: ProgramHeaderTable, /// `DT_INIT_ARRAY`. pub init_array: Option, /// Bytes between the image's lowest and highest virtual address. @@ -355,6 +360,8 @@ pub fn load_shared_lib( if extent.min() != 0 { return Err("ELF: a shared object must begin at vaddr 0"); } + // `SYS_QUERY_MODULES` answers with the mapped table. + let phdrs = layout.program_headers().ok_or("ELF: no PT_LOAD maps the program header table")?; // No writable segment yields an empty window; no relocation can target it. let (rw_lo, rw_hi) = layout.writable_window().unwrap_or((layout.span(), layout.span())); @@ -513,6 +520,7 @@ pub fn load_shared_lib( cached_relocs: None, rules, eh_frame_hdr: layout.eh_frame_hdr(), + phdrs, init_array, span: layout.span(), rw_lo, diff --git a/kernel/src/loader/mod.rs b/kernel/src/loader/mod.rs index ddd821de37b..fffdbc7f936 100644 --- a/kernel/src/loader/mod.rs +++ b/kernel/src/loader/mod.rs @@ -396,6 +396,12 @@ pub fn spawn( // Where the image's first byte lands: every `ImageOffset` the file's // numbers were parsed into is added to it, and its span fits above it. let image_start = UserAddr::new(USER_VM_BASE); + // `SYS_QUERY_MODULES` answers with the mapped table, so an image without + // one has no true answer to give. + let Some(phdrs) = layout.program_headers() else { + log!("spawn: {}: no PT_LOAD maps the program header table", path); + return Err(SyscallError::InvalidArgument.into()); + }; let exe = read_exe_tables(backing.as_ref(), &layout, path)?; let t1 = crate::clock::nanos_since_boot(); @@ -597,6 +603,7 @@ pub fn spawn( .eh_frame_hdr() .map_or((0, 0), |r| ((image_start + r.start().get()).raw(), r.len())), exe_vaddr_max: image_end, + exe_phdrs: ((image_start + phdrs.image().start().get()).raw(), phdrs.count()), lib_paths, }, mmap_regions: Vec::new(), diff --git a/kernel/src/process.rs b/kernel/src/process.rs index f8680f7be27..b747865cee7 100644 --- a/kernel/src/process.rs +++ b/kernel/src/process.rs @@ -492,6 +492,8 @@ pub struct ElfInfo { pub exe_eh_frame_hdr: (u64, u64), /// One past the executable's last byte. pub exe_vaddr_max: u64, + /// The executable's program header table as (address, count), `(0, 0)` with no executable. + pub exe_phdrs: (u64, u16), /// Paths of dlopen'd libraries (parallel to loaded_libs). pub lib_paths: Vec, } @@ -510,6 +512,7 @@ impl ElfInfo { elf_base: UserAddr::new(0), exe_eh_frame_hdr: (0, 0), exe_vaddr_max: 0, + exe_phdrs: (0, 0), lib_paths: Vec::new(), } } diff --git a/kernel/src/syscall/vm.rs b/kernel/src/syscall/vm.rs index fb52c5345a4..2b593d4269b 100644 --- a/kernel/src/syscall/vm.rs +++ b/kernel/src/syscall/vm.rs @@ -484,11 +484,15 @@ pub(super) fn sys_query_modules(out: &mut UserBytesMut) -> u64 { let mut path_offset = (module_count * info_size) as u32; let (eh_addr, eh_size) = data.elf.exe_eh_frame_hdr; + let (phdr, phnum) = data.elf.exe_phdrs; let exe_info = ModuleInfo { base: data.elf.elf_base.raw(), text_end: data.elf.exe_vaddr_max, eh_frame_hdr: eh_addr, eh_frame_hdr_size: eh_size, + phdr, + phnum: phnum.into(), + phentsize: toyos_elf::header::PROGRAM_HEADER_SIZE as u32, path_offset, path_len: exe_path_bytes.len() as u32, }; @@ -507,6 +511,9 @@ pub(super) fn sys_query_modules(out: &mut UserBytesMut) -> u64 { text_end: lib.user_end(), eh_frame_hdr: lib.eh_frame_hdr.map_or(0, |r| (lib.user_base + r.start().get()).raw()), eh_frame_hdr_size: lib.eh_frame_hdr.map_or(0, |r| r.len()), + phdr: (lib.user_base + lib.phdrs.image().start().get()).raw(), + phnum: lib.phdrs.count().into(), + phentsize: toyos_elf::header::PROGRAM_HEADER_SIZE as u32, path_offset, path_len: lib_path_bytes.len() as u32, }; diff --git a/tests/testcases/LICENSE b/tests/testcases/LICENSE index 8eb68294c2e..1bb2732ad04 100644 --- a/tests/testcases/LICENSE +++ b/tests/testcases/LICENSE @@ -22,9 +22,9 @@ against tinycc upstream at 64552b3faa39ee7948a9ea21bfcc11045b90c70d (repo.or.cz/tinycc.git, 2026-08-05), allowing for the `-` → `_` renames this project made to some filenames. - tinycc/ 316 files: 239 byte-identical to tinycc's `tests/tests2`, + tinycc/ 318 files: 239 byte-identical to tinycc's `tests/tests2`, 17 tinycc files this project modified, - 60 not upstream at all — 59 cases written here, plus + 62 not upstream at all — 61 cases written here, plus `fred.txt`, which is output `40_stdio.c` writes when it runs and which was committed by accident. diff --git a/tests/testcases/tinycc/204_dl_iterate_phdr.c b/tests/testcases/tinycc/204_dl_iterate_phdr.c new file mode 100644 index 00000000000..d0f9b152c96 --- /dev/null +++ b/tests/testcases/tinycc/204_dl_iterate_phdr.c @@ -0,0 +1,139 @@ +/* dl_iterate_phdr visits the executable and every loaded library with the + program headers the loader mapped, and stops where its callback says. + Each module's addresses are checked against where its own code and + _DYNAMIC really are, not against the kernel's other answers. */ + +#include +#include +#include +#include +#include + +/* The image's test library; naming it is also what stages it beside this + case. */ +#define LIB "/system/lib/libtls_lib.so" + +extern const char _DYNAMIC[] __attribute__((visibility("hidden"))); +int main(void); + +static int failed; + +static void fail(const char *what, const char *name) { + printf("FAIL %s: \"%s\"\n", what, name); + failed = 1; +} + +/* The PT_LOAD holding [addr, addr + len) at dlpi_addr + p_vaddr, or NULL. */ +static const ElfW(Phdr) *load_holding(const struct dl_phdr_info *info, uintptr_t addr, size_t len) { + for (int i = 0; i < info->dlpi_phnum; i++) { + const ElfW(Phdr) *ph = &info->dlpi_phdr[i]; + uintptr_t lo = info->dlpi_addr + ph->p_vaddr; + if (ph->p_type == PT_LOAD && addr >= lo && addr + len <= lo + ph->p_memsz) + return ph; + } + return NULL; +} + +/* Whether `code` lies in an executable PT_LOAD of this module. */ +static int runs(const struct dl_phdr_info *info, const void *code) { + const ElfW(Phdr) *ph = load_holding(info, (uintptr_t)code, 1); + return ph && (ph->p_flags & PF_X); +} + +struct walk { + int visited; + int stop_at; + int stop_with; + void *lib_code; +}; + +static int visit(struct dl_phdr_info *info, size_t size, void *data) { + struct walk *walk = data; + walk->visited++; + if (walk->stop_at) + return walk->visited == walk->stop_at ? walk->stop_with : 0; + + const char *name = info->dlpi_name; + if (size != sizeof(struct dl_phdr_info)) + fail("the size passed is not the record's", name); + if (info->dlpi_phnum == 0 || info->dlpi_phdr == NULL) { + fail("no program headers", name); + return 0; + } + uintptr_t table = (uintptr_t)info->dlpi_phdr; + if (!load_holding(info, table, info->dlpi_phnum * sizeof(ElfW(Phdr)))) + fail("the program headers lie in no PT_LOAD", name); + for (int i = 0; i < info->dlpi_phnum; i++) { + const ElfW(Phdr) *ph = &info->dlpi_phdr[i]; + if (ph->p_type == PT_PHDR && info->dlpi_addr + ph->p_vaddr != table) + fail("PT_PHDR names another address", name); + } + + if (walk->visited == 1) { + if (strcmp(name, "") != 0) + fail("the executable is not first, or is not named by the empty string", name); + if (!runs(info, (const void *)main)) + fail("main lies in no executable PT_LOAD", name); + int dynamic = 0; + for (int i = 0; i < info->dlpi_phnum; i++) { + const ElfW(Phdr) *ph = &info->dlpi_phdr[i]; + if (ph->p_type != PT_DYNAMIC) + continue; + dynamic = 1; + if (info->dlpi_addr + ph->p_vaddr != (uintptr_t)_DYNAMIC) + fail("PT_DYNAMIC is not at _DYNAMIC", name); + if (!load_holding(info, info->dlpi_addr + ph->p_vaddr, ph->p_memsz)) + fail("PT_DYNAMIC lies in no PT_LOAD", name); + } + if (!dynamic) + fail("no PT_DYNAMIC", name); + printf("executable: \"%s\"\n", name); + } else { + if (strcmp(name, LIB) != 0) + fail("a library other than the one loaded", name); + if (!runs(info, walk->lib_code)) + fail("tls_get_label lies in no executable PT_LOAD", name); + printf("library: %s\n", name); + } + return 0; +} + +/* Walk every module, checking each; the count visited. */ +static int walk_all(void *lib_code) { + struct walk walk = { 0, 0, 0, lib_code }; + int ret = dl_iterate_phdr(visit, &walk); + if (ret != 0) { + printf("FAIL a walk no callback stopped returned %d\n", ret); + failed = 1; + } + return walk.visited; +} + +/* A walk the callback stops at module `at` with `with`. */ +static void stop(int at, int with, int of) { + struct walk walk = { 0, at, with, NULL }; + int ret = dl_iterate_phdr(visit, &walk); + printf("stopped at %d of %d: visited %d, returned %d\n", at, of, walk.visited, ret); + if (walk.visited != at || ret != with) + failed = 1; +} + +int main(void) { + printf("modules: %d\n", walk_all(NULL)); + stop(1, 1, 1); + + void *lib = dlopen(LIB, RTLD_NOW); + if (!lib) { + printf("FAIL dlopen %s\n", LIB); + return 1; + } + void *code = dlsym(lib, "tls_get_label"); + if (!code) { + printf("FAIL dlsym tls_get_label\n"); + return 1; + } + printf("modules: %d\n", walk_all(code)); + stop(1, 7, 2); + stop(2, -1, 2); + return failed; +} diff --git a/tests/testcases/tinycc/204_dl_iterate_phdr.expect b/tests/testcases/tinycc/204_dl_iterate_phdr.expect new file mode 100644 index 00000000000..a07ee1587bc --- /dev/null +++ b/tests/testcases/tinycc/204_dl_iterate_phdr.expect @@ -0,0 +1,8 @@ +executable: "" +modules: 1 +stopped at 1 of 1: visited 1, returned 1 +executable: "" +library: /system/lib/libtls_lib.so +modules: 2 +stopped at 1 of 2: visited 1, returned 7 +stopped at 2 of 2: visited 2, returned -1 diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index f471ce741fe..ea41632d77a 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -2184,6 +2184,13 @@ pub struct ModuleInfo { pub eh_frame_hdr: u64, /// Size of `.eh_frame_hdr` in bytes. pub eh_frame_hdr_size: u64, + /// Absolute virtual address of the module's program header table: the + /// kernel loads no module whose table a `PT_LOAD` does not map. + pub phdr: u64, + /// Entries in that table, `e_phnum`. + pub phnum: u32, + /// Bytes per entry, `e_phentsize`. + pub phentsize: u32, /// Byte offset of the module's path string within the buffer. pub path_offset: u32, /// Length of the path string in bytes. @@ -2193,11 +2200,11 @@ pub struct ModuleInfo { /// Every byte belongs to a field: this crosses the boundary through /// [`ModuleInfo::as_bytes`], so a gap would publish whatever the kernel stack /// held. **This is the type where that matters most**, because the buffer it -/// is written into is a user address: the two `u32`s sit at the end of four -/// `u64`s, which is 8 bytes together at an 8-aligned offset, so there is no -/// tail padding today — and this is what says so. A field of any other width -/// added here reds here rather than publishing kernel stack bytes to userland. -const _: () = assert!(core::mem::size_of::() == 8 + 8 + 8 + 8 + 4 + 4); +/// is written into is a user address: the four `u32`s pair up after five +/// `u64`s, 8 bytes a pair at an 8-aligned offset, so there is no padding — +/// and this is what says so. A field of any other width added here reds here +/// rather than publishing kernel stack bytes to userland. +const _: () = assert!(core::mem::size_of::() == 8 + 8 + 8 + 8 + 8 + 4 + 4 + 4 + 4); impl ModuleInfo { /// The record's own bytes, which is what `SYS_QUERY_MODULES` writes. @@ -2232,6 +2239,28 @@ pub fn query_modules(buf: &mut [u8]) -> Result { check(syscall(SYS_QUERY_MODULES, buf.as_mut_ptr() as u64, buf.len() as u64, 0, 0)).map(|n| n as usize) } +/// Every record of one [`query_modules`] answer with its path, executable +/// first: `answer` is the `n` bytes the call reported. +/// +/// A record or path outside `answer` is a kernel that broke the layout above, +/// and panics. +pub fn modules(answer: &[u8]) -> impl Iterator { + let size = core::mem::size_of::(); + let record = move |i: usize| { + let bytes = &answer[i * size..(i + 1) * size]; + // SAFETY: `bytes` holds `size_of::()` bytes, read without + // an alignment requirement, and every bit pattern of its integer + // fields is a `ModuleInfo`. + unsafe { (bytes.as_ptr() as *const ModuleInfo).read_unaligned() } + }; + let count = if answer.is_empty() { 0 } else { record(0).path_offset as usize / size }; + (0..count).map(move |i| { + let info = record(i); + let path = info.path_offset as usize; + (info, &answer[path..path + info.path_len as usize]) + }) +} + /// Scheduler info for the calling process. #[repr(C)] #[derive(Clone, Copy, Debug)] @@ -2316,17 +2345,73 @@ mod tests { text_end: 0x2233_4455_6677_8899, eh_frame_hdr: 0x3344_5566_7788_99aa, eh_frame_hdr_size: 0x44, + phdr: 0x5566_7788_99aa_bbcc, + phnum: 0x77, + phentsize: 0x88, path_offset: 0x55, path_len: 0x66, }; let b = info.as_bytes(); - assert_eq!(b.len(), 40); + assert_eq!(b.len(), 56); assert_eq!(u64::from_ne_bytes(b[0..8].try_into().unwrap()), info.base); assert_eq!(u64::from_ne_bytes(b[8..16].try_into().unwrap()), info.text_end); assert_eq!(u64::from_ne_bytes(b[16..24].try_into().unwrap()), info.eh_frame_hdr); assert_eq!(u64::from_ne_bytes(b[24..32].try_into().unwrap()), info.eh_frame_hdr_size); - assert_eq!(u32::from_ne_bytes(b[32..36].try_into().unwrap()), info.path_offset); - assert_eq!(u32::from_ne_bytes(b[36..40].try_into().unwrap()), info.path_len); + assert_eq!(u64::from_ne_bytes(b[32..40].try_into().unwrap()), info.phdr); + assert_eq!(u32::from_ne_bytes(b[40..44].try_into().unwrap()), info.phnum); + assert_eq!(u32::from_ne_bytes(b[44..48].try_into().unwrap()), info.phentsize); + assert_eq!(u32::from_ne_bytes(b[48..52].try_into().unwrap()), info.path_offset); + assert_eq!(u32::from_ne_bytes(b[52..56].try_into().unwrap()), info.path_len); + } + + fn record(base: u64, path_offset: u32, path_len: u32) -> ModuleInfo { + ModuleInfo { + base, + text_end: base + 0x1000, + eh_frame_hdr: 0, + eh_frame_hdr_size: 0, + phdr: base + 0x40, + phnum: 3, + phentsize: 56, + path_offset, + path_len, + } + } + + /// An answer laid out as the kernel writes one — records, then the paths + /// packed in module order — decodes to those records and paths, in order, + /// at an offset whatever alignment the buffer has. + #[test] + fn modules_decodes_every_record_and_its_path() { + let size = core::mem::size_of::() as u32; + let exe = record(0x100_0000_0000, 2 * size, 9); + let lib = record(0x200_0000_0000, 2 * size + 9, 13); + let mut answer = [0u8; 1 + 2 * 56 + 9 + 13]; + let wire = &mut answer[1..]; + wire[..56].copy_from_slice(exe.as_bytes()); + wire[56..112].copy_from_slice(lib.as_bytes()); + wire[112..121].copy_from_slice(b"/bin/prog"); + wire[121..].copy_from_slice(b"/lib/libx.so\0"); + let got: [(u64, u64, &[u8]); 2] = { + let mut it = modules(&answer[1..]).map(|(m, path)| (m.base, m.phdr, path)); + let pair = [it.next().unwrap(), it.next().unwrap()]; + assert!(it.next().is_none(), "two records, and no third read out of the paths"); + pair + }; + assert_eq!(got[0], (exe.base, exe.phdr, &b"/bin/prog"[..])); + assert_eq!(got[1], (lib.base, lib.phdr, &b"/lib/libx.so\0"[..])); + assert_eq!(modules(&[]).count(), 0); + } + + /// A path the kernel says runs past its own answer is a broken layout, + /// not a shorter name. + #[test] + #[should_panic] + fn modules_refuses_a_path_past_the_answer() { + let size = core::mem::size_of::() as u32; + let mut answer = [0u8; 56 + 4]; + answer[..56].copy_from_slice(record(0, size, 5).as_bytes()); + modules(&answer).for_each(drop); } /// Four lowercase hex digits each and nothing else, because two spellings diff --git a/toyos-elf/src/layout.rs b/toyos-elf/src/layout.rs index c51bab1660e..397acacd20d 100644 --- a/toyos-elf/src/layout.rs +++ b/toyos-elf/src/layout.rs @@ -10,8 +10,8 @@ //! address it placed the image at. A raw `p_vaddr` never leaves this module. use crate::header::{ - FileHeader, Machine, ProgramHeader, PT_DYNAMIC, PT_GNU_EH_FRAME, PT_LOAD, PT_TLS, - SECTION_HEADER_SIZE, + FileHeader, Machine, ProgramHeader, PROGRAM_HEADER_SIZE, PT_DYNAMIC, PT_GNU_EH_FRAME, PT_LOAD, + PT_TLS, SECTION_HEADER_SIZE, }; use crate::{Error, MAX_LOAD_SEGMENTS, MAX_TLS_ALIGN}; @@ -237,6 +237,29 @@ impl DynamicSegment { } } +/// Where the loaded image holds its own program header table. +/// +/// Derived from where a `PT_LOAD` copies `e_phoff` out of the file, not from +/// `PT_PHDR`: that header is the file's claim about this, and this is what the +/// loader actually puts there. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct ProgramHeaderTable { + image: ImageRange, + count: u16, +} + +impl ProgramHeaderTable { + /// The table's bytes, inside the file-backed part of one `PT_LOAD`. + pub const fn image(&self) -> ImageRange { + self.image + } + + /// `e_phnum`, at least one. + pub const fn count(&self) -> u16 { + self.count + } +} + /// Where the section header table is, when the file has a usable one. /// /// Section headers are optional metadata — symbol names for backtraces, and @@ -271,7 +294,8 @@ impl SectionTableRef { /// - the extent runs from the smallest `p_vaddr` to the largest /// `p_vaddr + p_memsz` over those segments, so it covers every one of them; /// - the entry point, the file-backed extent of `PT_TLS`, and all of -/// `PT_DYNAMIC` and `PT_GNU_EH_FRAME`, lie inside the extent; +/// `PT_DYNAMIC` and `PT_GNU_EH_FRAME`, lie inside the extent, and the +/// program header table, when one is held, inside one `PT_LOAD`'s file bytes; /// - the TLS alignment is zero or a power of two no larger than /// [`MAX_TLS_ALIGN`], so that `!(align - 1)` is a mask and the TLS block's /// size cannot be dominated by a number the file chose. @@ -289,6 +313,7 @@ pub struct Layout { dynamic: Option, section_headers: Option, eh_frame_hdr: Option, + program_headers: Option, } impl Layout { @@ -410,6 +435,7 @@ impl Layout { None => None, Some(e) => Some(extent.range(e.vaddr, e.memsz).ok_or(Error::EhFrameOutsideImage)?), }; + let program_headers = program_header_table(&ehdr, segments.get(..placed).unwrap_or(&[])); Ok(Layout { extent, @@ -420,6 +446,7 @@ impl Layout { dynamic, section_headers: section_table(&ehdr), eh_frame_hdr, + program_headers, }) } @@ -460,6 +487,13 @@ impl Layout { self.eh_frame_hdr } + /// Where the loaded image holds the program header table, or `None` when + /// no `PT_LOAD`'s file bytes hold all of it and the table exists only in + /// the file. + pub const fn program_headers(&self) -> Option { + self.program_headers + } + /// The writable window `[lo, hi)` in image offsets, or `None` when no /// segment is writable. /// @@ -560,6 +594,24 @@ impl Layout { } } +/// The program header table as the first `PT_LOAD` whose file bytes hold all +/// of it places it in the image. +/// +/// Only `p_filesz` counts: past it a segment is zeroes, not the file. +fn program_header_table(ehdr: &FileHeader, segments: &[Segment]) -> Option { + let len = u64::from(ehdr.phnum).checked_mul(PROGRAM_HEADER_SIZE as u64)?; + segments.iter().find_map(|seg| { + let within = ehdr.phoff.checked_sub(seg.file_offset)?; + if within.checked_add(len)? > seg.filesz { + return None; + } + // Inside the segment's own range, which the extent holds: `within + + // len <= filesz <= memsz`. + let start = seg.image.start.checked_add(within)?; + Some(ProgramHeaderTable { image: ImageRange { start, len }, count: ehdr.phnum }) + }) +} + /// A section header table the loader can index, or `None`. /// /// `e_shentsize` must be exactly 64: consumers divide a byte count by it and diff --git a/toyos-elf/src/lib.rs b/toyos-elf/src/lib.rs index 46ee094f9e3..1d9685d7c65 100644 --- a/toyos-elf/src/lib.rs +++ b/toyos-elf/src/lib.rs @@ -47,7 +47,7 @@ pub use gnu_hash::GnuHash; pub use header::{FileHeader, Machine}; pub use layout::{ DynamicSegment, Extent, ImageOffset, ImageRange, Layout, Segment, SegmentFlags, - SectionTableRef, StackedImage, TlsSegment, + ProgramHeaderTable, SectionTableRef, StackedImage, TlsSegment, }; pub use rela::{Op, Rela, RelaCounts, RelaTable, Reloc, RelocError, RelocKind, Rules, TlsRef}; pub use section::{SectionHeader, SectionTable}; diff --git a/toyos-elf/tests/crafted.rs b/toyos-elf/tests/crafted.rs index baff9336062..3edf06af72c 100644 --- a/toyos-elf/tests/crafted.rs +++ b/toyos-elf/tests/crafted.rs @@ -347,6 +347,52 @@ fn the_file_bytes_behind_a_vaddr_are_the_containing_segments() { assert_eq!(layout.file_bytes_from(0x2400), Some(0x400)); } +/// `(image offset, bytes, count)` of the table a layout places, for comparing. +fn table_of(layout: &Layout) -> Option<(u64, u64, u16)> { + layout.program_headers().map(|t| (t.image().start().get(), t.image().len(), t.count())) +} + +/// The table is where the segment holding its file bytes puts them, measured +/// from the image's lowest address, whichever segment that is. +#[test] +fn the_program_header_table_is_where_its_segment_places_it() { + let two = 2 * PROGRAM_HEADER_SIZE as u64; + let at_zero = accepted(Elf::new(0x1000).ph(Phdr::load(0, 0, 0x1000, 0x1000, PF_R)).ph(Phdr::tls(0x800, 0, 8, 8)).build()); + assert_eq!(table_of(&at_zero), Some((PH_OFF as u64, two, 2))); + + // The first segment does not hold the table and the second does, at a + // vaddr above the first's: the offset is from the extent's minimum. + let second = accepted( + Elf::new(0x2000) + .entry(0x1000) + .ph(Phdr::load(0x1000, 0x1000, 0x1000, 0x1000, PF_R | PF_X)) + .ph(Phdr::load(0, 0x10_0000, 0x1000, 0x1000, PF_R)) + .build(), + ); + assert_eq!(table_of(&second), Some((0x10_0000 + PH_OFF as u64 - 0x1000, two, 2))); +} + +/// Held only where the file's bytes are: a segment whose file image stops +/// inside the table, or one that holds it only as zero-filled memory, holds +/// no table. +#[test] +fn a_table_no_segment_holds_whole_is_absent() { + let one = PROGRAM_HEADER_SIZE as u64; + let short = PH_OFF as u64 + one; + // Two headers; the file image ends one header in. + let cut = Elf::new(0x1000).ph(Phdr::load(0, 0, short, 0x1000, PF_R)).ph(Phdr::tls(0x800, 0, 8, 8)); + assert_eq!(table_of(&accepted(cut.build())), None); + // Exactly the table's end and it is held. + let whole = Elf::new(0x1000).ph(Phdr::load(0, 0, short + one, 0x1000, PF_R)).ph(Phdr::tls(0x800, 0, 8, 8)); + assert_eq!(table_of(&accepted(whole.build())), Some((PH_OFF as u64, 2 * one, 2))); + // Memory but no file bytes. + let zeroes = Elf::new(0x1000).ph(Phdr::load(0, 0, 0, 0x1000, PF_R)); + assert_eq!(table_of(&accepted(zeroes.build())), None); + // A segment that starts past the table's first byte. + let after = Elf::new(0x2000).entry(0x1000).ph(Phdr::load(PH_OFF as u64 + 8, 0x1000, 0x1000, 0x1000, PF_R)); + assert_eq!(table_of(&accepted(after.build())), None); +} + #[test] fn the_writable_window_spans_every_writable_segment() { let layout = accepted( diff --git a/toyos-elf/tests/fuzz.rs b/toyos-elf/tests/fuzz.rs index 871ba3973b1..2314fb05f15 100644 --- a/toyos-elf/tests/fuzz.rs +++ b/toyos-elf/tests/fuzz.rs @@ -309,6 +309,9 @@ fn load(case: &Case, placement: Placement, reached: &mut Reached) -> Result<(), assert!(offset <= span, "offset {offset:#x} past the span {span:#x}"); image_start.checked_add(offset).expect("an offset inside the span leaves the placement") }; + if let Some(table) = layout.program_headers() { + place(table.image().end().get()); + } let dyn_bytes = at(&case.bytes, layout.dynamic().ok_or(())?.image()); let dynamic = Dynamic::parse(dyn_bytes); diff --git a/toyos-elf/tests/real.rs b/toyos-elf/tests/real.rs index 6da7bdaa5a9..3fe9636746b 100644 --- a/toyos-elf/tests/real.rs +++ b/toyos-elf/tests/real.rs @@ -37,6 +37,11 @@ fn a_toyos_ld_binary_parses_to_what_readelf_says() { assert_eq!(layout.tls().unwrap().memsz(), 0x90); assert_eq!(layout.tls().unwrap().align(), 0x40); + // `e_phoff` 0x40, six headers of 56 bytes, inside the text segment's file + // bytes at offset 0. + let table = layout.program_headers().map(|t| (t.image().start().get(), t.image().len(), t.count())); + assert_eq!(table, Some((0x40, 6 * 56, 6))); + let sections = layout.section_headers().expect("a section header table"); assert_eq!((sections.count, sections.entry_size), (9, 64)); diff --git a/userland/libc/include/elf.h b/userland/libc/include/elf.h new file mode 100644 index 00000000000..cd1d16a787d --- /dev/null +++ b/userland/libc/include/elf.h @@ -0,0 +1,44 @@ +#ifndef _ELF_H +#define _ELF_H + +/* The program header vocabulary needs, as the System V gABI spells + it; no other part of the gABI is declared here. */ + +#include + +typedef uint64_t Elf64_Addr; +typedef uint64_t Elf64_Off; +typedef uint16_t Elf64_Half; +typedef uint32_t Elf64_Word; +typedef int32_t Elf64_Sword; +typedef uint64_t Elf64_Xword; +typedef int64_t Elf64_Sxword; + +typedef struct { + Elf64_Word p_type; + Elf64_Word p_flags; + Elf64_Off p_offset; + Elf64_Addr p_vaddr; + Elf64_Addr p_paddr; + Elf64_Xword p_filesz; + Elf64_Xword p_memsz; + Elf64_Xword p_align; +} Elf64_Phdr; + +#define PT_NULL 0 +#define PT_LOAD 1 +#define PT_DYNAMIC 2 +#define PT_INTERP 3 +#define PT_NOTE 4 +#define PT_SHLIB 5 +#define PT_PHDR 6 +#define PT_TLS 7 +#define PT_GNU_EH_FRAME 0x6474e550 +#define PT_GNU_STACK 0x6474e551 +#define PT_GNU_RELRO 0x6474e552 + +#define PF_X 0x1 +#define PF_W 0x2 +#define PF_R 0x4 + +#endif diff --git a/userland/libc/include/link.h b/userland/libc/include/link.h new file mode 100644 index 00000000000..2eef0a3203d --- /dev/null +++ b/userland/libc/include/link.h @@ -0,0 +1,22 @@ +#ifndef _LINK_H +#define _LINK_H + +#include +#include + +#define ElfW(type) Elf64_##type + +/* The first four fields of glibc's layout. There is no dlpi_adds or + dlpi_subs, so the size a callback is passed ends at dlpi_phnum. + dlpi_name is "" for the executable, and lives only until the callback + returns. */ +struct dl_phdr_info { + ElfW(Addr) dlpi_addr; + const char *dlpi_name; + const ElfW(Phdr) *dlpi_phdr; + ElfW(Half) dlpi_phnum; +}; + +int dl_iterate_phdr(int (*callback)(struct dl_phdr_info *info, size_t size, void *data), void *data); + +#endif diff --git a/userland/libc/src/lib.rs b/userland/libc/src/lib.rs index 9acf206adf3..1052461cf53 100644 --- a/userland/libc/src/lib.rs +++ b/userland/libc/src/lib.rs @@ -6,6 +6,7 @@ extern crate alloc; mod arch; mod ctype; mod errno; +mod link; mod math; mod memory; mod misc; diff --git a/userland/libc/src/link.rs b/userland/libc/src/link.rs new file mode 100644 index 00000000000..7faffb24762 --- /dev/null +++ b/userland/libc/src/link.rs @@ -0,0 +1,61 @@ +//! `dl_iterate_phdr`, from the kernel's `SYS_QUERY_MODULES` answer. +//! +//! The contract is glibc's `dl_iterate_phdr(3)`, which the LSB adopts: the +//! executable first, named by the empty string, then every library in load +//! order; the walk stops at the first callback that returns non-zero and +//! returns that value, else 0. The walk is over one answer, so a module a +//! callback or another thread loads meanwhile is not visited. + +use alloc::vec::Vec; + +use toyos_abi::syscall; + +/// `struct dl_phdr_info`, as `link.h` declares it. +#[repr(C)] +pub struct DlPhdrInfo { + dlpi_addr: u64, + dlpi_name: *const u8, + dlpi_phdr: *const u8, + dlpi_phnum: u16, +} + +type Callback = unsafe extern "C" fn(*mut DlPhdrInfo, usize, *mut u8) -> i32; + +#[no_mangle] +pub unsafe extern "C" fn dl_iterate_phdr(callback: Callback, data: *mut u8) -> i32 { + let answer = answer(); + let mut name = Vec::new(); + for (i, (module, path)) in syscall::modules(&answer).enumerate() { + name.clear(); + if i > 0 { + name.extend_from_slice(path); + } + name.push(0); + let mut info = DlPhdrInfo { + dlpi_addr: module.base, + dlpi_name: name.as_ptr(), + dlpi_phdr: module.phdr as *const u8, + dlpi_phnum: u16::try_from(module.phnum).expect("SYS_QUERY_MODULES: a phnum past e_phnum's u16"), + }; + // SAFETY: the caller's function, handed a record valid for the call. + let stop = unsafe { callback(&mut info, core::mem::size_of::(), data) }; + if stop != 0 { + return stop; + } + } + 0 +} + +/// One whole `SYS_QUERY_MODULES` answer: asked again while a `dlopen` +/// elsewhere grows it between the size and the read. +fn answer() -> Vec { + let mut buf = Vec::new(); + loop { + let need = syscall::query_modules(&mut buf).expect("SYS_QUERY_MODULES refused a buffer this process owns"); + if need <= buf.len() { + buf.truncate(need); + return buf; + } + buf.resize(need, 0); + } +} From bf35e83344af7f86d474d495ab787235fb5d0b61 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 08:41:46 +0200 Subject: [PATCH 2/6] libc: a dlopen handle is the module index plus one A C program with no startup libraries gets module index 0 from its first SYS_DLOPEN, and libc handed that back as the pointer 0, so dlopen reported failure for a load the kernel had committed. 204_dl_iterate_phdr reds on exactly that: the kernel log shows the module registered (its DTPMOD64 relocations applied under module_id 2), and the case prints "FAIL dlopen /system/lib/libtls_lib.so". The log's "dlopen: unresolved: main" is not the refusal: libtls_lib.so carries a global undefined `main`, the C executable exports none, and resolve_dlopen_relocs logs and leaves an unresolved slot by contract (kernel/src/elf/reloc.rs's module header). libc now hands out index + 1 and takes one off in dlsym and dlclose. dlsym(RTLD_DEFAULT) panics as unimplemented instead of reading module 0, and dlclose of a handle dlopen never returned answers -1. Co-Authored-By: Claude Opus 5.5 --- userland/libc/src/misc.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/userland/libc/src/misc.rs b/userland/libc/src/misc.rs index fee86cf4a2e..4443d6cd7c8 100644 --- a/userland/libc/src/misc.rs +++ b/userland/libc/src/misc.rs @@ -432,13 +432,20 @@ pub unsafe extern "C" fn longjmp(_env: *mut u8, _val: i32) -> ! { } // dlopen/dlsym/dlclose +// +// A C handle is the kernel's module index plus one: index 0 is a module, and +// NULL is dlopen's failure. + +fn module_index(handle: *mut u8) -> Option { + (handle as u64).checked_sub(1) +} #[no_mangle] pub unsafe extern "C" fn dlopen(path: *const u8, _flags: i32) -> *mut u8 { if path.is_null() { return ptr::null_mut(); } let path_bytes = super::posix_io::c_str_to_bytes(path); match syscall::dl_open(path_bytes) { - Ok(handle) => handle as *mut u8, + Ok(index) => (index + 1) as *mut u8, Err(_) => ptr::null_mut(), } } @@ -446,9 +453,10 @@ pub unsafe extern "C" fn dlopen(path: *const u8, _flags: i32) -> *mut u8 { #[no_mangle] pub unsafe extern "C" fn dlsym(handle: *mut u8, symbol: *const u8) -> *mut u8 { if symbol.is_null() { return ptr::null_mut(); } + let index = module_index(handle).expect("dlsym: RTLD_DEFAULT not implemented"); let name = super::posix_io::c_str_to_bytes(symbol); // SAFETY: handle is from a prior dlopen, name is a valid C string - match unsafe { syscall::dl_sym(handle as u64, name) } { + match unsafe { syscall::dl_sym(index, name) } { Ok(addr) => addr as *mut u8, Err(_) => ptr::null_mut(), } @@ -456,7 +464,10 @@ pub unsafe extern "C" fn dlsym(handle: *mut u8, symbol: *const u8) -> *mut u8 { #[no_mangle] pub unsafe extern "C" fn dlclose(handle: *mut u8) -> i32 { - syscall::dl_close(handle as u64) as i32 + match module_index(handle) { + Some(index) => syscall::dl_close(index) as i32, + None => -1, + } } #[no_mangle] From 11b89c8d68eabe70eb86b657ecc71e6959eafeb9 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 09:07:44 +0200 Subject: [PATCH 3/6] File what 204_dl_iterate_phdr's second round turned up A dlopen'ed library resolves only against other libraries, never the executable's exports; and one host gate run on this branch reddened two build-system tests on a TempDir removal that a plain rerun of the step did not reproduce. Co-Authored-By: Claude Opus 5.5 --- ...on-a-tempdir-something-still-wrote-into.md | 26 +++++++++++++++++++ ...-never-binds-to-the-executables-exports.md | 15 +++++++++++ 2 files changed, 41 insertions(+) create mode 100644 issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md create mode 100644 issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md diff --git a/issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md b/issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md new file mode 100644 index 00000000000..02f59a0ad89 --- /dev/null +++ b/issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md @@ -0,0 +1,26 @@ +--- +status: open +kind: tooling +opened: 2026-09-29 +--- + +# Two build-system tests red once on a TempDir something was still writing into + +One `cargo run -- --ci host` on `wt/toyos-phdr` (head `bf35e833`) reddened +"the build system: cargo test --lib" with two failures. In both, the test body +passed and `TempDir`'s drop (`toyos-tmpdir/src/lib.rs`, `fail`) panicked +because the directory was not empty when it removed it: + +- `compiler::tests::llvm_and_the_tools_move_the_key`: `remove + .../toyos-tmp-73906-0/compiler-key-33: Directory not empty (os error 66)`; +- `sysroot::tests::a_worktree_pinning_another_fork_commit_gets_its_own_checkout`: + the same error on the process root `.../toyos-tmp-73906-0`. + +Both tests drive `git` (commits, `update-index`, a worktree with a nested +submodule) inside the directory before it is removed. Something outside the +test body still wrote into it between the last entry `remove_dir_all` listed +and the final `rmdir`; what that is was not identified. + +A plain `cargo test --lib` on the same tree right after exited 0 with 408 +passed, so this is intermittent. The branch changes nothing under `src/` or +`toyos-tmpdir/`. diff --git a/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md b/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md new file mode 100644 index 00000000000..c313272119e --- /dev/null +++ b/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md @@ -0,0 +1,15 @@ +--- +status: open +kind: defect +opened: 2026-09-29 +--- + +# A dlopen'ed library never binds to the executable's exports + +`resolve_dlopen_relocs` (`kernel/src/elf/reloc.rs`) resolves a `dlopen`ed +module's `GLOB_DAT`/`JUMP_SLOT` slots against the other loaded libraries only. +Its startup sibling, `resolve_lib_bind_relocs`, tries the executable's +dynamic exports first (`kernel/src/loader/mod.rs` builds `exe_sym_map` for +it). glibc's `dlopen` searches the global scope, which starts with the +executable, so on ToyOS a plugin that calls back into a symbol its host +exports is left with an unresolved slot and faults when it uses it. From 04a4216312b635a89d8a17730fc216060f59b270 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 11:23:24 +0200 Subject: [PATCH 4/6] PR #599 round 3: both unmapped-table refusals are reached, and an executable off vaddr 0 checks its table Review of 11b89c8d, SEND BACK. BLOCKER 1. abuse_elf_loader gains case 21, `phdrs_unmapped`: a 0x2000-byte ELF whose only PT_LOAD is file 0x1000..0x2000 at vaddr 0, entry 0. The table at e_phoff 0x40 lies in no segment's file bytes, and vaddr_min is 0, so `rebase_base` and `load_shared_lib`'s vaddr-0 check pass it; the same bytes are spawned (`spawn_refused`) and dlopened (`dlopen_refused`). toyos-elf's crafted tests pin the premise on the host: `Layout::parse` accepts those bytes with extent min 0 and no table. BLOCKER 2. 205_dl_iterate_phdr_image_base is linked with -Wl,--image-base=0x200000 (tests/common/compile.rs, LINK_FLAGS, keyed by case name, applied in `link_toyos`, which every C link goes through). It compares the executable's dlpi_phdr with __ehdr_start + e_phoff and dlpi_phnum with e_phnum, both read PC-relative from the linker's own header, and __ehdr_start with dlpi_addr + the lowest p_vaddr. It prints that lowest p_vaddr, so a flag that stops reaching the case reds its .expect. Linked on the host with the toolchain's clang: llvm-readobj -h -l gives e_phoff 0x40, 10 headers, first PT_LOAD at offset 0 vaddr 0x200000, PT_PHDR at 0x200040; llvm-nm gives __ehdr_start 0x200000. Corpus licence: 318 -> 320 files. NOTEs. - ModuleInfo drops `phentsize`: Layout refuses any e_phentsize but 56 and nothing read it. `phnum` becomes a u64: after six u64s the two u32 path fields fill one 8-byte slot, so the record stays 56 bytes with no padding, where a u32 phnum would have left 4 bytes of tail padding the const assert refuses. - issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md names the backtrace crate's `native_libraries` beside std's `load_modules`, with both exits. - issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md gains its exit. - issues/build/libc-dlsym-rtld-default-panics-as-unimplemented.md records dlsym(RTLD_DEFAULT) dying as unimplemented. REMOVEs. - The tempdir-flake issue goes: #600 (bab6ed80, merged in with origin/main 7e151819) fixed the ENOTEMPTY it recorded. - toyos-abi's field-count sentence over ModuleInfo's const assert goes. - elf.h's opening comment goes. Co-Authored-By: Claude Opus 5.5 --- ...ym-rtld-default-panics-as-unimplemented.md | 20 +++++++ ...on-a-tempdir-something-still-wrote-into.md | 26 --------- ...ry-modules-byte-count-as-a-module-count.md | 29 ++++++---- ...-never-binds-to-the-executables-exports.md | 5 ++ kernel/src/syscall/vm.rs | 2 - tests/common/compile.rs | 8 +++ tests/testcases/LICENSE | 4 +- .../tinycc/205_dl_iterate_phdr_image_base.c | 53 +++++++++++++++++++ .../205_dl_iterate_phdr_image_base.expect | 4 ++ .../src/bin/abuse_elf_loader.rs | 8 +++ toyos-abi/src/syscall.rs | 19 +++---- toyos-elf/tests/crafted.rs | 4 ++ userland/libc/include/elf.h | 3 -- 13 files changed, 128 insertions(+), 57 deletions(-) create mode 100644 issues/build/libc-dlsym-rtld-default-panics-as-unimplemented.md delete mode 100644 issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md create mode 100644 tests/testcases/tinycc/205_dl_iterate_phdr_image_base.c create mode 100644 tests/testcases/tinycc/205_dl_iterate_phdr_image_base.expect diff --git a/issues/build/libc-dlsym-rtld-default-panics-as-unimplemented.md b/issues/build/libc-dlsym-rtld-default-panics-as-unimplemented.md new file mode 100644 index 00000000000..824956cdc9f --- /dev/null +++ b/issues/build/libc-dlsym-rtld-default-panics-as-unimplemented.md @@ -0,0 +1,20 @@ +--- +status: open +kind: defect +opened: 2026-09-29 +--- + +# libc's `dlsym(RTLD_DEFAULT, …)` panics as unimplemented + +`dlfcn.h` defines `RTLD_DEFAULT` as the null handle, and libc's `dlsym` +(`userland/libc/src/misc.rs`) panics on it: a handle is a module index plus +one, and `SYS_DLSYM` answers from one loaded library by index. Nothing searches +the global scope — the executable's exports, then every library in load order — +which is what glibc's `dlsym(RTLD_DEFAULT, …)` answers from. A C program that +probes the process for an optional symbol dies. The kernel-side sibling is +`issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md`: +both want the executable inside the global scope. + +**Exit**: `dlsym(RTLD_DEFAULT, name)` answers the first definition of `name` in +the executable and then every loaded library in load order, and NULL when there +is none, and a guest C case asserts both. diff --git a/issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md b/issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md deleted file mode 100644 index 02f59a0ad89..00000000000 --- a/issues/build/two-build-system-tests-red-once-on-a-tempdir-something-still-wrote-into.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-09-29 ---- - -# Two build-system tests red once on a TempDir something was still writing into - -One `cargo run -- --ci host` on `wt/toyos-phdr` (head `bf35e833`) reddened -"the build system: cargo test --lib" with two failures. In both, the test body -passed and `TempDir`'s drop (`toyos-tmpdir/src/lib.rs`, `fail`) panicked -because the directory was not empty when it removed it: - -- `compiler::tests::llvm_and_the_tools_move_the_key`: `remove - .../toyos-tmp-73906-0/compiler-key-33: Directory not empty (os error 66)`; -- `sysroot::tests::a_worktree_pinning_another_fork_commit_gets_its_own_checkout`: - the same error on the process root `.../toyos-tmp-73906-0`. - -Both tests drive `git` (commits, `update-index`, a worktree with a nested -submodule) inside the directory before it is removed. Something outside the -test body still wrote into it between the last entry `remove_dir_all` listed -and the final `rmdir`; what that is was not identified. - -A plain `cargo test --lib` on the same tree right after exited 0 with 408 -passed, so this is intermittent. The branch changes nothing under `src/` or -`toyos-tmpdir/`. diff --git a/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md b/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md index 597cfafaa2d..206991a2b55 100644 --- a/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md +++ b/issues/diagnostics/std-reads-a-query-modules-byte-count-as-a-module-count.md @@ -6,19 +6,26 @@ opened: 2026-09-29 # std reads a `SYS_QUERY_MODULES` byte count as a module count -The std fork's unwinder (`rust/library/std/src/sys/pal/toyos/mod.rs`, -`eh_frame::load_modules`) takes `query_modules`'s `Ok(n)` as a record count -and reads `size_of::()`-byte records from offset 0 until one would -pass the end of its 4096-byte buffer. `n` counts **bytes** -(`toyos_abi::syscall::query_modules`'s doc), so every "record" after the real -ones is the packed path strings and then the zeroed tail of the buffer, read -as `base`/`text_end`/`eh_frame_hdr`. It also takes `n > buf.len()`, which means -nothing was written, as an answer and reads the zeroes. +Two readers in the std fork take `query_modules`'s `Ok(n)` as a record count +and read `size_of::()`-byte records from offset 0 until one would +pass the end of their 4096-byte buffer: + +- the unwinder, `rust/library/std/src/sys/pal/toyos/mod.rs`, + `eh_frame::load_modules`, reads each as `base`/`text_end`/`eh_frame_hdr`; +- the backtrace crate, `rust/library/backtrace/src/symbolize/gimli/libs_toyos.rs`, + `native_libraries`, makes a `Library` of each whose one segment is + `text_end - base` bytes long, a subtraction that can underflow on them. + +`n` counts **bytes** (`toyos_abi::syscall::query_modules`'s doc), so every +"record" after the real ones is the packed path strings and then the zeroed +tail of the buffer. Both also take `n > buf.len()`, which means nothing was +written, as an answer and read the zeroes: the call answers a short buffer +with `Ok`, so neither ever reaches the `Err` arm that grows its buffer. No wrong frame has been seen: a path's ASCII read as a `u64` lies far above the user half, so no user PC falls in the range it makes, and a zeroed record's range is empty. That is luck in the bytes, not a check. -**Exit**: `load_modules` decodes the answer with `toyos_abi::syscall::modules` -over the `n` bytes the call reported, and grows the buffer to `n` when `n` -exceeds it. +**Exit**: `load_modules` and `native_libraries` each decode the answer with +`toyos_abi::syscall::modules` over the `n` bytes the call reported, and grow +the buffer to `n` when `n` exceeds it. diff --git a/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md b/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md index c313272119e..1e6eff8d8ba 100644 --- a/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md +++ b/issues/kernel/a-dlopened-library-never-binds-to-the-executables-exports.md @@ -13,3 +13,8 @@ dynamic exports first (`kernel/src/loader/mod.rs` builds `exe_sym_map` for it). glibc's `dlopen` searches the global scope, which starts with the executable, so on ToyOS a plugin that calls back into a symbol its host exports is left with an unresolved slot and faults when it uses it. + +**Exit**: `resolve_dlopen_relocs` binds a slot against the executable's dynamic +exports before the other loaded libraries, as `resolve_lib_bind_relocs` does, +and a guest case `dlopen`s a library that calls a function its executable +exports, and the call lands. diff --git a/kernel/src/syscall/vm.rs b/kernel/src/syscall/vm.rs index 2b593d4269b..fb2b798f0a6 100644 --- a/kernel/src/syscall/vm.rs +++ b/kernel/src/syscall/vm.rs @@ -492,7 +492,6 @@ pub(super) fn sys_query_modules(out: &mut UserBytesMut) -> u64 { eh_frame_hdr_size: eh_size, phdr, phnum: phnum.into(), - phentsize: toyos_elf::header::PROGRAM_HEADER_SIZE as u32, path_offset, path_len: exe_path_bytes.len() as u32, }; @@ -513,7 +512,6 @@ pub(super) fn sys_query_modules(out: &mut UserBytesMut) -> u64 { eh_frame_hdr_size: lib.eh_frame_hdr.map_or(0, |r| r.len()), phdr: (lib.user_base + lib.phdrs.image().start().get()).raw(), phnum: lib.phdrs.count().into(), - phentsize: toyos_elf::header::PROGRAM_HEADER_SIZE as u32, path_offset, path_len: lib_path_bytes.len() as u32, }; diff --git a/tests/common/compile.rs b/tests/common/compile.rs index 745ec8a3b0b..6fb4004cfda 100644 --- a/tests/common/compile.rs +++ b/tests/common/compile.rs @@ -88,6 +88,13 @@ pub fn compile_c(name: &str) -> Vec { .collect() } +/// Link flags a case needs beyond the corpus's, by case name. +/// +/// An image whose lowest address is 0 hides a table reported from the load bias +/// instead of the image's first byte; the case prints its lowest `PT_LOAD`, so +/// an entry that stops reaching its case reds it. +const LINK_FLAGS: &[(&str, &[&str])] = &[("205_dl_iterate_phdr_image_base", &["-Wl,--image-base=0x200000"])]; + /// Link `objects` into a ToyOS executable through the clang driver — which /// names `ld.lld` and the sysroot's `libtoyos_c.a`, and makes a PIE — and /// return its bytes. @@ -97,6 +104,7 @@ pub fn link_toyos(objects: &[PathBuf], name: &str) -> Vec { let output = Command::new(&c.clang) .args(c.args()) .args(objects) + .args(LINK_FLAGS.iter().filter(|(case, _)| *case == name).flat_map(|(_, flags)| flags.iter())) .arg("-o") .arg(&out) .output() diff --git a/tests/testcases/LICENSE b/tests/testcases/LICENSE index 1bb2732ad04..2e02f9cc01c 100644 --- a/tests/testcases/LICENSE +++ b/tests/testcases/LICENSE @@ -22,9 +22,9 @@ against tinycc upstream at 64552b3faa39ee7948a9ea21bfcc11045b90c70d (repo.or.cz/tinycc.git, 2026-08-05), allowing for the `-` → `_` renames this project made to some filenames. - tinycc/ 318 files: 239 byte-identical to tinycc's `tests/tests2`, + tinycc/ 320 files: 239 byte-identical to tinycc's `tests/tests2`, 17 tinycc files this project modified, - 62 not upstream at all — 61 cases written here, plus + 64 not upstream at all — 63 cases written here, plus `fred.txt`, which is output `40_stdio.c` writes when it runs and which was committed by accident. diff --git a/tests/testcases/tinycc/205_dl_iterate_phdr_image_base.c b/tests/testcases/tinycc/205_dl_iterate_phdr_image_base.c new file mode 100644 index 00000000000..2e5ade2ee92 --- /dev/null +++ b/tests/testcases/tinycc/205_dl_iterate_phdr_image_base.c @@ -0,0 +1,53 @@ +/* dl_iterate_phdr reports the executable's program headers where they are when + its lowest address is not 0: tests/common/compile.rs links this case at + --image-base=0x200000, so the load bias and the image's first byte differ. + Every fact the answer is checked against is the linker's, reached through + __ehdr_start and never through the kernel's answer. */ + +#include +#include +#include +#include + +extern const unsigned char __ehdr_start[] __attribute__((visibility("hidden"))); + +/* The executable is visited first; keep its record and stop. */ +static int first(struct dl_phdr_info *info, size_t size, void *data) { + (void)size; + *(struct dl_phdr_info *)data = *info; + return 1; +} + +static int failed; + +static void check(int holds, const char *what) { + printf("%s: %s\n", what, holds ? "yes" : "no"); + if (!holds) + failed = 1; +} + +int main(void) { + struct dl_phdr_info exe; + if (dl_iterate_phdr(first, &exe) != 1) { + printf("FAIL the walk did not stop at the executable\n"); + return 1; + } + + /* e_phoff and e_phnum, at their System V gABI offsets in Elf64_Ehdr. */ + uint64_t phoff; + uint16_t phnum; + memcpy(&phoff, __ehdr_start + 32, sizeof phoff); + memcpy(&phnum, __ehdr_start + 56, sizeof phnum); + const ElfW(Phdr) *table = (const ElfW(Phdr) *)(__ehdr_start + phoff); + + uintptr_t lowest = UINTPTR_MAX; + for (int i = 0; i < phnum; i++) + if (table[i].p_type == PT_LOAD && table[i].p_vaddr < lowest) + lowest = table[i].p_vaddr; + printf("lowest PT_LOAD: 0x%lx\n", (unsigned long)lowest); + + check(exe.dlpi_phnum == phnum, "dlpi_phnum is e_phnum"); + check(exe.dlpi_phdr == table, "dlpi_phdr is __ehdr_start + e_phoff"); + check((uintptr_t)__ehdr_start == exe.dlpi_addr + lowest, "__ehdr_start is dlpi_addr + the lowest p_vaddr"); + return failed; +} diff --git a/tests/testcases/tinycc/205_dl_iterate_phdr_image_base.expect b/tests/testcases/tinycc/205_dl_iterate_phdr_image_base.expect new file mode 100644 index 00000000000..17a85849748 --- /dev/null +++ b/tests/testcases/tinycc/205_dl_iterate_phdr_image_base.expect @@ -0,0 +1,4 @@ +lowest PT_LOAD: 0x200000 +dlpi_phnum is e_phnum: yes +dlpi_phdr is __ehdr_start + e_phoff: yes +__ehdr_start is dlpi_addr + the lowest p_vaddr: yes diff --git a/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs b/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs index 5e7b540b16e..7491a80b4eb 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs @@ -554,6 +554,14 @@ fn main() { // 20. The apply-time TLS refusals, each named by its reason in the log. tls_apply_time_refusals_are_reached(); + // 21. A program header table no `PT_LOAD` maps: the one segment's file + // bytes start at 0x1000, past the table at 0x40. Its vaddr is 0, so + // `rebase_base` and `load_shared_lib`'s vaddr-0 check pass it and only + // the table refuses it, in `spawn` and in `load_shared_lib` alike. + let unmapped = Elf::new(0x2000).ph(Phdr::load(0x1000, 0, 0x1000, 0x1000, PF_R | PF_X)).entry(0).build(); + spawn_refused("phdrs_unmapped", &unmapped); + dlopen_refused("phdrs_unmapped.so", &unmapped); + // The kernel heap is intact: allocate and touch enough to walk it, then // prove the real loader still works. let mut blocks: Vec> = Vec::new(); diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index d1223ce3215..4a84a27c473 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -2188,10 +2188,8 @@ pub struct ModuleInfo { /// Absolute virtual address of the module's program header table: the /// kernel loads no module whose table a `PT_LOAD` does not map. pub phdr: u64, - /// Entries in that table, `e_phnum`. - pub phnum: u32, - /// Bytes per entry, `e_phentsize`. - pub phentsize: u32, + /// Entries in that table, `e_phnum`: a `u64` so the record has no padding. + pub phnum: u64, /// Byte offset of the module's path string within the buffer. pub path_offset: u32, /// Length of the path string in bytes. @@ -2201,11 +2199,9 @@ pub struct ModuleInfo { /// Every byte belongs to a field: this crosses the boundary through /// [`ModuleInfo::as_bytes`], so a gap would publish whatever the kernel stack /// held. **This is the type where that matters most**, because the buffer it -/// is written into is a user address: the four `u32`s pair up after five -/// `u64`s, 8 bytes a pair at an 8-aligned offset, so there is no padding — -/// and this is what says so. A field of any other width added here reds here -/// rather than publishing kernel stack bytes to userland. -const _: () = assert!(core::mem::size_of::() == 8 + 8 + 8 + 8 + 8 + 4 + 4 + 4 + 4); +/// is written into is a user address. A field of any other width added here +/// reds here rather than publishing kernel stack bytes to userland. +const _: () = assert!(core::mem::size_of::() == 8 + 8 + 8 + 8 + 8 + 8 + 4 + 4); impl ModuleInfo { /// The record's own bytes, which is what `SYS_QUERY_MODULES` writes. @@ -2348,7 +2344,6 @@ mod tests { eh_frame_hdr_size: 0x44, phdr: 0x5566_7788_99aa_bbcc, phnum: 0x77, - phentsize: 0x88, path_offset: 0x55, path_len: 0x66, }; @@ -2359,8 +2354,7 @@ mod tests { assert_eq!(u64::from_ne_bytes(b[16..24].try_into().unwrap()), info.eh_frame_hdr); assert_eq!(u64::from_ne_bytes(b[24..32].try_into().unwrap()), info.eh_frame_hdr_size); assert_eq!(u64::from_ne_bytes(b[32..40].try_into().unwrap()), info.phdr); - assert_eq!(u32::from_ne_bytes(b[40..44].try_into().unwrap()), info.phnum); - assert_eq!(u32::from_ne_bytes(b[44..48].try_into().unwrap()), info.phentsize); + assert_eq!(u64::from_ne_bytes(b[40..48].try_into().unwrap()), info.phnum); assert_eq!(u32::from_ne_bytes(b[48..52].try_into().unwrap()), info.path_offset); assert_eq!(u32::from_ne_bytes(b[52..56].try_into().unwrap()), info.path_len); } @@ -2373,7 +2367,6 @@ mod tests { eh_frame_hdr_size: 0, phdr: base + 0x40, phnum: 3, - phentsize: 56, path_offset, path_len, } diff --git a/toyos-elf/tests/crafted.rs b/toyos-elf/tests/crafted.rs index 3edf06af72c..4d44324f914 100644 --- a/toyos-elf/tests/crafted.rs +++ b/toyos-elf/tests/crafted.rs @@ -391,6 +391,10 @@ fn a_table_no_segment_holds_whole_is_absent() { // A segment that starts past the table's first byte. let after = Elf::new(0x2000).entry(0x1000).ph(Phdr::load(PH_OFF as u64 + 8, 0x1000, 0x1000, 0x1000, PF_R)); assert_eq!(table_of(&accepted(after.build())), None); + // And one at vaddr 0, so a loader's vaddr-0 checks pass and only the table + // refuses it: `abuse_elf_loader`'s `phdrs_unmapped`. + let unmapped = accepted(Elf::new(0x2000).ph(Phdr::load(0x1000, 0, 0x1000, 0x1000, PF_R | PF_X)).build()); + assert_eq!((unmapped.extent().min(), table_of(&unmapped)), (0, None)); } #[test] diff --git a/userland/libc/include/elf.h b/userland/libc/include/elf.h index cd1d16a787d..e6ff35bb2ee 100644 --- a/userland/libc/include/elf.h +++ b/userland/libc/include/elf.h @@ -1,9 +1,6 @@ #ifndef _ELF_H #define _ELF_H -/* The program header vocabulary needs, as the System V gABI spells - it; no other part of the gABI is declared here. */ - #include typedef uint64_t Elf64_Addr; From af62dd52588de42f45e3037412fc47641025fa81 Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 12:57:44 +0200 Subject: [PATCH 5/6] abuse_elf_loader: dlopen_refused says only what every case shares Its doc named relocation writes as what each case is, which the unmapped-table case (21) is not. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01U6SVYFkdvV2t38KzNrESxs --- tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs b/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs index 7491a80b4eb..e471c1e74f8 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs @@ -276,9 +276,7 @@ fn dlopen_survives(name: &str, bytes: &[u8]) { drop(unsafe { libloading::Library::new(&path) }); } -/// `dlopen` must refuse this file. A success is the defect: each case here is -/// an image whose relocations would be written through a range the loader is -/// at that moment holding a `&[u8]` over, or into a page it maps `ReadExec`. +/// `dlopen` must refuse this file. A success is the defect. fn dlopen_refused(name: &str, bytes: &[u8]) { let path = write_file(name, bytes); match unsafe { libloading::Library::new(&path) } { From 293af009e6cf23e74c317ac43bbb314149b11b5e Mon Sep 17 00:00:00 2001 From: japabu Date: Tue, 29 Sep 2026 13:58:49 +0200 Subject: [PATCH 6/6] dl_iterate_phdr: 204 ties every module's table to its own ELF header 204 now reads the ELF header at each reported module's dlpi_addr and checks dlpi_phnum against e_phnum and dlpi_phdr against dlpi_addr + e_phoff, as 205 does for the executable through __ehdr_start. Before this, a kernel that reported a library's phnum as 3, or its table 56 bytes late, passed every check 204 made: the first three headers of libtls_lib.so are PT_PHDR and two PT_LOADs, and the shifted table still starts with a PT_LOAD. Both modules 204 walks are linked at vaddr 0 with their first PT_LOAD at file offset 0 and e_phoff 0x40 (llvm-readobj -h -l on the host-linked 204 and on libtls_lib.so), so the header is at dlpi_addr. toyos-elf's program_header_table computes the table's length and its image start with plain arithmetic: a u16 count of 56-byte entries cannot overflow, and the start is inside the segment's own range. A broken invariant now panics under the kernel's and bootloader's overflow checks instead of reading as a table no segment holds. Filed issues/build/libc-dl-iterate-phdr-allocates-on-every-walk.md: every walk allocates twice, so libunwind looking up a frame with the heap exhausted dies in libc's allocator. The answer grows with every library and path, so a fixed buffer does not hold it without an ABI change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01U6SVYFkdvV2t38KzNrESxs --- ...dl-iterate-phdr-allocates-on-every-walk.md | 19 +++++++++++++++ tests/testcases/tinycc/204_dl_iterate_phdr.c | 23 ++++++++++++++++++- toyos-elf/src/layout.rs | 5 ++-- 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 issues/build/libc-dl-iterate-phdr-allocates-on-every-walk.md diff --git a/issues/build/libc-dl-iterate-phdr-allocates-on-every-walk.md b/issues/build/libc-dl-iterate-phdr-allocates-on-every-walk.md new file mode 100644 index 00000000000..59c15cb81f4 --- /dev/null +++ b/issues/build/libc-dl-iterate-phdr-allocates-on-every-walk.md @@ -0,0 +1,19 @@ +--- +status: open +kind: defect +opened: 2026-09-29 +--- + +# libc's `dl_iterate_phdr` allocates on every walk + +`userland/libc/src/link.rs` sizes and reads a whole `SYS_QUERY_MODULES` +answer into a heap `Vec`, and builds each module's NUL-terminated name in a +second one: every walk allocates twice and makes at least two syscalls. +libunwind calls `dl_iterate_phdr` to find a frame's unwind tables, so a C++ +`throw std::bad_alloc` with the heap exhausted dies in libc's allocator +instead of reaching its handler. The answer's size grows with every library +and path, so no fixed buffer holds it: an allocation-free walk needs a query whose +answer a bounded buffer holds, which is an ABI change. + +**Exit**: `dl_iterate_phdr` makes no heap allocation, and a guest C case walks +every module with `malloc` exhausted and visits each. diff --git a/tests/testcases/tinycc/204_dl_iterate_phdr.c b/tests/testcases/tinycc/204_dl_iterate_phdr.c index d0f9b152c96..da1989087b7 100644 --- a/tests/testcases/tinycc/204_dl_iterate_phdr.c +++ b/tests/testcases/tinycc/204_dl_iterate_phdr.c @@ -1,6 +1,6 @@ /* dl_iterate_phdr visits the executable and every loaded library with the program headers the loader mapped, and stops where its callback says. - Each module's addresses are checked against where its own code and + Each module's addresses are checked against where its own header, code and _DYNAMIC really are, not against the kernel's other answers. */ #include @@ -34,6 +34,26 @@ static const ElfW(Phdr) *load_holding(const struct dl_phdr_info *info, uintptr_t return NULL; } +/* The table and its count are the ones the module's own ELF header names: + every module here is linked at vaddr 0 with its first PT_LOAD at file + offset 0, so that header is at dlpi_addr. */ +static void header_names_the_table(const struct dl_phdr_info *info, const char *name) { + const unsigned char *ehdr = (const unsigned char *)info->dlpi_addr; + if (memcmp(ehdr, "\177ELF", 4) != 0) { + fail("no ELF header at dlpi_addr", name); + return; + } + /* e_phoff and e_phnum, at their System V gABI offsets in Elf64_Ehdr. */ + uint64_t phoff; + uint16_t phnum; + memcpy(&phoff, ehdr + 32, sizeof phoff); + memcpy(&phnum, ehdr + 56, sizeof phnum); + if (info->dlpi_phnum != phnum) + fail("dlpi_phnum is not e_phnum", name); + if ((uintptr_t)info->dlpi_phdr != info->dlpi_addr + phoff) + fail("dlpi_phdr is not dlpi_addr + e_phoff", name); +} + /* Whether `code` lies in an executable PT_LOAD of this module. */ static int runs(const struct dl_phdr_info *info, const void *code) { const ElfW(Phdr) *ph = load_holding(info, (uintptr_t)code, 1); @@ -60,6 +80,7 @@ static int visit(struct dl_phdr_info *info, size_t size, void *data) { fail("no program headers", name); return 0; } + header_names_the_table(info, name); uintptr_t table = (uintptr_t)info->dlpi_phdr; if (!load_holding(info, table, info->dlpi_phnum * sizeof(ElfW(Phdr)))) fail("the program headers lie in no PT_LOAD", name); diff --git a/toyos-elf/src/layout.rs b/toyos-elf/src/layout.rs index 397acacd20d..18068cb0db0 100644 --- a/toyos-elf/src/layout.rs +++ b/toyos-elf/src/layout.rs @@ -599,7 +599,8 @@ impl Layout { /// /// Only `p_filesz` counts: past it a segment is zeroes, not the file. fn program_header_table(ehdr: &FileHeader, segments: &[Segment]) -> Option { - let len = u64::from(ehdr.phnum).checked_mul(PROGRAM_HEADER_SIZE as u64)?; + // A `u16` count of 56-byte entries: the product cannot overflow a `u64`. + let len = u64::from(ehdr.phnum) * PROGRAM_HEADER_SIZE as u64; segments.iter().find_map(|seg| { let within = ehdr.phoff.checked_sub(seg.file_offset)?; if within.checked_add(len)? > seg.filesz { @@ -607,7 +608,7 @@ fn program_header_table(ehdr: &FileHeader, segments: &[Segment]) -> Option