[PATCH 1/3] fork: assign refed credentials earlier

From: Mateusz Guzik <mjguzik_at_gmail.com>
Date: Sat, 21 Mar 2015 02:00:38 +0100
From: Mateusz Guzik <mjg_at_freebsd.org>

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.
---
 sys/kern/kern_fork.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index ae86fe1..15833fd 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
_at__at_ -410,9 +410,6 _at__at_ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
 	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);
 
_at__at_ -832,7 +829,7 _at__at_ fork1(struct thread *td, int flags, int pages, struct proc **procp,
 		td2 = thread_alloc(pages);
 		if (td2 == NULL) {
 			error = ENOMEM;
-			goto fail1;
+			goto fail2;
 		}
 		proc_linkup(newproc, td2);
 	} else {
_at__at_ -841,7 +838,7 _at__at_ fork1(struct thread *td, int flags, int pages, struct proc **procp,
 				vm_thread_dispose(td2);
 			if (!thread_alloc_stack(td2, pages)) {
 				error = ENOMEM;
-				goto fail1;
+				goto fail2;
 			}
 		}
 	}
_at__at_ -850,7 +847,7 _at__at_ fork1(struct thread *td, int flags, int pages, struct proc **procp,
 		vm2 = vmspace_fork(p1->p_vmspace, &mem_charged);
 		if (vm2 == NULL) {
 			error = ENOMEM;
-			goto fail1;
+			goto fail2;
 		}
 		if (!swap_reserve(mem_charged)) {
 			/*
_at__at_ -861,7 +858,7 _at__at_ fork1(struct thread *td, int flags, int pages, struct proc **procp,
 			 */
 			swap_reserve_force(mem_charged);
 			error = ENOMEM;
-			goto fail1;
+			goto fail2;
 		}
 	} else
 		vm2 = NULL;
_at__at_ -870,7 +867,7 _at__at_ fork1(struct thread *td, int flags, int pages, struct proc **procp,
 	 * 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.
_at__at_ -946,6 +943,8 _at__at_ 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);
-- 
2.3.2
Received on Sat Mar 21 2015 - 00:00:46 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:56 UTC