port-xen/52016: Mismatch between mem_clusters and pmap_enter bounds check

[coypu%sdf.org@localhost]

>Number:         52016
>Category:       port-xen
>Synopsis:       Mismatch between mem_clusters and pmap_enter bounds check
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-xen-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Feb 28 22:50:00 +0000 2017
>Originator:     coypu
>Release:        All NetBSD releases so far (7.99.63)
>Organization:
>Environment:
Not running Xen.
>Description:
As reported by sborrill on port-xen, running dmidecode (sysutils/dmidecode) on Xen domU or dom0 will panic, with the following backtrace:

panic: HYPERVISOR_mmu_update failed, ret: -1

fatal breakpoint trap in supervisor mode

trap type 1 code 0 rip ffffffff80134ead cs e030 rflags 246 cr2 7f7ff7ff6ff8 ilevel 8 rsp ffffa0005e37ba28

curlwp 0xffffa0000dda2960 pid 28894.1 lowest kstack 0xffffa0005e3792c0
Stopped in pid 28894.1 (dmidecode) at   netbsd:breakpoint+0x5:  leave
breakpoint() at netbsd:breakpoint+0x5
vpanic() at netbsd:vpanic+0x13c
snprintf() at netbsd:snprintf
xpq_queue_machphys_update() at netbsd:xpq_queue_machphys_update
pmap_enter_ma() at netbsd:pmap_enter_ma+0x440
pmap_enter() at netbsd:pmap_enter+0x35
udv_fault() at netbsd:udv_fault+0x146
uvm_fault_internal() at netbsd:uvm_fault_internal+0x50b
trap() at netbsd:trap+0x322


dmidecode reads /dev/mem

/dev/mem performs MD validity check mm_md_physacc
mm_md_physacc checks if it's part of a mem_cluster. The mem_clusters seem the same for Xen and other x86, so it sees e.g. BIOS values as valid.

a fault gets to pmap_enter. pmap_enter can't handle values outside of [pmap_pa_start, pmap_pa_end).
pmap_enter exacerbates the problem by using a bogus value.
>How-To-Repeat:

>Fix:
This is untested and somewhat of a hack.
Maybe mem_clusters and pmap_enter should be made consistent.

We can't just make pmap_enter fail with an error if it's out of bounds, because udv_fault will restart, then the process will spin making the same repeated syscall and failing. so fail with a panic call.

Make Xen mm_md_physacc check for the same as pmap_enter does.

Index: arch/x86/x86/x86_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/x86_machdep.c,v
retrieving revision 1.89
diff -u -p -u -r1.89 x86_machdep.c
--- arch/x86/x86/x86_machdep.c	14 Feb 2017 13:29:09 -0000	1.89
+++ arch/x86/x86/x86_machdep.c	28 Feb 2017 22:14:50 -0000
@@ -175,6 +175,16 @@ mm_md_physacc(paddr_t pa, vm_prot_t prot
 	extern phys_ram_seg_t mem_clusters[VM_PHYSSEG_MAX];
 	extern int mem_cluster_cnt;
 	int i;
+#ifdef XEN
+	/*
+	 * xen pmap_enter can't handle values outside this range
+	 * but in mem_clusters, reading BIOS (dmidecode) will panic
+	 */
+	extern paddr_t pmap_pa_start, pmap_pa_end;
+
+	if (pa < pmap_pa_start || pmap_pa_end <= pa)
+		return EPERM;
+#endif
 
 	for (i = 0; i < mem_cluster_cnt; i++) {
 		const phys_ram_seg_t *seg = &mem_clusters[i];



And we could probably retire the hack in pmap_enter.

Index: arch/xen/x86/xen_pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/x86/xen_pmap.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 xen_pmap.c
--- arch/xen/x86/xen_pmap.c	26 Dec 2016 08:53:11 -0000	1.25
+++ arch/xen/x86/xen_pmap.c	28 Feb 2017 22:14:50 -0000
@@ -149,11 +149,10 @@ pmap_enter(struct pmap *pmap, vaddr_t va
 {
         paddr_t ma;
 
-	if (__predict_false(pa < pmap_pa_start || pmap_pa_end <= pa)) {
-		ma = pa; /* XXX hack */
-	} else {
-		ma = xpmap_ptom(pa);
-	}
+	if (__predict_false(pa < pmap_pa_start || pmap_pa_end <= pa))
+		panic("%s: Invalid memory address", __func__);
+
+	ma = xpmap_ptom(pa);
 
 	return pmap_enter_ma(pmap, va, ma, pa, prot, flags, DOMID_SELF);
 }