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

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR port-xen/52016; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: port-xen-maintainer%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
        netbsd-bugs%netbsd.org@localhost
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@localhost 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@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --