Date: Sat, 21 Mar 2015 02:00:38 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-current@freebsd.org, jenkins-admin@freebsd.org, Mateusz Guzik <mjg@freebsd.org> Subject: [PATCH 1/3] fork: assign refed credentials earlier Message-ID: <1426899640-6599-2-git-send-email-mjguzik@gmail.com> In-Reply-To: <1426899640-6599-1-git-send-email-mjguzik@gmail.com> References: <20150320122125.GP2379@kib.kiev.ua> <1426899640-6599-1-git-send-email-mjguzik@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
From: Mateusz Guzik <mjg@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
@@ -410,9 +410,6 @@ 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);
@@ -832,7 +829,7 @@ 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 {
@@ -841,7 +838,7 @@ 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;
}
}
}
@@ -850,7 +847,7 @@ 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)) {
/*
@@ -861,7 +858,7 @@ 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;
@@ -870,7 +867,7 @@ 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.
@@ -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);
--
2.3.2
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1426899640-6599-2-git-send-email-mjguzik>