Subject: Re: inconsistency in MD mmmmap() implementations?
To: None <elad@NetBSD.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 10/25/2006 21:12:24
there is nothing wrong to be inconsistent between ports if they have
different memory layouts.
eg. "atop(off) >= physmem" can make sense if its memory is
contiguously mapped from physical address 0.
(well, maybe the "suser" part should be consistent.  but it isn't
what you are asking, right?)

whether each versions are correct or not is a different question. :-)
at least amd64 version seems wrong.  it should be basically the same as i386.

YAMAMOTO Takashi

> The message quoted below was sent on August 8th, over two months ago,
> left unanswered. I'm going to have to change some of that code for
> kauth(9). Can I adjust the policies as I see fit? does anyone have any
> insightful comments?
> 
> -e.
> 
> Elad Efrat wrote:
> > hi,
> > 
> > i've been looking lately at the different mmmmap() implementations
> > each of our archs has and noticed some inconsistency.
> > 
> > first, each mmmmap() routine can be (usually) found in
> > src/sys/arch/<arch>/<arch>/mem.c:mmmmap()
> > 
> > we'll take a look at differences between several archs: i386, amd64,
> > alpha, vax, hp300, powerpc, and sun3. for all archs, i'll be referring
> > to the access conditions for DEV_MEM.
> > 
> > i386:
> > if (check_pa_acc(off, prot) != 0) {
> > 	return -1;
> > }
> > 
> > (and check_pa_acc() is:)
> > 
> > static int
> > check_pa_acc(paddr_t pa, vm_prot_t prot)
> > {
> > 	extern phys_ram_seg_t mem_clusters[VM_PHYSSEG_MAX];
> > 	extern int mem_cluster_cnt;
> > 	int i;
> > 
> > 	if (securelevel <= 0) {
> > 		return 0;
> > 	}
> > 
> > 	for (i = 0; i < mem_cluster_cnt; i++) {
> > 		const phys_ram_seg_t *seg = &mem_clusters[i];
> > 		paddr_t start = seg->start;
> > 
> > 		if (start <= pa && pa - start <= seg->size) {
> > 			return 0;
> > 		}
> > 	}
> > 
> > 	return EPERM;
> > }
> > 
> > as we can see above, freely accessing physical memory, even outside
> > ram, is only permitted in securelevel 0 and below.
> > 
> > this is very similar to what happens on alpha:
> > 
> > if ((prot & alpha_pa_access(off)) != prot)
> > 	return (-1);
> > 
> > (and alpha_pa_access() is:)
> > 
> > int
> > alpha_pa_access(pa)
> > 	u_long pa;
> > {
> > 	int i;
> > 
> > 	for (i = 0; i < mem_cluster_cnt; i++) {
> > 		if (pa < mem_clusters[i].start)
> > 			continue;
> > 		if ((pa - mem_clusters[i].start) >=
> > 		    (mem_clusters[i].size & ~PAGE_MASK))
> > 			continue;
> > 		return (mem_clusters[i].size & PAGE_MASK); /* prot */
> > 	}
> > 
> > 	if (securelevel > 0)
> > 		return (PROT_NONE);
> > 	else
> > 		return (PROT_READ | PROT_WRITE);
> > }
> > 
> > so far so good. however, looking at amd64, vax, and powerpc we see
> > different behavior:
> > 
> > amd64:
> > if (off > ctob(physmem) && kauth_authorize_generic(l->l_cred,
> >     KAUTH_GENERIC_ISSUSER, &l->l_acflag) != 0)
> > 		return (-1);
> > 
> > vax:
> > if ((u_int)off > ctob(physmem) && kauth_authorize_generic(l->l_cred,
> >     KAUTH_GENERIC_ISSUSER, &l->l_acflag) != 0)
> > 		return (-1);
> > 
> > powerpc:
> > if (atop(off) >= physmem && kauth_authorize_generic(l->l_cred,
> >     KAUTH_GENERIC_ISSUSER, &l->l_acflag) != 0)
> > 		return (-1);
> > 
> > but alas, looking further, for example, hp300 and sun3, we see a
> > different behavior again:
> > 
> > hp300:
> > if ((u_int)off < lowram || (u_int)off >= 0xFFFFFFFC)
> > 	return -1;
> > 
> > sun3:
> > if (off < avail_start || off >= avail_end)
> > 	break;
> > 
> > 
> > now; i'm not an expert on any of these architectures so i can't tell
> > if there are special reasons why *eventually* these somehow achieve the
> > same behavior via different code, but the way i see it, we have three
> > (or more) kinds of behavior for more-or-less access to the same type
> > of resource (physical memory) on different architectures.
> > 
> > what i'm asking, is if the above is intentional (and if yes, if there
> > is any documentation on the matter), and if it's not, what should be
> > the consistent behavior.
> > 
> > thanks,
> > 
> > -e.
> > 
> 
> 
> -- 
> Elad Efrat