diff --git a/kernel/defs.h b/kernel/defs.h index 2d8f85b..2b082a3 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -193,7 +193,7 @@ uint64 walkaddr(pagetable_t, uint64); int copyout(pagetable_t, uint64, char *, uint64); int copyin(pagetable_t, char *, uint64, uint64); int copyinstr(pagetable_t, char *, uint64, uint64); -char* mapkstack(uint64); +void kmap(uint64, uint64, uint64, int); uint64 kernelpa(uint64); void clearpteu(pagetable_t, uint64); diff --git a/kernel/memlayout.h b/kernel/memlayout.h index c15e398..ace0464 100644 --- a/kernel/memlayout.h +++ b/kernel/memlayout.h @@ -53,6 +53,9 @@ // map the trampoline page to the highest address, // in both user and kernel space. #define TRAMPOLINE (MAXVA - PGSIZE) + +// map kernel stacks beneath the trampoline, +// each surrounded by invalid guard pages. #define KSTACK(p) (TRAMPOLINE - ((p)+1)* 2*PGSIZE) // User memory layout. diff --git a/kernel/proc.c b/kernel/proc.c index cebef1a..5ce31c7 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -28,12 +28,18 @@ procinit(void) initlock(&pid_lock, "nextpid"); for(p = proc; p < &proc[NPROC]; p++) { initlock(&p->lock, "proc"); - // Allocate a page for the kernel stack. - uint64 kstack = KSTACK((int) (p - proc)); - if((p->kstack = mapkstack(kstack)) == 0) { - panic("procinit"); - } + + // Allocate a page for the process's kernel stack. + // Map it high in memory, followed by an invalid + // guard page. + char *pa = kalloc(); + if(pa == 0) + panic("kalloc"); + uint64 va = KSTACK((int) (p - proc)); + kmap(va, (uint64)pa, PGSIZE, PTE_R | PTE_W); + p->kstack = va; } + kvminithart(); } // Must be called with interrupts disabled, @@ -113,7 +119,7 @@ found: // which returns to user space. memset(&p->context, 0, sizeof p->context); p->context.ra = (uint64)forkret; - p->context.sp = (uint64)p->kstack + PGSIZE; + p->context.sp = p->kstack + PGSIZE; return p; } diff --git a/kernel/proc.h b/kernel/proc.h index 8f94d30..0b358aa 100644 --- a/kernel/proc.h +++ b/kernel/proc.h @@ -96,7 +96,7 @@ struct proc { int pid; // Process ID // these are private to the process, so p->lock need not be held. - char *kstack; // Bottom of kernel stack for this process + uint64 kstack; // Bottom of kernel stack for this process uint64 sz; // Size of process memory (bytes) pagetable_t pagetable; // Page table struct trapframe *tf; // data page for trampoline.S diff --git a/kernel/trap.c b/kernel/trap.c index 1af7c00..a41301a 100644 --- a/kernel/trap.c +++ b/kernel/trap.c @@ -101,7 +101,7 @@ usertrapret(void) // set up values that trampoline.S will need when // the process next re-enters the kernel. p->tf->kernel_satp = r_satp(); - p->tf->kernel_sp = (uint64)p->kstack + PGSIZE; + p->tf->kernel_sp = p->kstack + PGSIZE; p->tf->kernel_trap = (uint64)usertrap; p->tf->kernel_hartid = r_tp(); diff --git a/kernel/virtio_disk.c b/kernel/virtio_disk.c index 6620037..3406478 100644 --- a/kernel/virtio_disk.c +++ b/kernel/virtio_disk.c @@ -199,6 +199,8 @@ virtio_disk_rw(struct buf *b) buf0.reserved = 0; buf0.sector = sector; + // buf0 is on a kernel stack, which is not direct mapped, + // thus the call to kernelpa(). desc[idx[0]].addr = (uint64) kernelpa((uint64) &buf0); desc[idx[0]].len = sizeof(buf0); desc[idx[0]].flags = VRING_DESC_F_NEXT; diff --git a/kernel/vm.c b/kernel/vm.c index 7dc84ba..b327b94 100644 --- a/kernel/vm.c +++ b/kernel/vm.c @@ -27,33 +27,26 @@ kvminit() memset(kernel_pagetable, 0, PGSIZE); // uart registers - mappages(kernel_pagetable, UART0, PGSIZE, - UART0, PTE_R | PTE_W); + kmap(UART0, UART0, PGSIZE, PTE_R | PTE_W); // virtio mmio disk interface - mappages(kernel_pagetable, VIRTIO0, PGSIZE, - VIRTIO0, PTE_R | PTE_W); + kmap(VIRTIO0, VIRTIO0, PGSIZE, PTE_R | PTE_W); // CLINT - mappages(kernel_pagetable, CLINT, 0x10000, - CLINT, PTE_R | PTE_W); + kmap(CLINT, CLINT, 0x10000, PTE_R | PTE_W); // PLIC - mappages(kernel_pagetable, PLIC, 0x4000000, - PLIC, PTE_R | PTE_W); + kmap(PLIC, PLIC, 0x400000, PTE_R | PTE_W); // map kernel text executable and read-only. - mappages(kernel_pagetable, KERNBASE, (uint64)etext-KERNBASE, - KERNBASE, PTE_R | PTE_X); + kmap(KERNBASE, KERNBASE, (uint64)etext-KERNBASE, PTE_R | PTE_X); // map kernel data and the physical RAM we'll make use of. - mappages(kernel_pagetable, (uint64)etext, PHYSTOP-(uint64)etext, - (uint64)etext, PTE_R | PTE_W); + kmap((uint64)etext, (uint64)etext, PHYSTOP-(uint64)etext, PTE_R | PTE_W); // map the trampoline for trap entry/exit to // the highest virtual address in the kernel. - mappages(kernel_pagetable, TRAMPOLINE, PGSIZE, - (uint64)trampout, PTE_R | PTE_X); + kmap(TRAMPOLINE, (uint64)trampout, PGSIZE, PTE_R | PTE_X); } // Switch h/w page table register to the kernel's page table, @@ -117,6 +110,15 @@ walkaddr(pagetable_t pagetable, uint64 va) return pa; } +// add a mapping to the kernel page table. +// only used when booting. +// does not flush TLB or enable paging. +void +kmap(uint64 va, uint64 pa, uint64 sz, int perm) +{ + if(mappages(kernel_pagetable, va, sz, pa, perm) != 0) + panic("kmap"); +} // Create PTEs for virtual addresses starting at va that refer to // physical addresses starting at pa. va and size might not @@ -405,25 +407,13 @@ copyinstr(pagetable_t pagetable, char *dst, uint64 srcva, uint64 max) } } -char * -mapkstack(uint64 kstack) -{ - char *k = kalloc(); - if(k == 0) { - return 0; - } - if (mappages(kernel_pagetable, kstack, PGSIZE, - (uint64) k, PTE_R | PTE_W) == 0) { - kvminithart(); - return (char *) kstack; - } - kfree(k); - return 0; -} - -// assumes va is page aligned +// translate a kernel virtual address to +// a physical address. only needed for +// addresses on the stack. +// assumes va is page aligned. uint64 -kernelpa(uint64 va) { +kernelpa(uint64 va) +{ uint64 off = va % PGSIZE; pte_t *pte; uint64 pa; @@ -437,8 +427,11 @@ kernelpa(uint64 va) { return pa+off; } +// mark a PTE invalid for user access. +// used by exec for the user stack guard page. void -clearpteu(pagetable_t pagetable, uint64 va) { +clearpteu(pagetable_t pagetable, uint64 va) +{ pte_t *pte; pte = walk(pagetable, va, 0); diff --git a/user/usertests.c b/user/usertests.c index 98a5838..42065f4 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -1884,26 +1884,30 @@ rand() return randstate; } +// check that there's an invalid page beneath +// the user stack, to catch stack overflow. void stacktest() { int pid; + int ppid = getpid(); - printf(1, "stack test\n"); + printf(1, "stack guard test\n"); pid = fork(); if(pid == 0) { char *sp = (char *) r_sp(); - printf(1, "%p\n", sp); sp -= 4096; + // the *sp should cause a trap. printf(1, "stacktest: read below stack %p\n", *sp); printf(1, "stacktest: test FAILED\n"); + kill(ppid); exit(); } else if(pid < 0){ printf (1, "fork failed\n"); exit(); } wait(); - printf(1, "stack test done\n"); + printf(1, "stack guard test ok\n"); } int