port-sh3/51555: Unsafe memory allocation in pmap.c

[coypu%sdf.org@localhost]

>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
>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)
 {