This is a multi-part message in MIME format.

--Boundary_(ID_1qVwx2/kaoiCY4Iem1P4rQ)
Content-type: text/plain; charset=ISO-8859-1
Content-transfer-encoding: 7BIT

YAMAMOTO Takashi wrote:
>> YAMAMOTO Takashi wrote:
>>
>>> 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?)
>> I'm interested in both knowing if these are not wrong (like you say,
>> amd64 is wrong, maybe others are too?) and I also want to use kauth(9)
>> there. So before I'm writing the code I'm verifying that the current
>> behavior is correct.
> 
> i386 and alpha seems sane.  amd64 seems wrong.
> i don't know others.
> 
>>> 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.
>> Do you want to fix it?
> 
> i don't think i'll fix it in a timely manner.
> 
>> or should we wait for the amd64 port-master?
> 
> no.  it shouldn't be too hard for anyone interested.

I tried making a patch (adapted copy/paste of the i386 code) for amd64;
does it look okay?

-e.

-- 
Elad Efrat

--Boundary_(ID_1qVwx2/kaoiCY4Iem1P4rQ)
Content-type: text/plain; name=amd64_mem.c.diff
Content-transfer-encoding: 7BIT
Content-disposition: inline; filename=amd64_mem.c.diff

Index: sys/arch/amd64/amd64/mem.c
===================================================================
RCS file: /cvsroot/src/sys/arch/amd64/amd64/mem.c,v
retrieving revision 1.7
diff -u -p -r1.7 mem.c
--- sys/arch/amd64/amd64/mem.c	23 Jul 2006 22:06:04 -0000	1.7
+++ sys/arch/amd64/amd64/mem.c	29 Oct 2006 23:14:07 -0000
@@ -115,6 +115,8 @@ const struct cdevsw mem_cdevsw = {
 	nostop, notty, nopoll, mmmmap, nokqfilter
 };
 
+static int check_pa_acc(paddr_t, vm_prot_t);
+
 /*ARGSUSED*/
 int
 mmrw(dev, uio, flags)
@@ -155,6 +157,9 @@ mmrw(dev, uio, flags)
 			v = uio->uio_offset;
 			prot = uio->uio_rw == UIO_READ ? VM_PROT_READ :
 			    VM_PROT_WRITE;
+			error = check_pa_acc(uio->uio_offset, prot);
+			if (error)
+				break;
 			pmap_enter(pmap_kernel(), (vaddr_t)vmmap,
 			    trunc_page(v), prot, PMAP_WIRED|prot);
 			o = uio->uio_offset & PGOFSET;
@@ -220,14 +225,40 @@ mmrw(dev, uio, flags)
 	return (error);
 }
 
+#include <sys/kcore.h>
+
+/*
+ * check_pa_acc: check if given pa is accessible.
+ */
+static int
+check_pa_acc(paddr_t pa, vm_prot_t prot __unused)
+{
+	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 lstart = seg->start;
+
+		if (lstart <= pa && pa - lstart <= seg->size) {
+			return 0;
+		}
+	}
+
+	return EPERM;
+}
+
 paddr_t
 mmmmap(dev, off, prot)
 	dev_t dev;
 	off_t off;
 	int prot;
 {
-	struct lwp *l = curlwp;	/* XXX */
-
 	/*
 	 * /dev/mem is the only one that makes sense through this
 	 * interface.  For /dev/kmem any physaddr we return here
@@ -239,8 +270,8 @@ mmmmap(dev, off, prot)
 	if (minor(dev) != DEV_MEM)
 		return (-1);
 
-	if (off > ctob(physmem) && kauth_authorize_generic(l->l_cred,
-	    KAUTH_GENERIC_ISSUSER, &l->l_acflag) != 0)
+	if (check_pa_acc(off, prot) != 0)
 		return (-1);
+
 	return (x86_btop(off));
 }

--Boundary_(ID_1qVwx2/kaoiCY4Iem1P4rQ)--