NetBSD Problem Report #52016

From www@NetBSD.org  Tue Feb 28 22:49:02 2017
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 DCE737A16A
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 28 Feb 2017 22:49:02 +0000 (UTC)
Message-Id: <20170228224901.8E54A7A2AB@mollari.NetBSD.org>
Date: Tue, 28 Feb 2017 22:49:01 +0000 (UTC)
From: coypu@sdf.org
Reply-To: coypu@sdf.org
To: gnats-bugs@NetBSD.org
Subject: Mismatch between mem_clusters and pmap_enter bounds check
X-Send-Pr-Version: www-1.0

>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
>Last-Modified:  Wed Mar 01 16:30:01 +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);
 }

>Audit-Trail:
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@netbsd.org
Cc: port-xen-maintainer@netbsd.org, gnats-admin@netbsd.org,
        netbsd-bugs@netbsd.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 09:54:18 +0100

 On Tue, Feb 28, 2017 at 10:50:00PM +0000, coypu@sdf.org wrote:
 > [...]
 > >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);
 >  }

 This is used by X11 to access the video adapter.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 09:59:01 +0100

 On Wed, Mar 01, 2017 at 08:55:01AM +0000, Manuel Bouyer wrote:
 >  This is used by X11 to access the video adapter.

 So how can we find the bounds that the hypervisor allows for that and
 deny access outside of that?

 Martin

From: Stephen Borrill <netbsd@precedence.co.uk>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 09:00:15 +0000 (GMT)

 Successfully tested with both patches:

 1# dmidecode
 # dmidecode 3.0
 Scanning /dev/mem for entry point.
 /dev/mem: Operation not permitted
 2#

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@netbsd.org
Cc: port-xen-maintainer@netbsd.org, gnats-admin@netbsd.org,
        netbsd-bugs@netbsd.org, coypu@sdf.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 10:04:07 +0100

 On Wed, Mar 01, 2017 at 09:00:01AM +0000, Martin Husemann wrote:
 > The following reply was made to PR port-xen/52016; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 >  bounds check
 > Date: Wed, 1 Mar 2017 09:59:01 +0100
 > 
 >  On Wed, Mar 01, 2017 at 08:55:01AM +0000, Manuel Bouyer wrote:
 >  >  This is used by X11 to access the video adapter.
 >  
 >  So how can we find the bounds that the hypervisor allows for that and
 >  deny access outside of that?

 I don't know. Maybe we should change the pmap to gracefully handle failure
 from mmu operations. But that's not trivial.
 On a physical hardware how do you know if a physical address is accessible or
 not ?

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: coypu@SDF.ORG
To: gnats-bugs@NetBSD.org
Cc: port-xen-maintainer@netbsd.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 09:30:58 +0000

 mm_md_physacc is called exclusively by /dev/mem, so it isn't
 too bad to limit it.

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: coypu@SDF.ORG
Cc: gnats-bugs@NetBSD.org, port-xen-maintainer@NetBSD.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 10:34:03 +0100

 On Wed, Mar 01, 2017 at 09:30:58AM +0000, coypu@SDF.ORG wrote:
 > mm_md_physacc is called exclusively by /dev/mem, so it isn't
 > too bad to limit it.

 AFAIK X11 uses /dev/mem to access the hardware.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: coypu@SDF.ORG
To: gnats-bugs@NetBSD.org
Cc: port-xen-maintainer@netbsd.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 09:55:40 +0000

 On Wed, Mar 01, 2017 at 09:35:01AM +0000, Manuel Bouyer wrote:
 >  AFAIK X11 uses /dev/mem to access the hardware.
 >  

 Huh, so it does.
 I guess X doesn't currently fail because it's already mapped
 by the boot process, so no fault and no trip to pmap_enter?

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: coypu@SDF.ORG
Cc: gnats-bugs@NetBSD.org, port-xen-maintainer@netbsd.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 12:13:10 +0100

 On Wed, Mar 01, 2017 at 09:55:40AM +0000, coypu@SDF.ORG wrote:
 > On Wed, Mar 01, 2017 at 09:35:01AM +0000, Manuel Bouyer wrote:
 > >  AFAIK X11 uses /dev/mem to access the hardware.
 > >  
 > 
 > Huh, so it does.
 > I guess X doesn't currently fail because it's already mapped
 > by the boot process, so no fault and no trip to pmap_enter?

 No, it works because the hypervisor allows mapping the video
 harware address. I don't know why it rejects the addresses used by dmidecode

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 12:18:18 +0100

 On Wed, Mar 01, 2017 at 11:15:01AM +0000, Manuel Bouyer wrote:
 >  No, it works because the hypervisor allows mapping the video
 >  harware address. I don't know why it rejects the addresses used by dmidecode

 There should be a fixed list of "forbidden" addresses, probably something
 like 0xc0000 - 0xfffff. We need to find out the details and mirror the
 behvaiour in the access check.

 Unless there is a way to query the hypervisor for this details...

 Martin

