svn commit: r280330 - head/sys/kern

[Mateusz Guzik]

Author: mjg
Date: Sat Mar 21 20:24:03 2015
New Revision: 280330
URL: https://svnweb.freebsd.org/changeset/base/280330

Log:
  fork: assign refed credentials earlier
  
  Prior to this change the kernel would take p1's credentials and assign
  them tempororarily to p2. But p1 could change credentials at that time
  and in effect give us a use-after-free.
  
  No objections from: kib

Modified:
  head/sys/kern/kern_fork.c

Modified: head/sys/kern/kern_fork.c
==============================================================================
--- head/sys/kern/kern_fork.c	Sat Mar 21 19:20:15 2015	(r280329)
+++ head/sys/kern/kern_fork.c	Sat Mar 21 20:24:03 2015	(r280330)
@@ -410,9 +410,6 @@ do_fork(struct thread *td, int flags, st
 	bzero(&p2->p_startzero,
 	    __rangeof(struct proc, p_startzero, p_endzero));
 
-	crhold(td->td_ucred);
-	proc_set_cred(p2, td->td_ucred);
-
 	/* Tell the prison that we exist. */
 	prison_proc_hold(p2->p_ucred->cr_prison);
 
@@ -832,7 +829,7 @@ fork1(struct thread *td, int flags, int 
 		td2 = thread_alloc(pages);
 		if (td2 == NULL) {
 			error = ENOMEM;
-			goto fail1;
+			goto fail2;
 		}
 		proc_linkup(newproc, td2);
 	} else {
@@ -841,7 +838,7 @@ fork1(struct thread *td, int flags, int 
 				vm_thread_dispose(td2);
 			if (!thread_alloc_stack(td2, pages)) {
 				error = ENOMEM;
-				goto fail1;
+				goto fail2;
 			}
 		}
 	}
@@ -850,7 +847,7 @@ fork1(struct thread *td, int flags, int 
 		vm2 = vmspace_fork(p1->p_vmspace, &mem_charged);
 		if (vm2 == NULL) {
 			error = ENOMEM;
-			goto fail1;
+			goto fail2;
 		}
 		if (!swap_reserve(mem_charged)) {
 			/*
@@ -861,7 +858,7 @@ fork1(struct thread *td, int flags, int 
 			 */
 			swap_reserve_force(mem_charged);
 			error = ENOMEM;
-			goto fail1;
+			goto fail2;
 		}
 	} else
 		vm2 = NULL;
@@ -870,7 +867,7 @@ fork1(struct thread *td, int flags, int 
 	 * XXX: This is ugly; when we copy resource usage, we need to bump
 	 *      per-cred resource counters.
 	 */
-	proc_set_cred(newproc, p1->p_ucred);
+	proc_set_cred(newproc, crhold(td->td_ucred));
 
 	/*
 	 * Initialize resource accounting for the child process.
@@ -946,6 +943,8 @@ fail:
 #endif
 	racct_proc_exit(newproc);
 fail1:
+	crfree(proc_set_cred(newproc, NULL));
+fail2:
 	if (vm2 != NULL)
 		vmspace_free(vm2);
 	uma_zfree(proc_zone, newproc);