From owner-freebsd-bugs@FreeBSD.ORG Sun Aug 29 17:10:28 2004 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 92F2716A4CE for ; Sun, 29 Aug 2004 17:10:28 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 738E743D2D for ; Sun, 29 Aug 2004 17:10:28 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.11/8.12.11) with ESMTP id i7THASaw099985 for ; Sun, 29 Aug 2004 17:10:28 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.11/8.12.11/Submit) id i7THASOv099984; Sun, 29 Aug 2004 17:10:28 GMT (envelope-from gnats) Resent-Date: Sun, 29 Aug 2004 17:10:28 GMT Resent-Message-Id: <200408291710.i7THASOv099984@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Uwe Doering Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4BAAA16A4CE for ; Sun, 29 Aug 2004 17:03:23 +0000 (GMT) Received: from gen129.n001.c02.escapebox.net (gen129.n001.c02.escapebox.net [213.73.91.129]) by mx1.FreeBSD.org (Postfix) with ESMTP id 973E643D41 for ; Sun, 29 Aug 2004 17:03:22 +0000 (GMT) (envelope-from gemini@geminix.org) Received: from gemini by geminix.org with local (Exim 3.36 #1) id 1C1T5L-0006sc-00; Sun, 29 Aug 2004 19:03:19 +0200 Message-Id: Date: Sun, 29 Aug 2004 19:03:19 +0200 From: Uwe Doering To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: kern/71109: Possible race conditions in pmap.c X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Uwe Doering List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Aug 2004 17:10:28 -0000 >Number: 71109 >Category: kern >Synopsis: Possible race conditions in pmap.c >Confidential: no >Severity: serious >Priority: high >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sun Aug 29 17:10:28 GMT 2004 >Closed-Date: >Last-Modified: >Originator: Uwe Doering >Release: FreeBSD 4.5-RELEASE i386 >Organization: EscapeBox - Managed On-Demand UNIX Servers http://www.escapebox.net >Environment: System: FreeBSD geminix.org 4.5-RELEASE FreeBSD 4.5-RELEASE #3: Sun Aug 29 09:13:05 GMT 2004 root@localhost:/RELENG_4_Enhanced i386 >Description: pmap_allocpte() and pmap_enter_quick() in 'src/sys/i386/i386/pmap.c' both call pmap_page_lookup() and _pmap_allocpte(). The latter two functions may return NULL, which can be an indicator that the respective function has been blocking. The caller is then supposed to start over again since the current operation at this level can no longer be considered atomic. Now, in pmap_allocpte() the return value of pmap_page_lookup() isn't checked for NULL, and in pmap_enter_quick() the same is true for _pmap_allocpte(). These flaws cause race conditions that can result in a kernel panic. >How-To-Repeat: The problem becomes apparent when looking at the relevant source code. >Fix: Please consider adopting the patch below. It applies cleanly to RELENG_4 as of today, and, as far as I can tell, also applies to the Alpha architecture (possibly with minor tweaks). Apart from adding the missing NULL checking we also optimize the other, already existing NULL check in pmap_enter_quick() in that we do it only after pmap_page_lookup() has been called, because 'mpte' cannot be NULL in case the first part of the if-else clause gets executed. This patch also removes a superfluous call of VM_WAIT in _pmap_allocpte(). The preceding function vm_page_grab() already does the appropriate sleeping before returning NULL, even if called without the VM_ALLOC_RETRY flag. So we can return from _pmap_allocpte() right away. --- pmap.c.diff begins here --- --- src/sys/i386/i386/pmap.c.orig Thu May 6 20:56:50 2004 +++ src/sys/i386/i386/pmap.c Mon May 31 13:03:52 2004 @@ -1228,7 +1228,6 @@ m = vm_page_grab(pmap->pm_pteobj, ptepindex, VM_ALLOC_ZERO); if (m == NULL) { - VM_WAIT; /* * Indicate the need to retry. While waiting, the page table * page may have been allocated. @@ -1316,6 +1315,8 @@ } else { m = pmap_page_lookup(pmap->pm_pteobj, ptepindex); pmap->pm_ptphint = m; + if (m == NULL) + goto retry; } m->hold_count++; } else { @@ -2105,12 +2106,14 @@ } else { mpte = pmap_page_lookup(pmap->pm_pteobj, ptepindex); pmap->pm_ptphint = mpte; + if (mpte == NULL) + goto retry; } - if (mpte == NULL) - goto retry; mpte->hold_count++; } else { mpte = _pmap_allocpte(pmap, ptepindex); + if (mpte == NULL) + goto retry; } } } else { --- pmap.c.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted: