Subject: Re: inconsistency in MD mmmmap() implementations?
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Elad Efrat <elad@NetBSD.org>
List: tech-kern
Date: 10/30/2006 01:50:49
This is a multi-part message in MIME format.

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

YAMAMOTO Takashi wrote:
>>>> 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.
> 
> seems sane to me.
> 
> (except that it probably ought to be moved into x86.)

attached version for x86. is it okay? (tested on amd64 only)

notice that I didn't add check_pa_acc() prototype to any header in
x86/include because I didn't know where it was appropriate...

-e.

-- 
Elad Efrat

--Boundary_(ID_GcMkaMYKGgKAK6vyLsOUkA)
Content-type: text/plain; name=x86_pa_acc.diff
Content-transfer-encoding: 7BIT
Content-disposition: inline; filename=x86_pa_acc.diff

Index: 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
--- amd64/amd64/mem.c	23 Jul 2006 22:06:04 -0000	1.7
+++ amd64/amd64/mem.c	29 Oct 2006 23:49:21 -0000
@@ -115,6 +115,8 @@ const struct cdevsw mem_cdevsw = {
 	nostop, notty, nopoll, mmmmap, nokqfilter
 };
 
+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;
@@ -226,8 +231,6 @@ mmmmap(dev, off, prot)
 	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 +242,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));
 }
Index: i386/i386/mem.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/mem.c,v
retrieving revision 1.65
diff -u -p -r1.65 mem.c
--- i386/i386/mem.c	12 Oct 2006 04:31:03 -0000	1.65
+++ i386/i386/mem.c	29 Oct 2006 23:49:21 -0000
@@ -109,7 +109,7 @@ const struct cdevsw mem_cdevsw = {
 	nostop, notty, nopoll, mmmmap, nokqfilter, D_OTHER,
 };
 
-static int check_pa_acc(paddr_t, vm_prot_t);
+int check_pa_acc(paddr_t, vm_prot_t);
 
 /*ARGSUSED*/
 int
@@ -248,34 +248,3 @@ mmmmap(dev_t dev, off_t off, int prot)
 
 	return x86_btop(off);
 }
-
-/* ---------------------------------------- */
-
-#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 start = seg->start;
-
-		if (start <= pa && pa - start <= seg->size) {
-			return 0;
-		}
-	}
-
-	return EPERM;
-}
Index: x86/x86/x86_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/x86_machdep.c,v
retrieving revision 1.1
diff -u -p -r1.1 x86_machdep.c
--- x86/x86/x86_machdep.c	30 Dec 2005 13:37:57 -0000	1.1
+++ x86/x86/x86_machdep.c	29 Oct 2006 23:49:21 -0000
@@ -40,9 +40,17 @@
 __KERNEL_RCSID(0, "$NetBSD: x86_machdep.c,v 1.1 2005/12/30 13:37:57 jmmv Exp $");
 
 #include <sys/types.h>
+#include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/kcore.h>
+#include <sys/errno.h>
 
 #include <machine/bootinfo.h>
+#include <machine/vmparam.h>
+
+#include <uvm/uvm_extern.h>
+
+int check_pa_acc(paddr_t, vm_prot_t __unused);
 
 /* --------------------------------------------------------------------- */
 
@@ -79,3 +87,29 @@ lookup_bootinfo(int type)
 
 	return found ? bic : NULL;
 }
+
+/*
+ * check_pa_acc: check if given pa is accessible.
+ */
+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;
+}

--Boundary_(ID_GcMkaMYKGgKAK6vyLsOUkA)--