Changeset View
Standalone View
sys/kern/imgact_elf.c
Show All 28 Lines | |||||
*/ | */ | ||||
#include <sys/cdefs.h> | #include <sys/cdefs.h> | ||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include "opt_capsicum.h" | #include "opt_capsicum.h" | ||||
#include "opt_compat.h" | #include "opt_compat.h" | ||||
#include "opt_core.h" | #include "opt_core.h" | ||||
#include "opt_pax.h" | |||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/capsicum.h> | #include <sys/capsicum.h> | ||||
#include <sys/exec.h> | #include <sys/exec.h> | ||||
#include <sys/fcntl.h> | #include <sys/fcntl.h> | ||||
#include <sys/imgact.h> | #include <sys/imgact.h> | ||||
#include <sys/imgact_elf.h> | #include <sys/imgact_elf.h> | ||||
#include <sys/kernel.h> | #include <sys/kernel.h> | ||||
#include <sys/lock.h> | #include <sys/lock.h> | ||||
#include <sys/malloc.h> | #include <sys/malloc.h> | ||||
#include <sys/mount.h> | #include <sys/mount.h> | ||||
#include <sys/mman.h> | #include <sys/mman.h> | ||||
#include <sys/namei.h> | #include <sys/namei.h> | ||||
#include <sys/pioctl.h> | #include <sys/pioctl.h> | ||||
#include <sys/jail.h> | |||||
#include <sys/pax.h> | |||||
#include <sys/proc.h> | #include <sys/proc.h> | ||||
#include <sys/procfs.h> | #include <sys/procfs.h> | ||||
#include <sys/racct.h> | #include <sys/racct.h> | ||||
#include <sys/resourcevar.h> | #include <sys/resourcevar.h> | ||||
#include <sys/rwlock.h> | #include <sys/rwlock.h> | ||||
#include <sys/sbuf.h> | #include <sys/sbuf.h> | ||||
#include <sys/sf_buf.h> | #include <sys/sf_buf.h> | ||||
#include <sys/smp.h> | #include <sys/smp.h> | ||||
▲ Show 20 Lines • Show All 592 Lines • ▼ Show 20 Lines | #endif | ||||
if (error) | if (error) | ||||
goto fail; | goto fail; | ||||
error = exec_map_first_page(imgp); | error = exec_map_first_page(imgp); | ||||
if (error) | if (error) | ||||
goto fail; | goto fail; | ||||
/* | /* | ||||
* Also make certain that the interpreter stays the same, so set | * Also make certain that the interpreter stays the same, so set | ||||
* its VV_TEXT flag, too. | * its VV_TEXT flag, too. | ||||
*/ | */ | ||||
VOP_SET_TEXT(nd->ni_vp); | VOP_SET_TEXT(nd->ni_vp); | ||||
imp: This change is gratuitous and should be removed since it also doesn't follow style(9). | |||||
Not Done Inline ActionsLeftover cruft. Can certainly remove. lattera-gmail.com: Leftover cruft. Can certainly remove. | |||||
imgp->object = nd->ni_vp->v_object; | imgp->object = nd->ni_vp->v_object; | ||||
hdr = (const Elf_Ehdr *)imgp->image_header; | hdr = (const Elf_Ehdr *)imgp->image_header; | ||||
if ((error = __elfN(check_header)(hdr)) != 0) | if ((error = __elfN(check_header)(hdr)) != 0) | ||||
goto fail; | goto fail; | ||||
Not Done Inline ActionsThis should be done seprately. imp: This should be done seprately. | |||||
Not Done Inline ActionsAgreed. lattera-gmail.com: Agreed. | |||||
if (hdr->e_type == ET_DYN) | if (hdr->e_type == ET_DYN) | ||||
rbase = *addr; | rbase = *addr; | ||||
else if (hdr->e_type == ET_EXEC) | else if (hdr->e_type == ET_EXEC) | ||||
rbase = 0; | rbase = 0; | ||||
else { | else { | ||||
error = ENOEXEC; | error = ENOEXEC; | ||||
goto fail; | goto fail; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 118 Lines • ▼ Show 20 Lines | __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp) | ||||
if (brand_info == NULL) { | if (brand_info == NULL) { | ||||
uprintf("ELF binary type \"%u\" not known.\n", | uprintf("ELF binary type \"%u\" not known.\n", | ||||
hdr->e_ident[EI_OSABI]); | hdr->e_ident[EI_OSABI]); | ||||
return (ENOEXEC); | return (ENOEXEC); | ||||
} | } | ||||
if (hdr->e_type == ET_DYN) { | if (hdr->e_type == ET_DYN) { | ||||
if ((brand_info->flags & BI_CAN_EXEC_DYN) == 0) | if ((brand_info->flags & BI_CAN_EXEC_DYN) == 0) | ||||
return (ENOEXEC); | return (ENOEXEC); | ||||
/* | } | ||||
* Honour the base load address from the dso if it is | |||||
* non-zero for some reason. | |||||
*/ | |||||
if (baddr == 0) | |||||
et_dyn_addr = ET_DYN_LOAD_ADDR; | |||||
else | |||||
et_dyn_addr = 0; | |||||
} else | |||||
et_dyn_addr = 0; | |||||
sv = brand_info->sysvec; | sv = brand_info->sysvec; | ||||
if (interp != NULL && brand_info->interp_newpath != NULL) | if (interp != NULL && brand_info->interp_newpath != NULL) | ||||
newinterp = brand_info->interp_newpath; | newinterp = brand_info->interp_newpath; | ||||
/* | /* | ||||
* Avoid a possible deadlock if the current address space is destroyed | * Avoid a possible deadlock if the current address space is destroyed | ||||
* and that address space maps the locked vnode. In the common case, | * and that address space maps the locked vnode. In the common case, | ||||
* the locked vnode's v_usecount is decremented but remains greater | * the locked vnode's v_usecount is decremented but remains greater | ||||
* than zero. Consequently, the vnode lock is not needed by vrele(). | * than zero. Consequently, the vnode lock is not needed by vrele(). | ||||
* However, in cases where the vnode lock is external, such as nullfs, | * However, in cases where the vnode lock is external, such as nullfs, | ||||
* v_usecount may become zero. | * v_usecount may become zero. | ||||
* | * | ||||
* The VV_TEXT flag prevents modifications to the executable while | * The VV_TEXT flag prevents modifications to the executable while | ||||
* the vnode is unlocked. | * the vnode is unlocked. | ||||
*/ | */ | ||||
VOP_UNLOCK(imgp->vp, 0); | VOP_UNLOCK(imgp->vp, 0); | ||||
error = exec_new_vmspace(imgp, sv); | error = exec_new_vmspace(imgp, sv); | ||||
imgp->proc->p_sysent = sv; | imgp->proc->p_sysent = sv; | ||||
et_dyn_addr = 0; | |||||
Not Done Inline ActionsIs the move of this block of code a bug fix? The diffs will be smaller if we move it in -current and then apply the modification. Not a big deal either way... I like to commit subsets of patches that are bug fixes when I'm doing other work to help keep patch-sets manageable. imp: Is the move of this block of code a bug fix? The diffs will be smaller if we move it in… | |||||
Not Done Inline ActionsIt's a code move and a simplification of code. You'll see a few lines above the removal of the et_dyn_addr variable initialization. Our code just simplifies what we moved a bit and applies ASLR to it if ASLR is active. lattera-gmail.com: It's a code move and a simplification of code. You'll see a few lines above the removal of the… | |||||
Not Done Inline ActionsThe original version of this code avoided assigning more than once, placing this assignment in two 'else' clauses. I'd rather you maintained the original structure, minimising differences. rwatson: The original version of this code avoided assigning more than once, placing this assignment in… | |||||
Not Done Inline ActionsThough this adds to the difference, it simplifies the logic. Previously, you had et_dyn_addr being set in multiple conditionals. This now only has a single conditional for modifying it. I'd rather keep it this way for simplicity sake. lattera-gmail.com: Though this adds to the difference, it simplifies the logic. Previously, you had et_dyn_addr… | |||||
if (hdr->e_type == ET_DYN) { | |||||
/* | |||||
* Honour the base load address from the dso if it is | |||||
* non-zero for some reason. | |||||
Not Done Inline ActionsIs referring to the binary as a 'dso' really accurate here? I wonder if 'binary' would be more appropriate? Also, 'for some reason' might just be because it needs to be mapped there due to its linkage -- so possibly we should just drop 'for some reason' as there are entirely legitimate reasons reasons, not vague 'some reasons'? rwatson: Is referring to the binary as a 'dso' really accurate here? I wonder if 'binary' would be more… | |||||
Not Done Inline ActionsThis comment was come from kib@: op@opn hardenedBSD.git> git blame freebsd/master -- sys/kern/kern_exec.c [...] edf781a81 (kib 2009-10-10 15:33:01 +0000 801) if ((brand_info->flags & BI_CAN_EXEC_DYN) == 0) edf781a81 (kib 2009-10-10 15:33:01 +0000 802) return (ENOEXEC); 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 803) /* 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 804) * Honour the base load address from the dso if it is 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 805) * non-zero for some reason. 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 806) */ 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 807) if (baddr == 0) 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 808) et_dyn_addr = ET_DYN_LOAD_ADDR; 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 809) else 2eb5677d2 (kib 2009-10-18 12:57:48 +0000 810) et_dyn_addr = 0; [...] op: This comment was come from kib@:
op@opn hardenedBSD.git> git blame freebsd/master… | |||||
*/ | |||||
if (baddr == 0) { | |||||
et_dyn_addr = ET_DYN_LOAD_ADDR; | |||||
#ifdef PAX_ASLR | |||||
if (pax_aslr_active(imgp->proc)) | |||||
et_dyn_addr += imgp->proc->p_vmspace->vm_aslr_delta_exec; | |||||
#endif | |||||
} | |||||
} | |||||
vn_lock(imgp->vp, LK_EXCLUSIVE | LK_RETRY); | vn_lock(imgp->vp, LK_EXCLUSIVE | LK_RETRY); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
for (i = 0; i < hdr->e_phnum; i++) { | for (i = 0; i < hdr->e_phnum; i++) { | ||||
switch (phdr[i].p_type) { | switch (phdr[i].p_type) { | ||||
case PT_LOAD: /* Loadable segment */ | case PT_LOAD: /* Loadable segment */ | ||||
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Lines | #endif | ||||
/* | /* | ||||
* We load the dynamic linker where a userland call | * We load the dynamic linker where a userland call | ||||
* to mmap(0, ...) would put it. The rationale behind this | * to mmap(0, ...) would put it. The rationale behind this | ||||
* calculation is that it leaves room for the heap to grow to | * calculation is that it leaves room for the heap to grow to | ||||
* its maximum allowed size. | * its maximum allowed size. | ||||
*/ | */ | ||||
addr = round_page((vm_offset_t)vmspace->vm_daddr + lim_max(imgp->proc, | addr = round_page((vm_offset_t)vmspace->vm_daddr + lim_max(imgp->proc, | ||||
RLIMIT_DATA)); | RLIMIT_DATA)); | ||||
Not Done Inline ActionsThe next update will issue the missing rtld randomization here. op: The next update will issue the missing rtld randomization here. | |||||
PROC_UNLOCK(imgp->proc); | PROC_UNLOCK(imgp->proc); | ||||
imgp->entry_addr = entry; | imgp->entry_addr = entry; | ||||
if (interp != NULL) { | if (interp != NULL) { | ||||
int have_interp = FALSE; | int have_interp = FALSE; | ||||
VOP_UNLOCK(imgp->vp, 0); | VOP_UNLOCK(imgp->vp, 0); | ||||
if (brand_info->emul_path != NULL && | if (brand_info->emul_path != NULL && | ||||
▲ Show 20 Lines • Show All 1,214 Lines • Show Last 20 Lines |
This change is gratuitous and should be removed since it also doesn't follow style(9).