From owner-freebsd-current@FreeBSD.ORG Sat Mar 21 01:00:46 2015 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C471B36D; Sat, 21 Mar 2015 01:00:46 +0000 (UTC) Received: from mail-wi0-x235.google.com (mail-wi0-x235.google.com [IPv6:2a00:1450:400c:c05::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5691A8D5; Sat, 21 Mar 2015 01:00:46 +0000 (UTC) Received: by wibdy8 with SMTP id dy8so1720128wib.0; Fri, 20 Mar 2015 18:00:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=c6UW0jqQpJmbe6VXmLgDJ0OZiJtr7ft3TXY9n8XVkWk=; b=zyK7orKS6y62BZV02mkoNW05Q6fkVSqtk8X7PsAlmNjxbP5S/Y6DcD1PYWQELdockp RYL19jMK54y4UW3jZJsy5RlUHWHKwUP7i9nDa6cDRafdbvJFKcRQ4GcVbsyFtbKVvAUE qePu5ngM0KRjyEREv2D9sPwlw0q6p2bhzf8Eqm1bybhrXotv23efhcDadlRObQA0VH56 Too4XuxKMHIS1TWPyuUw15H+xKVOEjL++G7HJxz82G5+HVDn1bNk3o7PIyULxzL1cXkc Q9vHbNlrVOPAvYvbccBs+oCVgdheFPOvd++7bCK5oOv5SaFE7c69x4CvHAyHORS5dGuQ 6BKg== X-Received: by 10.180.24.162 with SMTP id v2mr659912wif.80.1426899644727; Fri, 20 Mar 2015 18:00:44 -0700 (PDT) Received: from localhost.localdomain (ip-89-176-114-84.net.upcbroadband.cz. [89.176.114.84]) by mx.google.com with ESMTPSA id q6sm303207wix.3.2015.03.20.18.00.43 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Mar 2015 18:00:43 -0700 (PDT) From: Mateusz Guzik To: Konstantin Belousov Subject: [PATCH 1/3] fork: assign refed credentials earlier Date: Sat, 21 Mar 2015 02:00:38 +0100 Message-Id: <1426899640-6599-2-git-send-email-mjguzik@gmail.com> X-Mailer: git-send-email 1.8.3.1 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> Cc: freebsd-current@freebsd.org, jenkins-admin@freebsd.org, Mateusz Guzik X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Mar 2015 01:00:47 -0000 From: Mateusz Guzik 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