User pointer validity checks.

Added routines that check whether a user pointer is accessible by the kernel,
and if not ask the pager to map-in those pages. I haven't implemented yet the
bit that asks the pager for paging-in.

src/api/kip.c

@@ -1,12 +1,12 @@
 /*
  * Kernel Interface Page and sys_kdata_read()
  *
- * Copyright (C) 2007 Bahadir Balban
- *
+ * Copyright (C) 2007, 2008 Bahadir Balban
  */
-
 #include <l4/generic/tcb.h>
 #include <l4/generic/physmem.h>
+#include <l4/generic/space.h>
+#include <l4/api/errno.h>
 #include INC_API(kip.h)
 #include INC_API(syscall.h)
 #include INC_GLUE(memlayout.h)
@@ -19,30 +19,25 @@ UNIT("kip") struct kip kip;
 int __sys_kread(int rd, void *dest)
 {
 	int err = 0;
+	unsigned long vaddr = (unsigned long)dest;
 
 	switch(rd) {
 	case KDATA_PAGE_MAP:
-		/*
-		 * FIXME:FIXME: Check if address is mapped here first!!!
-		 * Also check if process has enough buffer for physmem to fit!!!
-		 */
 		printk("Handling KDATA_PAGE_MAP request.\n");
+		if (check_access(vaddr, sizeof(page_map), MAP_USR_RW_FLAGS) < 0)
+			return -EINVAL;
 		memcpy(dest, &page_map, sizeof(page_map));
 		break;
 	case KDATA_BOOTDESC:
 		printk("Handling KDATA_BOOTDESC request.\n");
-		/*
-		 * FIXME:FIXME: Check if address is mapped here first!!!
-		 * Also check if process has enough buffer for physmem to fit!!!
-		 */
+		if (check_access(vaddr, bootdesc->desc_size, MAP_USR_RW_FLAGS) < 0)
+			return -EINVAL;
 		memcpy(dest, bootdesc, bootdesc->desc_size);
 		break;
 	case KDATA_BOOTDESC_SIZE:
 		printk("Handling KDATA_BOOTDESC_SIZE request.\n");
-		/*
-		 * FIXME:FIXME: Check if address is mapped here first!!!
-		 * Also check if process has enough buffer for physmem to fit!!!
-		 */
+		if (check_access(vaddr, sizeof(unsigned int), MAP_USR_RW_FLAGS) < 0)
+			return -EINVAL;
 		*(unsigned int *)dest = bootdesc->desc_size;
 		break;
 
@@ -69,10 +64,8 @@ int sys_kread(struct syscall_args *a)
 	int rd = (int)arg[0];	/* Request descriptor */
 
 	/* Error checking */
-	if ((rd < 0) || (addr <= 0)) {
-		printk("%s: Invalid arguments.\n", __FUNCTION__);
-		return -1;
-	}
+	if (rd < 0)
+		return -EINVAL;
 	return __sys_kread(rd, addr);
 }
 

src/arch/arm/exception.c

@@ -1,7 +1,7 @@
 /*
- * Debug print support for unexpected exceptions
+ * Memory exception handling in process context.
  *
- * Copyright (C) 2007 Bahadir Balban
+ * Copyright (C) 2007, 2008 Bahadir Balban
  */
 #include <l4/generic/scheduler.h>
 #include <l4/generic/space.h>
@@ -62,11 +62,19 @@ void fault_ipc_to_pager(u32 faulty_pc, u32 fsr, u32 far)
 	ipc_sendrecv(current->pagerid, current->pagerid);
 
 	/*
-	 * Pager is now notified and handling the fault. We now sleep on
-	 * another queue.
+	 * FIXME: CHECK TASK KILL REPLY !!!
+	 * Here, pager has handled the request and sent us back a message.
+	 * It is natural that a pager might want to kill the task due to
+	 * illegal access. Here we ought to check this and kill it rather
+	 * than return back to it.
 	 */
 }
 
+int pager_pagein_request(unsigned long addr, unsigned long size, unsigned int flags)
+{
+	return 0;
+}
+
 int check_aborts(u32 faulted_pc, u32 fsr, u32 far)
 {
 	int ret = 0;

src/arch/arm/v5/mm.c

@@ -191,54 +191,6 @@ void attach_pmd(pgd_table_t *pgd, pmd_table_t *pmd, unsigned int vaddr)
 	pgd->entry[pgd_i] |= PGD_TYPE_COARSE;
 }
 
-/*
- * Maps @paddr to @vaddr, covering @size bytes also allocates new pmd if
- * necessary. This flavor explicitly supplies the pgd to modify. This is useful
- * when modifying userspace of processes that are not currently running. (Only
- * makes sense for userspace mappings since kernel mappings are common.)
- */
-void add_mapping_pgd(unsigned int paddr, unsigned int vaddr,
-		     unsigned int size, unsigned int flags,
-		     pgd_table_t *pgd)
-{
-	pmd_table_t *pmd;
-	unsigned int numpages = (size >> PAGE_BITS);
-
-	if (size < PAGE_SIZE) {
-		printascii("Error: Mapping size must be in bytes not pages.\n");
-		while(1);
-	}
-	if (size & PAGE_MASK)
-		numpages++;
-
-	/* Convert generic map flags to pagetable-specific */
-	BUG_ON(!(flags = space_flags_to_ptflags(flags)));
-
-	/* Map all consecutive pages that cover given size */
-	for (int i = 0; i < numpages; i++) {
-		/* Check if another mapping already has a pmd attached. */
-		pmd = pmd_exists(pgd, vaddr);
-		if (!pmd) {
-			/*
-			 * If this is the first vaddr in
-			 * this pmd, allocate new pmd
-			 */
-			pmd = alloc_pmd();
-
-			/* Attach pmd to its entry in pgd */
-			attach_pmd(pgd, pmd, vaddr);
-		}
-
-		/* Attach paddr to this pmd */
-		__add_mapping(page_align(paddr),
-			      page_align(vaddr), flags, pmd);
-
-		/* Go to the next page to be mapped */
-		paddr += PAGE_SIZE;
-		vaddr += PAGE_SIZE;
-	}
-}
-
 #if 0
 /* Maps @paddr to @vaddr, covering @size bytes,
  * also allocates new pmd if necessary. */
