Add missing mutex-locking in procesc.c ; discovered design fault somewhere.

6 files changed, 109 insertions, 28 deletions

diff --git a/src/kernel/core/frame.c b/src/kernel/core/frame.c
index 489d010..255abb6 100644
--- a/src/kernel/core/frame.c
+++ b/src/kernel/core/frame.c
@@ -53,6 +53,7 @@ uint32_t frame_alloc(size_t n) {
 				nused_frames += n;
 
 				mutex_unlock(&frame_allocator_mutex);
+				/*dbg_printf("AF 0x%p\n", i * 32 + j);*/
 				return i * 32 + j;
 			}
 		}
@@ -65,6 +66,7 @@ void frame_free(uint32_t base, size_t n) {
 	mutex_lock(&frame_allocator_mutex);
 
 	for (size_t i = 0; i < n; i++) {
+		/*dbg_printf("FF 0x%p\n", base + i);*/
 		uint32_t idx = INDEX_FROM_BIT(base + i);
 		uint32_t ofs = OFFSET_FROM_BIT(base + i);
 		if (frame_bitset[idx] & (0x1 << ofs)) {
diff --git a/src/kernel/core/paging.c b/src/kernel/core/paging.c
index fa22879..f7ebf9c 100644
--- a/src/kernel/core/paging.c
+++ b/src/kernel/core/paging.c
@@ -3,7 +3,6 @@
 #include <idt.h>
 #include <dbglog.h>
 #include <region.h>
-#include <mutex.h>
 #include <thread.h>
 #include <malloc.h>
 #include <freemem.h>
@@ -25,8 +24,6 @@ struct page_directory {
 
 	user_pf_handler_t user_pfh;
 	void* user_pfh_data;
-
-	mutex_t mutex;
 };
 
 
@@ -60,6 +57,11 @@ void page_fault_handler(registers_t *regs) {
 			ASSERT(current_thread->user_ex_handler != 0);
 			current_thread->user_ex_handler(regs);
 		} else {
+			if (pd->user_pfh == 0) {
+				dbg_printf("Error: usermode page fault on PD with no user PFH.\n");
+				dbg_printf("PD: 0x%p, kernel PD: 0x%p\n", get_current_pagedir(), get_kernel_pagedir());
+				PANIC("Un-handlable usermode PF.\n");
+			}
 			ASSERT(pd->user_pfh != 0);
 			pd->user_pfh(pd->user_pfh_data, regs, vaddr);
 		}
@@ -122,7 +124,6 @@ void paging_setup(void* kernel_data_end) {
 
 	// setup kernel_pd_d structure
 	kernel_pd_d.phys_addr = (size_t)&kernel_pd - K_HIGHHALF_ADDR;
-	kernel_pd_d.mutex = MUTEX_UNLOCKED;
 
 	// setup kernel_pt0
 	ASSERT(PAGE_OF_ADDR(K_HIGHHALF_ADDR) == 0);	// kernel is 4M-aligned
@@ -201,15 +202,15 @@ bool pd_map_page(void* vaddr, uint32_t frame_id, bool rw) {
 	
 	bool on_kernel_pd = (size_t)vaddr >= K_HIGHHALF_ADDR || current_thread == 0;
 	
-	pagedir_t *pdd = (on_kernel_pd ? &kernel_pd_d : current_thread->current_pd_d);
+	// pagedir_t *pdd = (on_kernel_pd ? &kernel_pd_d : current_thread->current_pd_d);
 	pagetable_t *pd = (on_kernel_pd ? &kernel_pd : current_pd);
 
-	mutex_lock(&pdd->mutex);
+	int st = enter_critical(CL_NOINT);
 
 	if (!(pd->page[pt] & PTE_PRESENT)) {
 		uint32_t new_pt_frame = frame_alloc(1);
 		if (new_pt_frame == 0) {
-			mutex_unlock(&pdd->mutex);
+			exit_critical(st);
 			return false;
 		}
 
@@ -227,7 +228,7 @@ bool pd_map_page(void* vaddr, uint32_t frame_id, bool rw) {
 			| (rw ? PTE_RW : 0);
 	invlpg(vaddr);
 
-	mutex_unlock(&pdd->mutex);
+	exit_critical(st);
 	return true;
 } 
 
@@ -236,7 +237,6 @@ void pd_unmap_page(void* vaddr) {
 	uint32_t page = PAGE_OF_ADDR(vaddr);
 
 	pagetable_t *pd = ((size_t)vaddr >= K_HIGHHALF_ADDR ? &kernel_pd : current_pd);
-	// no need to lock the PD's mutex
 
 	if (!(pd->page[pt] & PTE_PRESENT)) return;
 	if (!(current_pt[pt].page[page] & PTE_PRESENT)) return;
@@ -270,7 +270,6 @@ pagedir_t *create_pagedir(user_pf_handler_t pf, void* pfd) {
 	if (!map_ok) goto error;
 
 	pd->phys_addr = pd_phys * PAGE_SIZE;
-	pd->mutex = MUTEX_UNLOCKED;
 
 	pd->user_pfh = pf;
 	pd->user_pfh_data = pfd;
diff --git a/src/kernel/core/prng.c b/src/kernel/core/prng.c
index dbffba5..77dff7d 100644
--- a/src/kernel/core/prng.c
+++ b/src/kernel/core/prng.c
@@ -11,6 +11,8 @@ static const uint32_t a = 16807;
 static const uint32_t m = 0x7FFFFFFF;
 
 uint16_t prng_word() {
+	int st = enter_critical(CL_NOINT);
+
 	if (++n >= 100) {
 		n = 0;
 		if (entropy_count) {
@@ -18,7 +20,12 @@ uint16_t prng_word() {
 		}
 	}
 	x = (x * a) % m;
-	return x & 0xFFFF;
+
+	uint16_t ret = x & 0xFFFF;
+
+	exit_critical(st);
+
+	return ret;
 }
 
 void prng_bytes(uint8_t* out, size_t nbytes) {
diff --git a/src/kernel/core/sys.c b/src/kernel/core/sys.c
index 554c95e..290fdfa 100644
--- a/src/kernel/core/sys.c
+++ b/src/kernel/core/sys.c
@@ -71,7 +71,7 @@ void load_kernel_symbol_map(char* text, size_t len) {
 
 void kernel_stacktrace(uint32_t ebp, uint32_t eip) {
 	int i = 0;
-	while (ebp >= K_HIGHHALF_ADDR && eip >= K_HIGHHALF_ADDR && i++ < 10) {
+	while (ebp >= K_HIGHHALF_ADDR && eip >= K_HIGHHALF_ADDR) {
 		char* sym = 0;
 		if (kernel_symbol_map != 0) sym = btree_lower(kernel_symbol_map, (void*)eip);
 
@@ -80,6 +80,10 @@ void kernel_stacktrace(uint32_t ebp, uint32_t eip) {
 		uint32_t *d = (uint32_t*)ebp;
 		ebp = d[0];
 		eip = d[1];
+		if (i++ == 20) {
+			dbg_printf("| ...");
+			break;
+		}
 	}
 }
 
diff --git a/src/kernel/core/thread.c b/src/kernel/core/thread.c
index 1009ec7..53b3e70 100644
--- a/src/kernel/core/thread.c
+++ b/src/kernel/core/thread.c
@@ -43,8 +43,6 @@ void set_pit_frequency(uint32_t freq) {
 int enter_critical(int level) {
 	asm volatile("cli");
 
-	/*dbg_printf(" >%d< ", level);*/
-
 	if (current_thread == 0) return CL_EXCL;
 
 	int prev_level = current_thread->critical_level;
@@ -52,18 +50,20 @@ int enter_critical(int level) {
 	
 	if (current_thread->critical_level < CL_NOINT) asm volatile("sti");
 
+	/*dbg_printf(" >%d< ", current_thread->critical_level);*/
+
 	return prev_level;
 }
 
 void exit_critical(int prev_level) {
 	asm volatile("cli");
 
-	/*dbg_printf(" <%d> ", prev_level);*/
-
 	if (current_thread == 0) return;
 
 	if (prev_level < current_thread->critical_level) current_thread->critical_level = prev_level;
 	if (current_thread->critical_level < CL_NOINT) asm volatile("sti");
+
+	/*dbg_printf(" <%d> ", current_thread->critical_level);*/
 }
 
 // ================== //
@@ -315,12 +315,15 @@ bool wait_on_many(void** x, size_t n) {
 		waiters = current_thread->next_waiter;
 	} else {
 		ASSERT(waiters != 0);
+		bool deleted = false;
 		for (thread_t *w = waiters; w->next_waiter != 0; w = w->next_waiter) {
 			if (w->next_waiter == current_thread) {
 				w->next_waiter = current_thread->next_waiter;
+				deleted = true;
 				break;
 			}
 		}
+		ASSERT(deleted);
 	}
 
 	exit_critical(st);
diff --git a/src/kernel/user/process.c b/src/kernel/user/process.c
index 00a7e44..2a4d98f 100644
--- a/src/kernel/user/process.c
+++ b/src/kernel/user/process.c
@@ -143,14 +143,14 @@ bool start_process(process_t *p, void* entry) {
 	bool stack_ok = mmap(p, (void*)USERSTACK_ADDR, USERSTACK_SIZE, MM_READ | MM_WRITE);
 	if (!stack_ok) return false;
 
+	p->status = PS_RUNNING;
+
 	bool ok = process_new_thread(p, entry, (void*)USERSTACK_ADDR + USERSTACK_SIZE);
 	if (!ok) {
 		munmap(p, (void*)USERSTACK_ADDR);
 		return false;
 	}
 
-	p->status = PS_RUNNING;
-
 	return true;
 }
 
@@ -304,9 +304,13 @@ process_t *process_find_child(process_t *p, int pid) {
 }
 
 void process_get_status(process_t *p, proc_status_t *st) {
+	mutex_lock(&p->lock);
+
 	st->pid = p->pid;
 	st->status = p->status;
 	st->exit_code = p->exit_code;
+
+	mutex_unlock(&p->lock);
 }
 
 void process_wait(process_t *p, proc_status_t *st, bool wait) {
@@ -433,7 +437,16 @@ bool proc_add_fs(process_t *p, fs_t *fs, const char* name) {
 	char *n = strdup(name);
 	if (n == 0) return false;
 
-	bool add_ok = hashtbl_add(p->filesystems, n, fs);
+	bool add_ok = false;
+
+	mutex_lock(&p->lock);
+
+	if (hashtbl_find(p->filesystems, n) != 0) goto end;
+
+	add_ok = hashtbl_add(p->filesystems, n, fs);
+
+end:
+	mutex_unlock(&p->lock);
 	if (!add_ok) {
 		free(n);
 		return false;
@@ -443,21 +456,35 @@ bool proc_add_fs(process_t *p, fs_t *fs, const char* name) {
 }
 
 fs_t *proc_find_fs(process_t *p, const char* name) {
-	return (fs_t*)hashtbl_find(p->filesystems, name);
+	mutex_lock(&p->lock);
+
+	fs_t *ret = (fs_t*)hashtbl_find(p->filesystems, name);
+
+	mutex_unlock(&p->lock);
+	return ret;
 }
 
 void proc_rm_fs(process_t *p, const char* name) {
 	fs_t *fs = proc_find_fs(p, name);
 	if (fs) {
+		mutex_lock(&p->lock);
+
 		unref_fs(fs);
 		hashtbl_remove(p->filesystems, name);
+
+		mutex_unlock(&p->lock);
 	}
 }
 
 int proc_add_fd(process_t *p, fs_handle_t *f) {
 	int fd = p->next_fd++;
 
+	mutex_lock(&p->lock);
+
 	bool add_ok = hashtbl_add(p->files, (void*)fd, f);
+
+	mutex_unlock(&p->lock);
+
 	if (!add_ok) return 0;
 
 	return fd;
@@ -466,24 +493,40 @@ int proc_add_fd(process_t *p, fs_handle_t *f) {
 bool proc_add_fd_as(process_t *p, fs_handle_t *f, int fd) {
 	if (fd <= 0) return false;
 
-	if (hashtbl_find(p->files, (void*)fd) != 0) return false;
+	bool add_ok = false;
+
+	mutex_lock(&p->lock);
+
+	if (hashtbl_find(p->files, (void*)fd) != 0) goto end;
 
 	if (fd >= p->next_fd) p->next_fd = fd + 1;
 
-	bool add_ok = hashtbl_add(p->files, (void*)fd, f);
+	add_ok = hashtbl_add(p->files, (void*)fd, f);
+
+end:
+	mutex_unlock(&p->lock);
 	return add_ok;
 }
 
 fs_handle_t *proc_read_fd(process_t *p, int fd) {
-	return (fs_handle_t*)hashtbl_find(p->files, (void*)fd);
+	mutex_lock(&p->lock);
+
+	fs_handle_t *ret = (fs_handle_t*)hashtbl_find(p->files, (void*)fd);
+
+	mutex_unlock(&p->lock);
+	return ret;
 }
 
 void proc_close_fd(process_t *p, int fd) {
-	fs_handle_t *x = proc_read_fd(p, fd);
+	mutex_lock(&p->lock);
+
+	fs_handle_t *x = (fs_handle_t*)hashtbl_find(p->files, (void*)fd);
 	if (x != 0) {
 		unref_file(x);
 		hashtbl_remove(p->files, (void*)fd);
 	}
+
+	mutex_unlock(&p->lock);
 }
 
 // ============================= //
@@ -491,12 +534,17 @@ void proc_close_fd(process_t *p, int fd) {
 // ============================= //
 
 user_region_t *find_user_region(process_t *proc, const void* addr) {
+	mutex_lock(&proc->lock);
+
 	user_region_t *r = (user_region_t*)btree_lower(proc->regions_idx, addr);
-	if (r == 0) return 0;
 
-	ASSERT(addr >= r->addr);
+	if (r != 0) {
+		ASSERT(addr >= r->addr);
 
-	if (addr >= r->addr + r->size) return 0;
+		if (addr >= r->addr + r->size) r = 0;
+	}
+
+	mutex_unlock(&proc->lock);
 	return r;
 }
 
@@ -511,6 +559,8 @@ bool mmap(process_t *proc, void* addr, size_t size, int mode) {
 	user_region_t *r = (user_region_t*)malloc(sizeof(user_region_t));
 	if (r == 0) return false;
 
+	mutex_lock(&proc->lock);
+
 	r->proc = proc;
 	r->addr = addr;
 	r->size = size;
@@ -531,11 +581,13 @@ bool mmap(process_t *proc, void* addr, size_t size, int mode) {
 
 	pager_add_map(r->pager, r);
 
+	mutex_unlock(&proc->lock);
 	return true;
 
 error:
 	if (r && r->pager) delete_pager(r->pager);
 	if (r) free(r);
+	mutex_unlock(&proc->lock);
 	return false;
 }
 
@@ -557,6 +609,8 @@ bool mmap_file(process_t *proc, fs_handle_t *h, size_t offset, void* addr, size_
 	user_region_t *r = (user_region_t*)malloc(sizeof(user_region_t));
 	if (r == 0) return false;
 
+	mutex_lock(&proc->lock);
+
 	r->addr = addr;
 	r->size = size;
 
@@ -576,10 +630,12 @@ bool mmap_file(process_t *proc, fs_handle_t *h, size_t offset, void* addr, size_
 	r->next_in_proc = proc->regions;
 	proc->regions = r;
 
+	mutex_unlock(&proc->lock);
 	return true;
 
 error:
 	if (r) free(r);
+	mutex_unlock(&proc->lock);
 	return false;
 }
 
@@ -591,6 +647,9 @@ bool mchmap(process_t *proc, void* addr, int mode) {
 	if (r->file != 0) {
 		if ((mode & MM_WRITE) && !(r->file->mode & FM_WRITE)) return false;
 	}
+
+	mutex_lock(&proc->lock);
+
 	r->mode = mode;
 
 	// change mode on already mapped pages
@@ -610,6 +669,7 @@ bool mchmap(process_t *proc, void* addr, int mode) {
 	}
 	switch_pagedir(save_pd);
 
+	mutex_unlock(&proc->lock);
 	return true;
 }
 
@@ -617,6 +677,8 @@ bool munmap(process_t *proc, void* addr) {
 	user_region_t *r = find_user_region(proc, addr);
 	if (r == 0) return false;
 
+	mutex_lock(&proc->lock);
+
 	if (proc->regions == r) {
 		proc->regions = r->next_in_proc;
 	} else {
@@ -645,6 +707,8 @@ bool munmap(process_t *proc, void* addr) {
 	}
 	switch_pagedir(save_pd);
 
+	mutex_unlock(&proc->lock);
+
 	pager_rm_map(r->pager, r);
 
 	if (r->file != 0) unref_file(r->file);
@@ -656,7 +720,7 @@ bool munmap(process_t *proc, void* addr) {
 }
 
 void dbg_dump_proc_memmap(process_t *proc) {
-	//WARNING not thread safe
+	mutex_lock(&proc->lock);
 
 	dbg_printf("/ Region map for process %d\n", proc->pid);
 
@@ -671,6 +735,8 @@ void dbg_dump_proc_memmap(process_t *proc) {
 	}
 	
 	dbg_printf("\\ ----\n");
+
+	mutex_unlock(&proc->lock);
 }
 
 // =============================== //