From: coypu@SDF.ORG
To: Manuel Bouyer <bouyer@antioche.eu.org>
Cc: gnats-bugs@NetBSD.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 12:03:10 +0000

 From a really far from sufficiently thorough look at linux, it looks
 like it might be equivalent to putting stuff that is directly accessed
 within [pmap_pa_start,pmap_pa_end) and having magic to map it to
 "identity".

 And we can decide for domU that some stuff points to a dummy page and
 so HYPERVISOR_mmu_update doesn't fail for it.

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: port-xen-maintainer@netbsd.org, gnats-admin@netbsd.org,
        netbsd-bugs@netbsd.org, coypu@sdf.org
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 bounds check
Date: Wed, 1 Mar 2017 13:12:23 +0100

 On Wed, Mar 01, 2017 at 12:05:00PM +0000, coypu@SDF.ORG wrote:
 > The following reply was made to PR port-xen/52016; it has been noted by GNATS.
 > 
 > From: coypu@SDF.ORG
 > To: Manuel Bouyer <bouyer@antioche.eu.org>
 > Cc: gnats-bugs@NetBSD.org
 > Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter
 >  bounds check
 > Date: Wed, 1 Mar 2017 12:03:10 +0000
 > 
 >  From a really far from sufficiently thorough look at linux, it looks
 >  like it might be equivalent to putting stuff that is directly accessed
 >  within [pmap_pa_start,pmap_pa_end) and having magic to map it to
 >  "identity".

 But then you may have holes in between pmap_pa_start and pmap_pa_end.
 I think we should keep pmap_pa* for pseudo-physical addresses only,
 and add something else for machine addresses - if we have a way to
 know mappable machine addresses.
 One problem is that you may want to map machine addresses which falls into
 the pseudo-physical address range via /dev/mem. In this case you don't have
 a way to know which kind of address this is.

 Using /dev/mem for both kind of addresses is bogus.

 >  
 >  And we can decide for domU that some stuff points to a dummy page and
 >  so HYPERVISOR_mmu_update doesn't fail for it.

 a domU doesn't need to map machine addresses via /dev/mem (and the hypervisor
 won't allow it).

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: John Nemeth <jnemeth@cue.bc.ca>
To: gnats-bugs@NetBSD.org, port-xen-maintainer@netbsd.org,
        gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, coypu@sdf.org
Cc: 
Subject: Re: port-xen/52016: Mismatch between mem_clusters and pmap_enter bounds check
Date: Wed, 1 Mar 2017 08:44:12 -0800

 On Mar 1, 11:15am, Manuel Bouyer wrote:
 }
 } The following reply was made to PR port-xen/52016; it has been noted by GNATS.
 } 
 } From: Manuel Bouyer <bouyer@antioche.eu.org>
 } To: coypu@SDF.ORG
 } Cc: gnats-bugs@NetBSD.org, port-xen-maintainer@netbsd.org
 } Date: Wed, 1 Mar 2017 12:13:10 +0100
 } 
 }  On Wed, Mar 01, 2017 at 09:55:40AM +0000, coypu@SDF.ORG wrote:
 }  > On Wed, Mar 01, 2017 at 09:35:01AM +0000, Manuel Bouyer wrote:
 }  > >  AFAIK X11 uses /dev/mem to access the hardware.
 }  > >  
 }  > 
 }  > Huh, so it does.
 }  > I guess X doesn't currently fail because it's already mapped
 }  > by the boot process, so no fault and no trip to pmap_enter?
 }  
 }  No, it works because the hypervisor allows mapping the video
 }  harware address. I don't know why it rejects the addresses used by dmidecode

      I haven't done a deep dive, but based on comments here and
 elsewhere, it is likely that dmidecode is trying to access the
 "BIOS ROM" and in PV mode there is no BIOS.  We need a way for
 mappings done via /dev/mem to fail gracefully (i.e. not panic the
 system).

 }-- End of excerpt from Manuel Bouyer

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.