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

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

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