DragonFlyBSD Kernel Audit
DF-2952 / fix.diff
← back to finding ↓ download raw
fix.diff for DF-2952 — resident-exec stale-stack disclosure
============================================================

Authored AFTER guest verification (leak reproduced 5/5 iterations, ~7600
marker hits per exec). Never applied to the read-only sys/ tree; applied
only to the guest's /usr/src copy for fix validation.

Root cause (two halves):
 1. sys/kern/kern_exec.c exec_new_vmspace(): the resident branch forks the
    registration snapshot (whose vm_map already contains the registering
    process's COW stack entry covering [USRSTACK-maxssiz, USRSTACK)), then
    calls vm_map_stack() for the same range.  On pc64 USRSTACK ==
    VM_MAX_USER_ADDRESS, so vm_map_findspace() cannot fit maxssiz below the
    existing entry and vm_map_stack() returns KERN_NO_SPACE on *every*
    resident exec.  The "remap the stack" step has never actually happened.
 2. sys/kern/imgact_resident.c exec_resident_imgact(): discards
    exec_new_vmspace()'s return value, so the exec silently continues on the
    snapshot's stale COW stack — exposing the registration-time (privileged)
    process's stack contents (environment strings, rtld/libc frames) to any
    unprivileged user who execs the registered binary.

Fix: destroy the snapshot's stack mappings in the forked map before
creating the fresh stack (restores the intended "remap the stack"
semantics), and propagate exec_new_vmspace() failures instead of
swallowing them (fail-safe: kern_exec exits the process).

diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
--- a/sys/kern/kern_exec.c
+++ b/sys/kern/kern_exec.c
@@ -949,11 +949,22 @@ exec_new_vmspace(struct image_params *imgp, struct vmspace *vmcopy)
 	 * Blow away entire process VM, if address space not shared,
 	 * otherwise, create a new VM space so that other threads are
 	 * not disrupted.  If we are execing a resident vmspace we
 	 * create a duplicate of it and remap the stack.
 	 */
 	map = &vmspace->vm_map;
 	if (vmcopy) {
 		vmspace_exec(imgp->proc, vmcopy);
 		vmspace = imgp->proc->p_vmspace;
 		pmap_remove_pages(vmspace_pmap(vmspace), stack_addr, USRSTACK);
 		map = &vmspace->vm_map;
+		/*
+		 * The forked snapshot still contains the registering
+		 * process's COW stack entry occupying the top of the
+		 * user address space.  On platforms where USRSTACK is
+		 * the maximum user address (pc64), vm_map_stack() below
+		 * cannot fit maxssiz and fails with KERN_NO_SPACE,
+		 * silently leaving the registration-time stack contents
+		 * (privileged process data) mapped into whoever execs
+		 * the resident image (DF-2952).  Remove the snapshot's
+		 * stack mappings so the fresh stack is actually created.
+		 */
+		vm_map_remove(map, stack_addr, USRSTACK);
 	} else if (vmspace_getrefs(vmspace) == 1) {
 		shmexit(vmspace);
 		pmap_remove_pages(vmspace_pmap(vmspace),
 				  0, VM_MAX_USER_ADDRESS);
diff --git a/sys/kern/imgact_resident.c b/sys/kern/imgact_resident.c
--- a/sys/kern/imgact_resident.c
+++ b/sys/kern/imgact_resident.c
@@ -165,17 +165,21 @@ SYSCTL_PROC(_vm, OID_AUTO, resident, CTLTYPE_OPAQUE|CTLFLAG_RD, 0, 0,
 int
 exec_resident_imgact(struct image_params *imgp)
 {
 	struct vmresident *vmres;
+	int error;
 
 	/*
 	 * resident image activator
 	 */
 	lockmgr(&exec_list_lock, LK_SHARED);
 	if ((vmres = imgp->vp->v_resident) == NULL) {
 	    lockmgr(&exec_list_lock, LK_RELEASE);
 	    return(-1);
 	}
 	atomic_add_int(&vmres->vr_refs, 1);
 	lockmgr(&exec_list_lock, LK_RELEASE);
 
 	/*
 	 * We want to exec the new vmspace without holding the lock to
 	 * improve concurrency.
 	 */
-	exec_new_vmspace(imgp, vmres->vr_vmspace);
-	imgp->resident = 1;
-	imgp->interpreted = 0;
-	imgp->proc->p_sysent = vmres->vr_sysent;
-	imgp->entry_addr = vmres->vr_entry_addr;
+	error = exec_new_vmspace(imgp, vmres->vr_vmspace);
+	if (error == 0) {
+		imgp->resident = 1;
+		imgp->interpreted = 0;
+		imgp->proc->p_sysent = vmres->vr_sysent;
+		imgp->entry_addr = vmres->vr_entry_addr;
+	} else {
+		/* exec_new_vmspace() is the point of no return */
+		error = ENOMEM;
+	}
 	atomic_subtract_int(&vmres->vr_refs, 1);
 
-	return(0);
+	return(error);
 }