@@ -326,12 +278,93 @@ void add_mapping(unsigned int paddr, unsigned int vaddr,
 }
 #endif
 
+/*
+ * Maps @paddr to @vaddr, covering @size bytes also allocates new pmd if
+ * necessary. This flavor explicitly supplies the pgd to modify. This is useful
+ * when modifying userspace of processes that are not currently running. (Only
+ * makes sense for userspace mappings since kernel mappings are common.)
+ */
+void add_mapping_pgd(unsigned int paddr, unsigned int vaddr,
+		     unsigned int size, unsigned int flags,
+		     pgd_table_t *pgd)
+{
+	pmd_table_t *pmd;
+	unsigned int numpages = (size >> PAGE_BITS);
+
+	if (size < PAGE_SIZE) {
+		printascii("Error: Mapping size must be in bytes not pages.\n");
+		while(1);
+	}
+	if (size & PAGE_MASK)
+		numpages++;
+
+	/* Convert generic map flags to pagetable-specific */
+	BUG_ON(!(flags = space_flags_to_ptflags(flags)));
+
+	/* Map all consecutive pages that cover given size */
+	for (int i = 0; i < numpages; i++) {
+		/* Check if another mapping already has a pmd attached. */
+		pmd = pmd_exists(pgd, vaddr);
+		if (!pmd) {
+			/*
+			 * If this is the first vaddr in
+			 * this pmd, allocate new pmd
+			 */
+			pmd = alloc_pmd();
+
+			/* Attach pmd to its entry in pgd */
+			attach_pmd(pgd, pmd, vaddr);
+		}
+
+		/* Attach paddr to this pmd */
+		__add_mapping(page_align(paddr),
+			      page_align(vaddr), flags, pmd);
+
+		/* Go to the next page to be mapped */
+		paddr += PAGE_SIZE;
+		vaddr += PAGE_SIZE;
+	}
+}
+
 void add_mapping(unsigned int paddr, unsigned int vaddr,
 		 unsigned int size, unsigned int flags)
 {
 	add_mapping_pgd(paddr, vaddr, size, flags, current->pgd);
 }
 
+/*
+ * Checks if a virtual address range has same or more permissive
+ * flags than the given ones, returns 0 if not, and 1 if OK.
+ */
+int check_mapping_pgd(unsigned long vaddr, unsigned long size,
+		      unsigned int flags, pgd_table_t *pgd)
+{
+	unsigned int npages = __pfn(align_up(size, PAGE_SIZE));
+	pte_t pte;
+
+	/* Convert generic map flags to pagetable-specific */
+	BUG_ON(!(flags = space_flags_to_ptflags(flags)));
+	
+	for (int i = 0; i < npages; i++) {
+		pte = virt_to_pte(vaddr + i * PAGE_SIZE);
+
+		/* Check if pte perms are equal or gt given flags */
+		if ((pte & PTE_PROT_MASK) >= (flags & PTE_PROT_MASK))
+			continue;
+		else
+			return 0;
+	}
+
+	return 1;
+}
+
+
+int check_mapping(unsigned long vaddr, unsigned long size,
+		  unsigned int flags)
+{
+	return check_mapping_pgd(vaddr, size, flags, current->pgd);
+}
+
 /* FIXME: Empty PMDs should be returned here !!! */
 void __remove_mapping(pmd_table_t *pmd, unsigned long vaddr)
 {

src/generic/SConscript

@@ -4,7 +4,7 @@
 Import('env')
 
 # The set of source files associated with this SConscript file.
-src_local = ['physmem.c', 'irq.c', 'scheduler.c', 'time.c', 'tcb.c', 'pgalloc.c', 'kmalloc.c']
+src_local = ['physmem.c', 'irq.c', 'scheduler.c', 'time.c', 'tcb.c', 'pgalloc.c', 'kmalloc.c', 'space.c']
 
 obj = env.Object(src_local)
 Return('obj')

src/generic/space.c

@@ -0,0 +1,38 @@
+/*
+ * Addess space related routines.
+ *
+ * Copyright (C) 2008 Bahadir Balban
+ */
+#include INC_GLUE(memory.h)
+#include INC_GLUE(memlayout.h)
+#include INC_ARCH(exception.h)
+#include <l4/generic/space.h>
+#include <l4/api/space.h>
+#include <l4/api/errno.h>
+
+/*
+ * Checks whether the given user address is a valid userspace address.
+ * If so, whether it is currently mapped into its own address space.
+ * If its not mapped-in, it generates a page-in request to the thread's
+ * pager. If fault hasn't cleared, aborts.
+ */
+int check_access(unsigned long vaddr, unsigned long size, unsigned int flags)
+{
+	int err;
+
+	/* Do not allow ridiculously big sizes */
+	if (size >= USER_AREA_SIZE)
+		return -EINVAL;
+
+	/* Check if in user range, but this is more up to the pager to decide */
+	if (!(vaddr >= USER_AREA_START && vaddr < USER_AREA_END))
+		return -EINVAL;
+
+	/* If not mapped, ask pager whether this is possible */
+	if (!check_mapping(vaddr, size, flags))
+		if((err = pager_pagein_request(vaddr, size, flags)) < 0)
+			return err;
+
+	return 0;
+}
+