syscall/memflag: implement freeing memory

4 files changed, 63 insertions, 26 deletions

diff --git a/src/init/tests/main.c b/src/init/tests/main.c
index 182c545..20923b0 100644
--- a/src/init/tests/main.c
+++ b/src/init/tests/main.c
@@ -1,5 +1,6 @@
 #include <init/stdlib.h>
 #include <init/tests/main.h>
+#include <shared/flags.h>
 #include <shared/syscalls.h>
 
 #define argify(str) str, sizeof(str) - 1
@@ -91,6 +92,23 @@ static void test_orphaned_fs(void) {
 	}
 }
 
+static void test_memflag(void) {
+	void *page = (void*)0x77777000;
+	_syscall_memflag(page, 4096, MEMFLAG_PRESENT); // allocate page
+	memset(page, 77, 4096); // write to it
+	_syscall_memflag(page, 4096, 0); // free it
+
+	if (!_syscall_fork(0, NULL)) {
+		memset(page, 11, 4096); // should segfault
+		_syscall_exit(0);
+	} else {
+		assert(_syscall_await() != 0); // test if the process crashed
+	}
+
+	_syscall_memflag((void*)0x100000, 4096, 0); // try to free kernel memory
+	// TODO the kernel shouldn't even be mapped in userland
+}
+
 static void stress_fork(void) {
 	/* run a lot of processes */
 	for (size_t i = 0; i < 2048; i++) {
@@ -105,5 +123,6 @@ void test_all(void) {
 	run_forked(test_faults);
 	run_forked(test_interrupted_fs);
 	run_forked(test_orphaned_fs);
+	run_forked(test_memflag);
 	run_forked(stress_fork);
 }
diff --git a/src/kernel/arch/generic.h b/src/kernel/arch/generic.h
index fd44e38..3e98001 100644
--- a/src/kernel/arch/generic.h
+++ b/src/kernel/arch/generic.h
@@ -30,8 +30,10 @@ struct pagedir *pagedir_new(void);
 struct pagedir *pagedir_copy(const struct pagedir *orig);
 
 void pagedir_free(struct pagedir *);
+void *pagedir_unmap(struct pagedir *dir, void __user *virt);
 void pagedir_map(struct pagedir *dir, void __user *virt, void *phys,
                  bool user, bool writeable);
+bool pagedir_iskern(struct pagedir *, const void __user *virt);
 
 void pagedir_switch(struct pagedir *);
 
diff --git a/src/kernel/arch/i386/pagedir.c b/src/kernel/arch/i386/pagedir.c
index 00539b7..28608dc 100644
--- a/src/kernel/arch/i386/pagedir.c
+++ b/src/kernel/arch/i386/pagedir.c
@@ -68,6 +68,25 @@ void pagedir_free(struct pagedir *dir) {
 	page_free(dir, 1);
 }
 
+static struct pagetable_entry*
+get_entry(struct pagedir *dir, const void __user *virt) {
+	uint32_t pd_idx = ((uintptr_t)virt) >> 22;
+	uint32_t pt_idx = ((uintptr_t)virt) >> 12 & 0x03FF;
+	struct pagetable_entry *pagetable;
+
+	if (!dir->e[pd_idx].present) return NULL;
+
+	pagetable = (void*)(dir->e[pd_idx].address << 11);
+	return &pagetable[pt_idx];
+}
+
+void *pagedir_unmap(struct pagedir *dir, void __user *virt) {
+	void *phys = pagedir_virt2phys(dir, virt, false, false);
+	struct pagetable_entry *page = get_entry(dir, virt);
+	page->present = false;
+	return phys;
+}
+
 void pagedir_map(struct pagedir *dir, void __user *virt, void *phys,
                  bool user, bool writeable)
 {
@@ -146,29 +165,22 @@ struct pagedir *pagedir_copy(const struct pagedir *orig) {
 	return clone;
 }
 
+bool pagedir_iskern(struct pagedir *dir, const void __user *virt) {
+	struct pagetable_entry *page = get_entry(dir, virt);
+	return page && page->present && !page->user;
+}
+
 void *pagedir_virt2phys(struct pagedir *dir, const void __user *virt,
                         bool user, bool writeable)
 {
-	uintptr_t virt_cast = (uintptr_t) virt;
+	struct pagetable_entry *page;
 	uintptr_t phys;
-	uint32_t pd_idx = virt_cast >> 22;
-	uint32_t pt_idx = virt_cast >> 12 & 0x03FF;
-	struct pagetable_entry *pagetable, page;
-
-	/* DOESN'T CHECK PERMISSIONS ON PAGE DIRS, TODO
-	 * while i don't currently see a reason to set permissions
-	 * directly on page dirs, i might see one in the future.
-	 * leaving this as-is would be a security bug */
-	if (!dir->e[pd_idx].present) return NULL;
-
-	pagetable = (void*)(dir->e[pd_idx].address << 11);
-	page      = pagetable[pt_idx];
-
-	if (!page.present)                return NULL;
-	if (user      && !page.user)      return NULL;
-	if (writeable && !page.writeable) return NULL;
+	page = get_entry(dir, virt);
+	if (!page || !page->present) return NULL;
+	if (user && !page->user) return NULL;
+	if (writeable && !page->writeable) return NULL;
 
-	phys  = page.address << 11;
-	phys |= virt_cast & 0xFFF;
+	phys  = page->address << 11;
+	phys |= ((uintptr_t)virt) & 0xFFF;
 	return (void*)phys;
 }
diff --git a/src/kernel/syscalls.c b/src/kernel/syscalls.c
index 35b243f..560cd37 100644
--- a/src/kernel/syscalls.c
+++ b/src/kernel/syscalls.c
@@ -253,17 +253,21 @@ int _syscall_fs_respond(char __user *buf, int ret) {
 int _syscall_memflag(void __user *addr, size_t len, int flags) {
 	userptr_t goal = addr + len;
 	struct pagedir *pages = process_current->pages;
-	if (flags != MEMFLAG_PRESENT) panic_unimplemented(); // TODO
 
 	addr = (userptr_t)((int __force)addr & ~PAGE_MASK); // align to page boundary
 	for (; addr < goal; addr += PAGE_SIZE) {
-		if (pagedir_virt2phys(pages, addr, false, false)) {
-			// allocated page, currently a no-op
-			// if you'll be changing this - remember to check if the pages are owned by the kernel!
-		} else {
-			// allocate the new pages
-			pagedir_map(pages, addr, page_alloc(1), true, true);
+		if (pagedir_iskern(pages, addr)) {
+			// TODO reflect failure in return value
+			continue;
+		}
+
+		if (!(flags & MEMFLAG_PRESENT)) {
+			page_free(pagedir_unmap(pages, addr), 1);
+			continue;
 		}
+
+		if (!pagedir_virt2phys(pages, addr, false, false))
+			pagedir_map(pages, addr, page_alloc(1), true, true);
 	}
 
 	SYSCALL_RETURN(-1);