NetBSD Problem Report #51555

From www@NetBSD.org  Wed Oct 12 17:35:20 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 83A2B7A2AE
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 12 Oct 2016 17:35:20 +0000 (UTC)
Message-Id: <20161012173519.10B4E7A2CF@mollari.NetBSD.org>
Date: Wed, 12 Oct 2016 17:35:19 +0000 (UTC)
From: coypu@sdf.org
Reply-To: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Subject: Unsafe memory allocation in pmap.c
X-Send-Pr-Version: www-1.0

>Number:         51555
>Category:       port-sh3
>Synopsis:       Unsafe memory allocation in pmap.c
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-sh3-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Oct 12 17:40:00 +0000 2016
>Closed-Date:    
>Last-Modified:  Sun Feb 26 18:32:25 +0000 2017
>Originator:     coypu
>Release:        NetBSD 7.99.39
>Organization:
>Environment:
No sh3 hardware here, sorry.
>Description:
In sys/arch/sh3/sh3/pmap.c:503.., we call
    503 	pv = __pmap_pv_alloc();
and then immediately use it:
    504 	pv->pv_pmap = pmap;
    505 	pv->pv_va = va;

note:
     77 #define	__pmap_pv_alloc()	pool_get(&__pmap_pv_pool, PR_NOWAIT)

This call may return NULL, so we will fault on use.
>How-To-Repeat:

>Fix:
suggested by mrg-
this function seems to have no failure option, so maybe pool_get before
disabling some interrupts, so we can use PR_WAITOK.

patch for suggestion (untested)

Index: pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sh3/sh3/pmap.c,v
retrieving revision 1.78
diff -U 10 -r1.78 pmap.c
--- pmap.c	3 Sep 2016 09:07:54 -0000	1.78
+++ pmap.c	12 Oct 2016 16:34:10 -0000
@@ -465,49 +465,50 @@

 /*
  * void __pmap_pv_enter(pmap_t pmap, struct vm_page *pg, vaddr_t vaddr):
  *	Insert physical-virtual map to vm_page.
  *	Assume pre-existed mapping is already removed.
  */
 void
 __pmap_pv_enter(pmap_t pmap, struct vm_page *pg, vaddr_t va)
 {
 	struct vm_page_md *pvh;
-	struct pv_entry *pv;
+	struct pv_entry *opv, *pv;
 	int s;

+	pv = pool_get(&__pmap_pv_pool, PR_WAITOK);
+
 	s = splvm();
 	if (SH_HAS_VIRTUAL_ALIAS) {
 		/*
 		 * Remove all other mappings on this physical page
 		 * which have different virtual cache indexes to
 		 * avoid virtual cache aliases.
 		 *
 		 * XXX We should also handle shared mappings which
 		 * XXX have different virtual cache indexes by
 		 * XXX mapping them uncached (like arm and mips do).
 		 */
  again:
 		pvh = VM_PAGE_TO_MD(pg);
-		SLIST_FOREACH(pv, &pvh->pvh_head, pv_link) {
+		SLIST_FOREACH(opv, &pvh->pvh_head, pv_link) {
 			if (sh_cache_indexof(va) !=
-			    sh_cache_indexof(pv->pv_va)) {
-				pmap_remove(pv->pv_pmap, pv->pv_va,
-				    pv->pv_va + PAGE_SIZE);
+			    sh_cache_indexof(opv->pv_va)) {
+				pmap_remove(opv->pv_pmap, opv->pv_va,
+				    opv->pv_va + PAGE_SIZE);
 				goto again;
 			}
 		}
 	}

 	/* Register pv map */
 	pvh = VM_PAGE_TO_MD(pg);
-	pv = __pmap_pv_alloc();
 	pv->pv_pmap = pmap;
 	pv->pv_va = va;

 	SLIST_INSERT_HEAD(&pvh->pvh_head, pv, pv_link);
 	splx(s);
 }

 void
 pmap_remove(pmap_t pmap, vaddr_t sva, vaddr_t eva)
 {

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Sun, 26 Feb 2017 16:07:12 +0000
State-Changed-Why:


State-Changed-From-To: closed->open
State-Changed-By: uwe@NetBSD.org
State-Changed-When: Sun, 26 Feb 2017 18:32:25 +0000
State-Changed-Why:
Put the toys backs into the pram.


>Unformatted:

Home	PR Database Search