- enable remembering of device memory ranges set by PCI and

told to kernel
  - makes VM ask the kernel if a certain process is allowed
    to map in a range of physical memory (VM rounds it to page
    boundaries afterwards - but it's impossible to map anything
    smaller otherwise so I assume this is safe, i.e. there won't
    be anything else in that page; certainly no regular memory)
  - VM permission check cleanup (no more hardcoded calls, less
    hardcoded logic, more readable main loop), a loose end left
    by GQ
  - remove do_copy warning, as the ipc server triggers this but
    it's no more harmful than the special cases already excluded
    explicitly (VFS, PM, etc).

drivers/pci/pci.c

@@ -358,11 +358,6 @@ endpoint_t proc;
 			mr.mr_limit= mr.mr_base +
 				pcidev[devind].pd_bar[i].pb_size-1;
 
-			if(debug) {
-			   printf(
-	"pci_reserve3: for proc %d, should add memory range [0x%x..0x%x]\n",
-				proc, mr.mr_base, mr.mr_limit);
-			}
 			r= sys_privctl(proc, SYS_PRIV_ADD_MEM, &mr);
 			if (r != OK)
 			{

include/minix/com.h

@@ -362,6 +362,7 @@
 #define SYS_PRIV_USER		5	/* Make a process an oridinary user 
 					 * process.
 					 */
+#define SYS_PRIV_QUERY_MEM	6	/* Verify memory privilege. */
 
 /* Subfunctions for SYS_SETGRANT */
 #define SYS_PARAM_SET_GRANT	1	/* Set address and size of grant table */
@@ -512,6 +513,10 @@
 #define CTL_ADDRESS    m2_l1	/* address at traced process' space */
 #define CTL_DATA       m2_l2	/* data field for tracing */
 
+/* SYS_PRIVCTL with CTL_REQUEST == SYS_PRIV_QUERY_MEM */
+#define CTL_PHYSSTART  m2_l1	/* physical memory start in bytes*/
+#define CTL_PHYSLEN    m2_l2	/* length in bytes */
+
 /* Field names for SYS_SETGRANT */
 #define SG_ADDR		m2_p1	/* address */
 #define SG_SIZE		m2_i2	/* no. of entries */

include/minix/syslib.h

@@ -47,6 +47,8 @@ _PROTOTYPE( int sys_trace, (int req, endpoint_t proc_ep, long addr, long *data_p
 _PROTOTYPE( int sys_runctl, (endpoint_t proc_ep, int action, int flags));
 
 _PROTOTYPE( int sys_privctl, (endpoint_t proc_ep, int req, void *p));
+_PROTOTYPE( int sys_privquery_mem, (endpoint_t proc_ep,
+	phys_bytes physstart, phys_bytes physlen));
 _PROTOTYPE( int sys_setgrant, (cp_grant_t *grants, int ngrants));
 _PROTOTYPE( int sys_nice, (endpoint_t proc_ep, int priority));
 

kernel/system/do_copy.c

@@ -31,6 +31,7 @@ register message *m_ptr;	/* pointer to request message */
   phys_bytes bytes;		/* number of bytes to copy */
   int i;
 
+#if 0
   if (m_ptr->m_source != PM_PROC_NR && m_ptr->m_source != VFS_PROC_NR &&
 	m_ptr->m_source != RS_PROC_NR && m_ptr->m_source != MEM_PROC_NR &&
 	m_ptr->m_source != VM_PROC_NR)
@@ -48,6 +49,7 @@ register message *m_ptr;	/* pointer to request message */
 			m_ptr->CP_DST_SPACE);
 	}
   }
+#endif
 
   /* Dismember the command message. */
   vir_addr[_SRC_].proc_nr_e = m_ptr->CP_SRC_ENDPT;

kernel/system/do_privctl.c

@@ -27,7 +27,6 @@ message *m_ptr;			/* pointer to request message */
  */
   register struct proc *caller_ptr;
   register struct proc *rp;
-  register struct priv *sp;
   int proc_nr;
   int priv_id;
   int i, r;
@@ -198,16 +197,14 @@ message *m_ptr;			/* pointer to request message */
 	if((r=data_copy(who_e, (vir_bytes) m_ptr->CTL_ARG_PTR,
 		SYSTEM, (vir_bytes) &mem_range, sizeof(mem_range))) != OK)
 		return r;
-	priv(rp)->s_flags |= CHECK_MEM;	/* Check I/O accesses */
+	priv(rp)->s_flags |= CHECK_MEM;	/* Check memory mappings */
 	i= priv(rp)->s_nr_mem_range;
 	if (i >= NR_MEM_RANGE)
 		return ENOMEM;
 
