Changeset View
Standalone View
sys/kern/kern_exec.c
Show All 24 Lines | |||||
*/ | */ | ||||
#include <sys/cdefs.h> | #include <sys/cdefs.h> | ||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include "opt_capsicum.h" | #include "opt_capsicum.h" | ||||
#include "opt_hwpmc_hooks.h" | #include "opt_hwpmc_hooks.h" | ||||
#include "opt_ktrace.h" | #include "opt_ktrace.h" | ||||
#include "opt_pax.h" | |||||
#include "opt_vm.h" | #include "opt_vm.h" | ||||
#include <sys/param.h> | #include <sys/param.h> | ||||
#include <sys/capsicum.h> | #include <sys/capsicum.h> | ||||
#include <sys/systm.h> | #include <sys/systm.h> | ||||
#include <sys/eventhandler.h> | #include <sys/eventhandler.h> | ||||
#include <sys/lock.h> | #include <sys/lock.h> | ||||
#include <sys/mutex.h> | #include <sys/mutex.h> | ||||
#include <sys/sysproto.h> | #include <sys/sysproto.h> | ||||
#include <sys/signalvar.h> | #include <sys/signalvar.h> | ||||
#include <sys/kernel.h> | #include <sys/kernel.h> | ||||
#include <sys/mount.h> | #include <sys/mount.h> | ||||
#include <sys/filedesc.h> | #include <sys/filedesc.h> | ||||
#include <sys/fcntl.h> | #include <sys/fcntl.h> | ||||
#include <sys/acct.h> | #include <sys/acct.h> | ||||
#include <sys/exec.h> | #include <sys/exec.h> | ||||
#include <sys/imgact.h> | #include <sys/imgact.h> | ||||
#include <sys/imgact_elf.h> | #include <sys/imgact_elf.h> | ||||
#include <sys/wait.h> | #include <sys/wait.h> | ||||
#include <sys/malloc.h> | #include <sys/malloc.h> | ||||
#include <sys/pax.h> | |||||
#include <sys/priv.h> | #include <sys/priv.h> | ||||
#include <sys/proc.h> | #include <sys/proc.h> | ||||
#include <sys/pioctl.h> | #include <sys/pioctl.h> | ||||
#include <sys/namei.h> | #include <sys/namei.h> | ||||
#include <sys/resourcevar.h> | #include <sys/resourcevar.h> | ||||
#include <sys/rwlock.h> | #include <sys/rwlock.h> | ||||
#include <sys/sched.h> | #include <sys/sched.h> | ||||
#include <sys/sdt.h> | #include <sys/sdt.h> | ||||
#include <sys/sf_buf.h> | #include <sys/sf_buf.h> | ||||
#include <sys/syscallsubr.h> | #include <sys/syscallsubr.h> | ||||
#include <sys/sysent.h> | #include <sys/sysent.h> | ||||
#include <sys/shm.h> | #include <sys/shm.h> | ||||
#include <sys/sysctl.h> | #include <sys/sysctl.h> | ||||
#include <sys/vnode.h> | #include <sys/vnode.h> | ||||
#include <sys/stat.h> | #include <sys/stat.h> | ||||
#include <sys/jail.h> | |||||
#ifdef KTRACE | #ifdef KTRACE | ||||
#include <sys/ktrace.h> | #include <sys/ktrace.h> | ||||
#endif | #endif | ||||
#include <vm/vm.h> | #include <vm/vm.h> | ||||
#include <vm/vm_param.h> | #include <vm/vm_param.h> | ||||
#include <vm/pmap.h> | #include <vm/pmap.h> | ||||
#include <vm/vm_page.h> | #include <vm/vm_page.h> | ||||
#include <vm/vm_map.h> | #include <vm/vm_map.h> | ||||
#include <vm/vm_kern.h> | #include <vm/vm_kern.h> | ||||
#include <vm/vm_extern.h> | #include <vm/vm_extern.h> | ||||
#include <vm/vm_object.h> | #include <vm/vm_object.h> | ||||
#include <vm/vm_pager.h> | #include <vm/vm_pager.h> | ||||
#ifdef HWPMC_HOOKS | #ifdef HWPMC_HOOKS | ||||
#include <sys/pmckern.h> | #include <sys/pmckern.h> | ||||
#endif | #endif | ||||
#include <machine/reg.h> | #include <machine/reg.h> | ||||
#include <security/audit/audit.h> | #include <security/audit/audit.h> | ||||
#include <security/mac/mac_framework.h> | #include <security/mac/mac_framework.h> | ||||
#include <security/mac_bsdextended/mac_bsdextended.h> | |||||
rwatson: MAC-policy headers should never be included outside of their specific policy modules; only… | |||||
Not Done Inline ActionsWhat is CACM 56(2)? lattera-gmail.com: What is CACM 56(2)? | |||||
Not Done Inline ActionsRobert N. M. Watson. A decade of OS access-control extensibility. Communications of the ACM 56(2), February 2013. Robert N. M. Watson. A decade of OS access-control extensibility. ACM Queue 11(1), January 2013. (Open access, extended version of CACM article.) rwatson: Robert N. M. Watson. A decade of OS access-control extensibility. Communications of the ACM 56… | |||||
#ifdef KDTRACE_HOOKS | #ifdef KDTRACE_HOOKS | ||||
#include <sys/dtrace_bsd.h> | #include <sys/dtrace_bsd.h> | ||||
dtrace_execexit_func_t dtrace_fasttrap_exec; | dtrace_execexit_func_t dtrace_fasttrap_exec; | ||||
#endif | #endif | ||||
SDT_PROVIDER_DECLARE(proc); | SDT_PROVIDER_DECLARE(proc); | ||||
SDT_PROBE_DEFINE1(proc, kernel, , exec, "char *"); | SDT_PROBE_DEFINE1(proc, kernel, , exec, "char *"); | ||||
SDT_PROBE_DEFINE1(proc, kernel, , exec__failure, "int"); | SDT_PROBE_DEFINE1(proc, kernel, , exec__failure, "int"); | ||||
Show All 26 Lines | |||||
SYSCTL_INT(_kern, OID_AUTO, disallow_high_osrel, CTLFLAG_RW, | SYSCTL_INT(_kern, OID_AUTO, disallow_high_osrel, CTLFLAG_RW, | ||||
&disallow_high_osrel, 0, | &disallow_high_osrel, 0, | ||||
"Disallow execution of binaries built for higher version of the world"); | "Disallow execution of binaries built for higher version of the world"); | ||||
static int map_at_zero = 0; | static int map_at_zero = 0; | ||||
SYSCTL_INT(_security_bsd, OID_AUTO, map_at_zero, CTLFLAG_RWTUN, &map_at_zero, 0, | SYSCTL_INT(_security_bsd, OID_AUTO, map_at_zero, CTLFLAG_RWTUN, &map_at_zero, 0, | ||||
"Permit processes to map an object at virtual address 0."); | "Permit processes to map an object at virtual address 0."); | ||||
#ifdef PAX_ASLR | |||||
int exec_check_aslr(struct image_params *imgp); | |||||
Not Done Inline ActionsShouldn't this non-static function prototype be in a header? rwatson: Shouldn't this non-static function prototype be in a header? | |||||
Not Done Inline ActionsIt can be made static. lattera-gmail.com: It can be made static. | |||||
Not Done Inline ActionsIf it's not referenced outside of the file and isn't 'public', then it should be static. rwatson: If it's not referenced outside of the file and isn't 'public', then it should be static. | |||||
#endif | |||||
static int | static int | ||||
sysctl_kern_ps_strings(SYSCTL_HANDLER_ARGS) | sysctl_kern_ps_strings(SYSCTL_HANDLER_ARGS) | ||||
{ | { | ||||
struct proc *p; | struct proc *p; | ||||
int error; | int error; | ||||
p = curproc; | p = curproc; | ||||
#ifdef SCTL_MASK32 | #ifdef SCTL_MASK32 | ||||
▲ Show 20 Lines • Show All 255 Lines • ▼ Show 20 Lines | #endif | ||||
imgp->auxarg_size = 0; | imgp->auxarg_size = 0; | ||||
imgp->args = args; | imgp->args = args; | ||||
imgp->execpath = imgp->freepath = NULL; | imgp->execpath = imgp->freepath = NULL; | ||||
imgp->execpathp = 0; | imgp->execpathp = 0; | ||||
imgp->canary = 0; | imgp->canary = 0; | ||||
imgp->canarylen = 0; | imgp->canarylen = 0; | ||||
imgp->pagesizes = 0; | imgp->pagesizes = 0; | ||||
imgp->pagesizeslen = 0; | imgp->pagesizeslen = 0; | ||||
imgp->stack_prot = 0; | imgp->stack_prot = 0; | ||||
Not Done Inline ActionsHow does PAX_MPROTECT get defined without PAX_ASLR? And if so, how does the prototype from sys/pax.h for pax_elf get defined in that case? imp: How does PAX_MPROTECT get defined without PAX_ASLR? And if so, how does the prototype from… | |||||
Not Done Inline ActionsI should have left the PAX_MPROTECT stuff out. We haven't actually started on that feature, but will be worked on in the future. lattera-gmail.com: I should have left the PAX_MPROTECT stuff out. We haven't actually started on that feature, but… | |||||
imgp->pax_flags = 0; | |||||
#ifdef MAC | #ifdef MAC | ||||
error = mac_execve_enter(imgp, mac_p); | error = mac_execve_enter(imgp, mac_p); | ||||
if (error) | if (error) | ||||
goto exec_fail; | goto exec_fail; | ||||
#endif | #endif | ||||
imgp->image_header = NULL; | imgp->image_header = NULL; | ||||
▲ Show 20 Lines • Show All 648 Lines • ▼ Show 20 Lines | exec_new_vmspace(imgp, sv) | ||||
} else { | } else { | ||||
error = vmspace_exec(p, sv_minuser, sv->sv_maxuser); | error = vmspace_exec(p, sv_minuser, sv->sv_maxuser); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
vmspace = p->p_vmspace; | vmspace = p->p_vmspace; | ||||
map = &vmspace->vm_map; | map = &vmspace->vm_map; | ||||
} | } | ||||
#ifdef PAX_ASLR | |||||
pax_aslr_init(imgp); | |||||
#endif | |||||
/* Map a shared page */ | /* Map a shared page */ | ||||
obj = sv->sv_shared_page_obj; | obj = sv->sv_shared_page_obj; | ||||
if (obj != NULL) { | if (obj != NULL) { | ||||
vm_object_reference(obj); | vm_object_reference(obj); | ||||
error = vm_map_fixed(map, obj, 0, | error = vm_map_fixed(map, obj, 0, | ||||
sv->sv_shared_page_base, sv->sv_shared_page_len, | sv->sv_shared_page_base, sv->sv_shared_page_len, | ||||
VM_PROT_READ | VM_PROT_EXECUTE, | VM_PROT_READ | VM_PROT_EXECUTE, | ||||
VM_PROT_READ | VM_PROT_EXECUTE, | VM_PROT_READ | VM_PROT_EXECUTE, | ||||
Show All 18 Lines | if (error) | ||||
return (error); | return (error); | ||||
/* | /* | ||||
* vm_ssize and vm_maxsaddr are somewhat antiquated concepts, but they | * vm_ssize and vm_maxsaddr are somewhat antiquated concepts, but they | ||||
* are still used to enforce the stack rlimit on the process stack. | * are still used to enforce the stack rlimit on the process stack. | ||||
*/ | */ | ||||
vmspace->vm_ssize = sgrowsiz >> PAGE_SHIFT; | vmspace->vm_ssize = sgrowsiz >> PAGE_SHIFT; | ||||
vmspace->vm_maxsaddr = (char *)sv->sv_usrstack - ssiz; | vmspace->vm_maxsaddr = (char *)sv->sv_usrstack - ssiz; | ||||
#ifdef PAX_ASLR | |||||
vmspace->vm_maxsaddr -= vmspace->vm_aslr_delta_stack; | |||||
#endif | |||||
Not Done Inline ActionsAs an example, this #ifdef block can be unconditionally replaced by But you do not understand that exec not always reinitialize the vmspace, which causes weird reduction of the available stack size for deep chains of execed images. kib: As an example, this #ifdef block can be unconditionally replaced by
p->p_vmspace->vm_maxsaddr… | |||||
Not Done Inline ActionsThe placed #ifdef blocks are because performance decision. If someone choose to disable the ASLR completely, that this #ifdef blocks saves some CPU cycle. For the reinitialization, could you please create a simple test-case or proof of concept code? The deltas which corresponds to the newly execed image should be reinitialized in every exec calls, independently from reusing the previous vmspace or create a new one. This is an design decision. "In practice brute forcing can be applied to bugs that are in network daemons that fork() on each connection since fork() preserves the randomized layout, as opposed to execve() which replaces it with a new one." - http://pax.grsecurity.net/docs/aslr.txt op: The placed #ifdef blocks are because performance decision. If someone choose to disable the… | |||||
return (0); | return (0); | ||||
} | } | ||||
/* | /* | ||||
* Copy out argument and environment strings from the old process address | * Copy out argument and environment strings from the old process address | ||||
* space into the temporary string buffer. | * space into the temporary string buffer. | ||||
*/ | */ | ||||
int | int | ||||
▲ Show 20 Lines • Show All 142 Lines • ▼ Show 20 Lines | exec_copyout_strings(imgp) | ||||
p = imgp->proc; | p = imgp->proc; | ||||
szsigcode = 0; | szsigcode = 0; | ||||
arginfo = (struct ps_strings *)p->p_sysent->sv_psstrings; | arginfo = (struct ps_strings *)p->p_sysent->sv_psstrings; | ||||
if (p->p_sysent->sv_sigcode_base == 0) { | if (p->p_sysent->sv_sigcode_base == 0) { | ||||
if (p->p_sysent->sv_szsigcode != NULL) | if (p->p_sysent->sv_szsigcode != NULL) | ||||
szsigcode = *(p->p_sysent->sv_szsigcode); | szsigcode = *(p->p_sysent->sv_szsigcode); | ||||
} | } | ||||
destp = (uintptr_t)arginfo; | destp = (uintptr_t)arginfo; | ||||
#ifdef PAX_ASLR | |||||
pax_aslr_stack(curthread, &destp); | |||||
#endif | |||||
/* | /* | ||||
* install sigcode | * install sigcode | ||||
*/ | */ | ||||
if (szsigcode != 0) { | if (szsigcode != 0) { | ||||
destp -= szsigcode; | destp -= szsigcode; | ||||
destp = rounddown2(destp, sizeof(void *)); | destp = rounddown2(destp, sizeof(void *)); | ||||
copyout(p->p_sysent->sv_sigcode, (void *)destp, szsigcode); | copyout(p->p_sysent->sv_sigcode, (void *)destp, szsigcode); | ||||
▲ Show 20 Lines • Show All 104 Lines • ▼ Show 20 Lines | #endif | ||||
} | } | ||||
/* end of vector table is a null pointer */ | /* end of vector table is a null pointer */ | ||||
suword(vectp, 0); | suword(vectp, 0); | ||||
return (stack_base); | return (stack_base); | ||||
} | } | ||||
#ifdef PAX_ASLR | |||||
/* | /* | ||||
* If we've disabled ASLR via ptrace, do not allow execution of | |||||
* setuid/setgid binaries. | |||||
*/ | |||||
int | |||||
exec_check_aslr(struct image_params *imgp) | |||||
{ | |||||
struct proc *p = imgp->proc; | |||||
struct ucred *oldcred = p->p_ucred; | |||||
struct vattr *attr = imgp->attr; | |||||
struct prison *pr; | |||||
int error, credential_changing; | |||||
credential_changing = 0; | |||||
credential_changing |= (attr->va_mode & S_ISUID) && oldcred->cr_uid != | |||||
attr->va_uid; | |||||
credential_changing |= (attr->va_mode & S_ISGID) && oldcred->cr_gid != | |||||
Not Done Inline ActionsI have pointed out in prior reviews that you have incorrectly replicated credential-changing logic from elsewhere. It is still wrong. Please centralise the logic so that it doesn't appear in multiple places, as I requested before, and make sure you use the right version. rwatson: I have pointed out in prior reviews that you have incorrectly replicated credential-changing… | |||||
Not Done Inline ActionsThe next patch will include MAC integration into this logic. However, the spot where I got the credential changing logic from needs additional details. There's no way to remove code duplication. If you'd like to see a different approach than the one I take with the next patch, please provide instruction on how to do so and I'd be happy to make the change. lattera-gmail.com: The next patch will include MAC integration into this logic. However, the spot where I got the… | |||||
attr->va_gid; | |||||
Not Done Inline ActionsMAC is also a source of exec-time credential change. If you need to reuse the credential-changing decision, it needs to be passed safely from the authoritative context to this one -- e.g., via image_params. Otherwise you may miss instances of credential change and/or suffer race conditions. rwatson: MAC is also a source of exec-time credential change. If you need to reuse the credential… | |||||
Not Done Inline ActionsThis credential changing logic was copied literally from elsewhere in this file. Should that code be modified to use MAC as part of this ASLR change? lattera-gmail.com: This credential changing logic was copied literally from elsewhere in this file. Should that… | |||||
Not Done Inline ActionsCan you instead eliminate the copying by making the choice once and then caching the result? Making the decision twice opens the opportunity for race conditions, locking problems, errors in updating the code in the future, etc. E.g.: Mac OS X contains a fix to differentiate "credential change" from "can't debug" for MAC, which we should likely pick up so that informational labelling schemes don't break debugging. When we make that change, it would be nice to have to make it in only one place. rwatson: Can you instead eliminate the copying by making the choice once and then caching the result? | |||||
Not Done Inline ActionsYou continue to ignore my request that this logic should *not* be copied and pasted, but instead be centralised -- ideally with the decision made once, atomically, and cached, rather than in multiple places, non-atomically. This is for both functionality and code maintenance reasons. rwatson: You continue to ignore my request that this logic should *not* be copied and pasted, but… | |||||
if (credential_changing) { | |||||
if ((p->p_pax & PAX_NOTE_NOASLR) == PAX_NOTE_NOASLR) { | |||||
pr = pax_get_prison(p); | |||||
if ((pr && pr->pr_pax_aslr_status > 1) || pax_aslr_status > 1) | |||||
Not Done Inline ActionsAvoid using pointers as booleans; instead, compare explicitly with NULL. rwatson: Avoid using pointers as booleans; instead, compare explicitly with NULL. | |||||
Not Done Inline ActionsWill address with the next patch. lattera-gmail.com: Will address with the next patch. | |||||
return (EPERM); | |||||
} | |||||
} | |||||
Not Done Inline ActionsShouldn't the mac code be making this choice? imp: Shouldn't the mac code be making this choice? | |||||
Not Done Inline ActionsNo. MAC is not involved at all here. We had previously implemented mac_bsdextended/ugidfw integration and used mac_bsdextended's main header as the integration point for per-binary flags. However, the mac_bsdxtended integration has been left out due to concerns other FreBSD developers had that were indirectly brought to our attention. lattera-gmail.com: No. MAC is not involved at all here. We had previously implemented mac_bsdextended/ugidfw… | |||||
Not Done Inline ActionsThis local variable 'error' is unnecessary -- just return (pax_elf(...)). rwatson: This local variable 'error' is unnecessary -- just return (pax_elf(...)). | |||||
error = pax_elf(imgp, | |||||
p->p_pax & PAX_NOTE_NOASLR ? MBI_ASLR_DISABLED : 0); | |||||
return (error); | |||||
} | |||||
#endif /* PAX_ASLR */ | |||||
/* | |||||
* Check permissions of file to execute. | * Check permissions of file to execute. | ||||
* Called with imgp->vp locked. | * Called with imgp->vp locked. | ||||
* Return 0 for success or error code on failure. | * Return 0 for success or error code on failure. | ||||
*/ | */ | ||||
int | int | ||||
exec_check_permissions(imgp) | exec_check_permissions(imgp) | ||||
struct image_params *imgp; | struct image_params *imgp; | ||||
{ | { | ||||
struct vnode *vp = imgp->vp; | struct vnode *vp = imgp->vp; | ||||
struct vattr *attr = imgp->attr; | struct vattr *attr = imgp->attr; | ||||
struct thread *td; | struct thread *td; | ||||
int error, writecount; | int error, writecount; | ||||
td = curthread; | td = curthread; | ||||
/* Get file attributes */ | /* Get file attributes */ | ||||
error = VOP_GETATTR(vp, attr, td->td_ucred); | error = VOP_GETATTR(vp, attr, td->td_ucred); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
#if defined(PAX_ASLR) | |||||
error = exec_check_aslr(imgp); | |||||
if (error) | |||||
return (error); | |||||
#endif | |||||
#ifdef MAC | #ifdef MAC | ||||
error = mac_vnode_check_exec(td->td_ucred, imgp->vp, imgp); | error = mac_vnode_check_exec(td->td_ucred, imgp->vp, imgp); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
#endif | #endif | ||||
/* | /* | ||||
▲ Show 20 Lines • Show All 105 Lines • Show Last 20 Lines |
MAC-policy headers should never be included outside of their specific policy modules; only framework headers should be visible to the kernel as a whole. This is an explicit and critical KBI design choice: please see the article in CACM 56(2) for why this is the case.