Hello,
This is the final patch in the series (5/5). This patch makes the boot_script parser and the userland-boot ELF loader more secure against malformed binary files, resource exhaustion and unchecked Mach/VM system calls. Key Changes: Error Handling and Loop Protection in the Script Parser: Parsing errors (BOOT_SCRIPT_INVALID_SYM) are now properly reported rather than being ignored, and a depth-tracking feature has been added to provide protection against infinite symbol reference loops during symbol resolution. Robust File I/O (read_full and load_fail): Direct read() calls have been replaced with read_full() to handle partial reads and EINTR errors correctly. A dedicated load_fail() helper function has been added to properly terminate failed operations and log diagnostic messages without relying on assertions. ELF and Segment Boundary Validation: Comprehensive validation has been added for ELF programme headers (hdr.e_phnum), segment boundaries based on file size (fstat), and power-of-two alignment checks (p_align) before memory is allocated or segments are written to the task space. Mach Task and Memory Hardening: The process of generating synthesised linker names has been limited to prevent infinite loops. The return values of all memory and thread management calls (vm_allocate, vm_write, thread_create and thread_resume) have been checked; this ensures proper clean-up and deallocation of memory in the event of an error. The patch is provided below for your reference; Please await the next email for further information notes. >From 6a9ac9915e800ddc9fe597d17f84bae441d13f30 Mon Sep 17 00:00:00 2001 From: Alperen ERKAN <[email protected]> Date: Sat, 16 Sep 2026 12:50:33 +0300 Subject: [PATCH 5/5] boot: harden boot script parser and userland loader In boot_script.c, propagate parse errors correctly and guard against symbol reference cycles while resolving symbol values. In userland-boot.c, read files fully, validate ELF program headers and segment bounds before loading, bound the synthesized port name, and check the vm/thread operations in boot_script_exec_cmd. --- hurd/boot/boot_script.c | 18 ++++- hurd/boot/userland-boot.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 215 insertions(+), 64 deletions(-) diff --git a/hurd/boot/boot_script.c b/hurd/boot/boot_script.c --- a/hurd/boot/boot_script.c +++ b/hurd/boot/boot_script.c @@ -1,6 +2,7 @@ /* Boot script parser for Mach. */ /* Written by Shantanu Goel ([email protected]). */ +/* Copyright (C) 2026 Alperen ERKAN */ #include <mach/mach_types.h> #if !KERNEL || OSKIT_MACH @@ -340,7 +343,10 @@ boot_script_parse_line (void *hook, char *cmdline) /* Only values are allowed in ${...} constructs. */ if (end_char == '}' && s->type == VAL_FUNC) - return BOOT_SCRIPT_INVALID_SYM; + { + error = BOOT_SCRIPT_INVALID_SYM; + goto bad; + } /* Check that assignment is valid. */ if (c == '=' && s->type == VAL_FUNC) @@ -559,9 +567,17 @@ boot_script_exec (void) { struct sym *sym = (struct sym *) arg->val; - /* Resolve symbol value. */ - while (sym->type == VAL_SYM) + /* Resolve symbol value. Guard against reference + cycles. */ + unsigned int depth = 0; + while (sym->type == VAL_SYM + && depth++ <= (unsigned int) symtab_index) sym = (struct sym *) sym->val; + if (sym->type == VAL_SYM) + { + error = BOOT_SCRIPT_SYNTAX_ERROR; + goto done; + } if (sym->type == VAL_NONE) { error = BOOT_SCRIPT_UNDEF_SYM; diff --git a/hurd/boot/userland-boot.c b/hurd/boot/userland-boot.c --- a/hurd/boot/userland-boot.c +++ b/hurd/boot/userland-boot.c @@ -1,5 +2,6 @@ /* boot_script.c support functions for running in a Mach user task. Copyright (C) 2001 Free Software Foundation, Inc. + Copyright (C) 2026 Alperen ERKAN This file is part of the GNU Hurd. @@ -25,9 +27,11 @@ #include <mach/machine/vm_param.h> /* For VM_XXX_ADDRESS */ #include <mach/gnumach.h> /* For task_set_name */ #include <stdlib.h> +#include <stdint.h> #include <stdio.h> #include <string.h> #include <sys/mman.h> +#include <sys/stat.h> #include <unistd.h> #include <errno.h> #include <error.h> @@ -39,6 +77,44 @@ #include "boot_script.h" #include "private.h" +/* Read exactly LEN bytes from FD, returning 0 on success. */ +static int +read_full (int fd, void *buf, size_t len) +{ + char *p = buf; + while (len > 0) + { + ssize_t n = read (fd, p, len); + if (n == 0) + return -1; + if (n < 0) + { + if (errno == EINTR) + continue; + return -1; + } + p += n; + len -= n; + } + return 0; +} + +static void __attribute__ ((__noreturn__)) +load_fail (task_t t, const char *file) +{ + char msg[] = ": truncated or unreadable bootstrap file\n"; + size_t len = strlen (file); + ssize_t err; + do + err = write (2, file, len); + while (err < 0 && errno == EINTR); + do + err = write (2, msg, sizeof msg - 1); + while (err < 0 && errno == EINTR); + task_terminate (t); + exit (1); +} + void * boot_script_malloc (unsigned int size) { @@ -163,9 +168,14 @@ boot_script_insert_right (struct cmd *cmd, mach_port_t port, mach_port_t *name) *name = MACH_PORT_NULL; do { + if (*name >= (1 << 20)) + { + error (0, ENOMEM, "%s: mach_port_insert_right", cmd->path); + return BOOT_SCRIPT_MACH_ERROR; + } *name += 1; err = mach_port_insert_right (cmd->task, - *name, port, MACH_MSG_TYPE_COPY_SEND); + *name, port, MACH_MSG_TYPE_COPY_SEND); } while (err == KERN_NAME_EXISTS); @@ -239,44 +300,105 @@ load_image (task_t t, exit (1); } - err = read (fd, &hdr, sizeof hdr); - assert_backtrace (err == (sizeof hdr)); + if (read_full (fd, &hdr, sizeof hdr) < 0) + { + close (fd); + load_fail (t, file); + } /* File must have magic ELF number. */ if (hdr.e.e_ident[0] == 0177 && hdr.e.e_ident[1] == 'E' && hdr.e.e_ident[2] == 'L' && hdr.e.e_ident[3] == 'F') { - ElfW(Phdr) phdrs[hdr.e.e_phnum], *ph; - lseek (fd, hdr.e.e_phoff, SEEK_SET); - err = read (fd, phdrs, sizeof phdrs); - assert_backtrace (err == (sizeof phdrs)); - for (ph = phdrs; ph < &phdrs[sizeof phdrs/sizeof phdrs[0]]; ++ph) - if (ph->p_type == PT_LOAD) + /* Refuse pathological or extended program-header counts; + we do not support PN_XNUM. */ + if (hdr.e.e_phnum == 0 || hdr.e.e_phnum >= PN_XNUM) + { + close (fd); + load_fail (t, file); + } + + { + struct stat st; + ElfW(Phdr) *phdrs, *ph; + + if (fstat (fd, &st) < 0 + || hdr.e.e_phoff > (uintmax_t) st.st_size + || (uintmax_t) hdr.e.e_phnum * sizeof (ElfW(Phdr)) + > (uintmax_t) st.st_size - hdr.e.e_phoff) + { + close (fd); + load_fail (t, file); + } + + phdrs = malloc (hdr.e.e_phnum * sizeof (ElfW(Phdr))); + if (! phdrs) + { + close (fd); + load_fail (t, file); + } + if (lseek (fd, hdr.e.e_phoff, SEEK_SET) < 0 + || read_full (fd, phdrs, + hdr.e.e_phnum * sizeof (ElfW(Phdr))) < 0) { - vm_address_t buf; - vm_size_t offs = ph->p_offset & (ph->p_align - 1); - vm_size_t bufsz = round_page (ph->p_filesz + offs); - - buf = (vm_address_t) mmap (0, bufsz, - PROT_READ|PROT_WRITE, MAP_ANON, 0, 0); - assert_backtrace (buf != MAP_FAILED); - - lseek (fd, ph->p_offset, SEEK_SET); - err = read (fd, (void *)(buf + offs), ph->p_filesz); - assert_backtrace (err == (ph->p_filesz)); - - ph->p_memsz = ((ph->p_vaddr + ph->p_memsz + ph->p_align - 1) - & ~(ph->p_align - 1)); - ph->p_vaddr &= ~(ph->p_align - 1); - ph->p_memsz -= ph->p_vaddr; - - vm_allocate (t, (vm_address_t*)&ph->p_vaddr, ph->p_memsz, 0); - vm_write (t, ph->p_vaddr, buf, bufsz); - munmap ((caddr_t) buf, bufsz); - vm_protect (t, ph->p_vaddr, ph->p_memsz, 0, - ((ph->p_flags & PF_R) ? VM_PROT_READ : 0) | - ((ph->p_flags & PF_W) ? VM_PROT_WRITE : 0) | - ((ph->p_flags & PF_X) ? VM_PROT_EXECUTE : 0)); + free (phdrs); + close (fd); + load_fail (t, file); } + for (ph = phdrs; ph < &phdrs[hdr.e.e_phnum]; ++ph) + if (ph->p_type == PT_LOAD) + { + /* A segment must lie within the file and have a + valid, power-of-two alignment. */ + if (ph->p_align == 0 + || (ph->p_align & (ph->p_align - 1)) != 0 + || ph->p_offset > (uintmax_t) st.st_size + || (uintmax_t) ph->p_filesz + > (uintmax_t) st.st_size - ph->p_offset + || ph->p_memsz < ph->p_filesz) + continue; + + { + vm_address_t buf; + vm_size_t offs = ph->p_offset & (ph->p_align - 1); + vm_size_t bufsz = round_page (ph->p_filesz + offs); + + buf = (vm_address_t) mmap (0, bufsz, PROT_READ|PROT_WRITE, + MAP_ANON, 0, 0); + if (buf == MAP_FAILED + || lseek (fd, ph->p_offset, SEEK_SET) < 0 + || read_full (fd, (void *) (buf + offs), ph->p_filesz) < 0) + { + if (buf != MAP_FAILED) + munmap ((caddr_t) buf, bufsz); + free (phdrs); + close (fd); + load_fail (t, file); + } + + ph->p_memsz = ((ph->p_vaddr + ph->p_memsz + ph->p_align - 1) + & ~(ph->p_align - 1)); + ph->p_vaddr &= ~(ph->p_align - 1); + ph->p_memsz -= ph->p_vaddr; + + if (vm_allocate (t, (vm_address_t *) &ph->p_vaddr, ph->p_memsz, + 0) + || vm_write (t, ph->p_vaddr, buf, bufsz)) + { + munmap ((caddr_t) buf, bufsz); + free (phdrs); + close (fd); + load_fail (t, file); + } + munmap ((caddr_t) buf, bufsz); + vm_protect (t, ph->p_vaddr, ph->p_memsz, 0, + ((ph->p_flags & PF_R) ? VM_PROT_READ : 0) | + ((ph->p_flags & PF_W) ? VM_PROT_WRITE : 0) | + ((ph->p_flags & PF_X) ? VM_PROT_EXECUTE : 0)); + } + } + free (phdrs); + } + close (fd); return hdr.e.e_entry; } else @@ -357,8 +361,12 @@ load_image (task_t t, buf = mmap (0, rndamount, PROT_READ|PROT_WRITE, MAP_ANON, 0, 0); assert_backtrace (buf != MAP_FAILED); lseek (fd, sizeof hdr.a - headercruft, SEEK_SET); - err = read (fd, buf, amount); - assert_backtrace (err == amount); + if (read_full (fd, buf, amount) < 0) + { + munmap ((caddr_t) buf, rndamount); + close (fd); + load_fail (t, file); + } vm_allocate (t, &base, rndamount, 0); vm_write (t, base, (vm_address_t) buf, rndamount); if (magic != OMAGIC) @@ -372,59 +382,69 @@ load_image (task_t t, bssstart = base + hdr.a.a_text + hdr.a.a_data + headercruft; bsspagestart = round_page (bssstart); - vm_allocate (t, &bsspagestart, - hdr.a.a_bss - (bsspagestart - bssstart), 0); + if (hdr.a.a_bss > bsspagestart - bssstart) + vm_allocate (t, &bsspagestart, + hdr.a.a_bss - (bsspagestart - bssstart), 0); + close (fd); return hdr.a.a_entry; } } +static void +write_str (const char *msg, size_t len) +{ + ssize_t err; + do + err = write (2, msg, len); + while (err < 0 && errno == EINTR); +} + int boot_script_exec_cmd (void *hook, mach_port_t task, char *path, int argc, char **argv, char *strings, int stringlen) { char *args, *p; - int arg_len, i; + int i; + size_t arg_len, len; mach_msg_type_number_t reg_size; void *arg_pos; vm_offset_t stack_start, stack_end; vm_address_t startpc, str_start; thread_t thread; - ssize_t err; - size_t len; + error_t err; len = strlen (path); - err = write (2, path, len); - assert_backtrace (err == len); + write_str (path, len); for (i = 1; i < argc; ++i) { int quote = !! index (argv[i], ' ') || !! index (argv[i], '\t'); - err = write (2, " ", 1); - assert_backtrace (err == 1); + write_str (" ", 1); if (quote) - { - err = write (2, "\"", 1); - assert_backtrace (err == 1); - } + write_str ("\"", 1); len = strlen (argv[i]); - err = write (2, argv[i], len); - assert_backtrace (err == len); + write_str (argv[i], len); if (quote) - { - err = write (2, "\"", 1); - assert_backtrace (err == 1); - } + write_str ("\"", 1); } - err = write (2, "\r\n", 2); - assert_backtrace (err == 2); + write_str ("\r\n", 2); startpc = load_image (task, path); arg_len = stringlen + (argc + 2) * sizeof (char *) + sizeof (intptr_t); arg_len += 5 * sizeof (intptr_t); + if (arg_len > 16 * 1024 * 1024) + { + error (0, ENOMEM, "%s: argument list too long", path); + return BOOT_SCRIPT_EXEC_ERROR; + } stack_end = VM_MAX_ADDRESS; stack_start = VM_MAX_ADDRESS - 16 * 1024 * 1024; - vm_allocate (task, &stack_start, stack_end - stack_start, FALSE); + if (vm_allocate (task, &stack_start, stack_end - stack_start, FALSE)) + { + error (0, ENOMEM, "%s: vm_allocate", path); + return BOOT_SCRIPT_EXEC_ERROR; + } arg_pos = (void *) ((stack_end - arg_len) & ~(sizeof (intptr_t) - 1)); args = mmap (0, stack_end - trunc_page ((vm_offset_t) arg_pos), PROT_READ|PROT_WRITE, MAP_ANON, 0, 0); @@ -455,12 +467,24 @@ boot_script_exec_cmd (void *hook, p = (void *) p + sizeof (char *); memcpy (p, strings, stringlen); memset (args, 0, (vm_offset_t)arg_pos & (vm_page_size - 1)); - vm_write (task, trunc_page ((vm_offset_t) arg_pos), (vm_address_t) args, - stack_end - trunc_page ((vm_offset_t) arg_pos)); + if (vm_write (task, trunc_page ((vm_offset_t) arg_pos), + (vm_address_t) args, + stack_end - trunc_page ((vm_offset_t) arg_pos))) + { + error (0, ENOMEM, "%s: vm_write", path); + munmap ((caddr_t) args, + stack_end - trunc_page ((vm_offset_t) arg_pos)); + return BOOT_SCRIPT_EXEC_ERROR; + } munmap ((caddr_t) args, stack_end - trunc_page ((vm_offset_t) arg_pos)); - thread_create (task, &thread); + err = thread_create (task, &thread); + if (err) + { + error (0, err, "%s: thread_create", path); + return BOOT_SCRIPT_EXEC_ERROR; + } #ifdef i386_THREAD_STATE_COUNT { struct i386_thread_state regs; @@ -515,7 +521,13 @@ boot_script_exec_cmd (void *hook, # error needs to be ported #endif - thread_resume (thread); + err = thread_resume (thread); + if (err) + { + error (0, err, "%s: thread_resume", path); + mach_port_deallocate (mach_task_self (), thread); + return BOOT_SCRIPT_EXEC_ERROR; + } mach_port_deallocate (mach_task_self (), thread); return 0; } -- 2.43.0