-#if 0
 	priv(rp)->s_mem_tab[i].mr_base= mem_range.mr_base;
 	priv(rp)->s_mem_tab[i].mr_limit= mem_range.mr_limit;
 	priv(rp)->s_nr_mem_range++;
-#endif
 
 	return OK;
 
@@ -230,6 +227,28 @@ message *m_ptr;			/* pointer to request message */
 	priv(rp)->s_nr_irq++;
 
 	return OK;
+  case SYS_PRIV_QUERY_MEM:
+  {
+	phys_bytes addr, limit;
+  	struct priv *sp;
+	/* See if a certain process is allowed to map in certain physical
+	 * memory.
+	 */
+	addr = (phys_bytes) m_ptr->CTL_PHYSSTART;
+	limit = addr + (phys_bytes) m_ptr->CTL_PHYSLEN - 1;
+	if(limit < addr)
+		return EPERM;
+	if(!(sp = priv(rp)))
+		return EPERM;
+	if (!(sp->s_flags & SYS_PROC))
+		return EPERM;
+	for(i = 0; i < sp->s_nr_mem_range; i++) {
+		if(addr >= sp->s_mem_tab[i].mr_base &&
+		   limit <= sp->s_mem_tab[i].mr_limit)
+			return OK;
+	}
+	return EPERM;
+  }
   default:
 	kprintf("do_privctl: bad request %d\n", m_ptr->CTL_REQUEST);
 	return EINVAL;

lib/other/_vm_set_priv.c

@@ -9,3 +9,4 @@ PUBLIC int vm_set_priv(int nr, void *buf)
 	m.VM_RS_BUF = (long) buf;
 	return _syscall(VM_PROC_NR, VM_RS_SET_PRIV, &m);
 }
+

lib/syslib/sys_privctl.c

@@ -10,3 +10,15 @@ int sys_privctl(endpoint_t proc_ep, int request, void *p)
 
   return _taskcall(SYSTASK, SYS_PRIVCTL, &m);
 }
+
+int sys_privquery_mem(endpoint_t proc_ep, phys_bytes start, phys_bytes len)
+{
+  message m;
+
+  m.CTL_ENDPT = proc_ep;
+  m.CTL_REQUEST = SYS_PRIV_QUERY_MEM;
+  m.CTL_PHYSSTART = start;
+  m.CTL_PHYSLEN = len;
+
+  return _taskcall(SYSTASK, SYS_PRIVCTL, &m);
+}

servers/vm/main.c

@@ -44,10 +44,10 @@ typedef u32_t mask_t;
 #define MINEPM 0
 #define MAXMASK (sizeof(mask_t)*8)
 #define ANYEPM (MINEPM+MAXMASK-1)
-#define MAXEPM (ANYEPM-1)
+#define NEEDACL (MINEPM+MAXMASK-2)
+#define MAXEPM (NEEDACL-1)
 #define EPM(e) ((1L) << ((e)-MINEPM))
 #define EPMOK(mask, ep) (((mask) & EPM(ANYEPM)) || ((ep) >= MINEPM && (ep) <= MAXEPM && (EPM(ep) & (mask))))
