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

[co...@sdf.org]

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