From: Mateusz Guzik <m...@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

_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to