-#define EPMANYOK(mask, ep) ((mask) & EPM(ANYEPM))
 
 /* Table of calls and a macro to test for being in range. */
 struct {
@@ -65,10 +65,8 @@ struct {
 			((c) - VM_RQ_BASE) : -1)
 
 FORWARD _PROTOTYPE(void vm_init, (void));
+FORWARD _PROTOTYPE(int vm_acl_ok, (endpoint_t caller, int call));
 
-#if SANITYCHECKS
-extern int kputc_use_private_grants;
-#endif
 
 /*===========================================================================*
  *				main					     *
@@ -131,35 +129,14 @@ PUBLIC int main(void)
 		continue;
 	}
 	who_e = msg.m_source;
-	c = msg.m_type - VM_RQ_BASE;
+	c = CALLNUMBER(msg.m_type);
 	result = ENOSYS; /* Out of range or restricted calls return this. */
-	if((c=CALLNUMBER(msg.m_type)) < 0 || !vm_calls[c].vmc_func) {
+	if(c < 0 || !vm_calls[c].vmc_func) {
 		printf("VM: out of range or missing callnr %d from %d\n",
-			msg.m_type, msg.m_source);
+			msg.m_type, who_e);
-	} else if(!EPMOK(vm_calls[c].vmc_callers, msg.m_source)) {
+	} else if (vm_acl_ok(who_e, c) != OK) {
-		printf("VM: restricted call %s from %d instead of 0x%lx\n",
+		printf("VM: unauthorized %s by %d\n",
-			vm_calls[c].vmc_name, msg.m_source,
-			vm_calls[c].vmc_callers);
-	} else if (EPMANYOK(vm_calls[c].vmc_callers, who_e) &&
-		   c != VM_MMAP-VM_RQ_BASE &&
-		   c != VM_MUNMAP_TEXT-VM_RQ_BASE &&
-		   c != VM_MUNMAP-VM_RQ_BASE) {
-		/* check VM acl, we care ANYEPM only,
-		 * and omit other hard-coded permission checks.
-		 */
-		int n;
-
-		if ((r = vm_isokendpt(who_e, &n)) != OK)
-			vm_panic("VM: from strange source.", who_e);
-
-		if (!GET_BIT(vmproc[n].vm_call_priv_mask, c))
-			printf("VM: restricted call %s from %d\n",
 			vm_calls[c].vmc_name, who_e);
-		else {
-	SANITYCHECK(SCL_FUNCTIONS);
-			result = vm_calls[c].vmc_func(&msg);
-	SANITYCHECK(SCL_FUNCTIONS);
-		}
 	} else {
 	SANITYCHECK(SCL_FUNCTIONS);
 		result = vm_calls[c].vmc_func(&msg);
@@ -326,7 +303,8 @@ PRIVATE void vm_init(void)
 	vm_calls[i].vmc_func = (func); 				      \
 	vm_calls[i].vmc_name = #code; 				      \
 	if(((thecaller) < MINEPM || (thecaller) > MAXEPM) 		\
-		&& (thecaller) != ANYEPM) {				\
+		&& (thecaller) != ANYEPM				\
+		&& (thecaller) != NEEDACL ) {				\
 		vm_panic(#thecaller " invalid", (code));  		\
 	}								\
 	vm_calls[i].vmc_callers |= EPM(thecaller);		      \
@@ -350,35 +328,23 @@ PRIVATE void vm_init(void)
 	CALLMAP(VM_ALLOCMEM, do_allocmem, PM_PROC_NR);
 	CALLMAP(VM_NOTIFY_SIG, do_notify_sig, PM_PROC_NR);
 
-	/* Physical mapping requests.
+	/* Requests from RS */
-	 * tty (for /dev/video) does this.
+	CALLMAP(VM_RS_SET_PRIV, do_rs_set_priv, RS_PROC_NR);
-	 * memory (for /dev/mem) does this.
-	 */
-	CALLMAP(VM_MAP_PHYS, do_map_phys, TTY_PROC_NR);
-	CALLMAP(VM_UNMAP_PHYS, do_unmap_phys, TTY_PROC_NR);
-	CALLMAP(VM_MAP_PHYS, do_map_phys, MEM_PROC_NR);
-	CALLMAP(VM_UNMAP_PHYS, do_unmap_phys, MEM_PROC_NR);
 
 	/* Requests from userland (source unrestricted). */
 	CALLMAP(VM_MMAP, do_mmap, ANYEPM);
 	CALLMAP(VM_MUNMAP, do_munmap, ANYEPM);
 	CALLMAP(VM_MUNMAP_TEXT, do_munmap, ANYEPM);
-	CALLMAP(VM_REMAP, do_remap, ANYEPM);
+	CALLMAP(VM_MAP_PHYS, do_map_phys, ANYEPM); /* Does its own checking. */
-	CALLMAP(VM_GETPHYS, do_get_phys, ANYEPM);
+	CALLMAP(VM_UNMAP_PHYS, do_unmap_phys, ANYEPM);
-	CALLMAP(VM_SHM_UNMAP, do_shared_unmap, ANYEPM);
-	CALLMAP(VM_GETREF, do_get_refcount, ANYEPM);
-	CALLMAP(VM_CTL, do_ctl, ANYEPM);
 
-	/* Request only from IPC server */
+	/* Requests from userland (anyone can call but need an ACL bit). */
-	CALLMAP(VM_QUERY_EXIT, do_query_exit, ANYEPM);
+	CALLMAP(VM_REMAP, do_remap, NEEDACL);
-
+	CALLMAP(VM_GETPHYS, do_get_phys, NEEDACL);
-	/* Requests (actually replies) from VFS (restricted to VFS only). */
+	CALLMAP(VM_SHM_UNMAP, do_shared_unmap, NEEDACL);
-	CALLMAP(VM_VFS_REPLY_OPEN, do_vfs_reply, VFS_PROC_NR);
+	CALLMAP(VM_GETREF, do_get_refcount, NEEDACL);
-	CALLMAP(VM_VFS_REPLY_MMAP, do_vfs_reply, VFS_PROC_NR);
+	CALLMAP(VM_CTL, do_ctl, NEEDACL);
-	CALLMAP(VM_VFS_REPLY_CLOSE, do_vfs_reply, VFS_PROC_NR);
+	CALLMAP(VM_QUERY_EXIT, do_query_exit, NEEDACL);
-
-	/* Requests from RS */
-	CALLMAP(VM_RS_SET_PRIV, do_rs_set_priv, RS_PROC_NR);
 
 	/* Sanity checks */
 	if(find_kernel_top() >= VM_PROCSTART)
@@ -392,3 +358,30 @@ PRIVATE void vm_init(void)
 	_minix_unmapzero();
 }
 
+/*===========================================================================*
+ *				vm_acl_ok				     *
+ *===========================================================================*/
+PRIVATE int vm_acl_ok(endpoint_t caller, int call)
+{
+	int n, r;
+
+	/* Some calls are always allowed by some, or all, processes. */
+	if(EPMOK(vm_calls[call].vmc_callers, caller)) {
+		return OK;
+	}
+
+	if ((r = vm_isokendpt(caller, &n)) != OK)
+		vm_panic("VM: from strange source.", caller);
+
+	/* Other calls need an ACL bit. */
+	if (!(vm_calls[call].vmc_callers & EPM(NEEDACL))) {
+		return EPERM;
+	}
+	if (!GET_BIT(vmproc[n].vm_call_priv_mask, call)) {
+		printf("VM: no ACL for %s for %d\n",
+			vm_calls[call].vmc_name, caller);
+		return EPERM;
+	}
+
+	return OK;
+}

servers/vm/mmap.c

@@ -88,6 +88,34 @@ PUBLIC int do_mmap(message *m)
 	return OK;
 }
 
+/*===========================================================================*
+ *				map_perm_check		     		     *
+ *===========================================================================*/
+PUBLIC int map_perm_check(endpoint_t caller, endpoint_t target,
+	phys_bytes physaddr, phys_bytes len)
+{
+	int r;
+
+	/* TTY and memory are allowed to do anything.
+	 * They have to be special cases as they have to be able to do
+	 * anything; TTY even on behalf of anyone for the TIOCMAPMEM
+	 * ioctl. MEM just for itself.
+	 */
+	if(caller == TTY_PROC_NR)
+		return OK;
+	if(caller != target)
+		return EPERM;
+	if(caller == MEM_PROC_NR)
+		return OK;
+
+	/* Anyone else needs explicit permission from the kernel (ultimately
+	 * set by PCI).
+	 */
+	r = sys_privquery_mem(caller, physaddr, len);
+
+	return r;
+}
+
 /*===========================================================================*
  *				do_map_phys		     		     *
  *===========================================================================*/
@@ -98,25 +126,38 @@ PUBLIC int do_map_phys(message *m)
 	endpoint_t target;
 	struct vir_region *vr;
 	vir_bytes len;
+	phys_bytes startaddr;
 
 	target = m->VMMP_EP;
+	len = m->VMMP_LEN;
+
 	if(target == SELF)
 		target = m->m_source;
 
 	if((r=vm_isokendpt(target, &n)) != OK)
 		return EINVAL;
 
+	startaddr = (vir_bytes)m->VMMP_PHADDR;
+
+	/* First check permission, then round range down/up. Caller can't
+	 * help it if we can't map in lower than page granularity.
+	 */
+	if(map_perm_check(m->m_source, target, startaddr, len) != OK) {
+		printf("VM: unauthorized mapping of 0x%lx by %d\n",
+			startaddr, m->m_source);
+		return EPERM;
+	}
+
 	vmp = &vmproc[n];
 
 	if(!(vmp->vm_flags & VMF_HASPT))
 		return ENXIO;
 
-	len = m->VMMP_LEN;
 	if(len % VM_PAGE_SIZE)
 		len += VM_PAGE_SIZE - (len % VM_PAGE_SIZE);
 
 	if(!(vr = map_page_region(vmp, arch_vir2map(vmp, vmp->vm_stacktop),
-		VM_DATATOP, len, (vir_bytes)m->VMMP_PHADDR,
+		VM_DATATOP, len, startaddr,
 		VR_DIRECT | VR_NOPF | VR_WRITABLE, 0))) {
 		return ENOMEM;
 	}

servers/vm/rs.c

@@ -51,6 +51,7 @@ PUBLIC int do_rs_set_priv(message *m)
 		if (r != OK)
 			return r;
 	}
+
 	return OK;
 }