From b1d41d678888fd1a51e4844ab583f7c47f9fb218 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Wed, 1 Sep 2010 16:46:37 -0400 Subject: [PATCH] Remove the stack guard page. Processes are now contiguous from 0 to proc->sz, which means our syscall argument validation is correct. Add a pointer validation test and remove the stack test, which tested for the guard page. --- exec.c | 1 - syscall.c | 5 +---- usertests.c | 60 ++++++++++++++++++++++++++++++++++++----------------- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/exec.c b/exec.c index 39530aa..c518c04 100644 --- a/exec.c +++ b/exec.c @@ -52,7 +52,6 @@ exec(char *path, char **argv) // Allocate and initialize stack at sz sz = PGROUNDUP(sz); - sz += PGSIZE; // leave an invalid page if(!allocuvm(pgdir, (char *)sz, PGSIZE)) goto bad; mem = uva2ka(pgdir, (char *)sz); diff --git a/syscall.c b/syscall.c index 9296cff..f8aeae7 100644 --- a/syscall.c +++ b/syscall.c @@ -22,8 +22,6 @@ fetchint(struct proc *p, uint addr, int *ip) return 0; } -// XXX should we copy the string? - // Fetch the nul-terminated string at addr from process p. // Doesn't actually copy the string - just sets *pp to point at it. // Returns length of string, not including nul. @@ -62,8 +60,7 @@ argptr(int n, char **pp, int size) return -1; if((uint)i >= proc->sz || (uint)i+size >= proc->sz) return -1; - // *pp = proc->mem + i; // XXXXX - *pp = (char *) i; // XXXXX + *pp = (char *) i; return 0; } diff --git a/usertests.c b/usertests.c index 946565f..77495bf 100644 --- a/usertests.c +++ b/usertests.c @@ -3,6 +3,8 @@ #include "user.h" #include "fs.h" #include "fcntl.h" +#include "syscall.h" +#include "traps.h" char buf[2048]; char name[3]; @@ -1375,26 +1377,46 @@ sbrktest(void) } void -stacktest(void) +validateint(int *p) { - printf(stdout, "stack test\n"); - char dummy = 1; - char *p = &dummy; - int ppid = getpid(); - int pid = fork(); - if(pid < 0){ - printf(stdout, "fork failed\n"); - exit(); + int res; + asm("mov %%esp, %%ebx\n\t" + "mov %3, %%esp\n\t" + "int %2\n\t" + "mov %%ebx, %%esp" : + "=a" (res) : + "a" (SYS_sleep), "n" (T_SYSCALL), "c" (p) : + "ebx"); +} + +void +validatetest(void) +{ + int hi = 1100*1024; + + printf(stdout, "validate test\n"); + + uint p; + for (p = 0; p <= (uint)hi; p += 4096) { + int pid; + if ((pid = fork()) == 0) { + // try to crash the kernel by passing in a badly placed integer + validateint((int*)p); + exit(); + } + sleep(0); + sleep(0); + kill(pid); + wait(); + + // try to crash the kernel by passing in a bad string pointer + if (link("nosuchfile", (char*)p) != -1) { + printf(stdout, "link should not succeed\n"); + exit(); + } } - if(pid == 0){ - // should cause a trap: - p[-4096] = 'z'; - kill(ppid); - printf(stdout, "stack test failed: page before stack was writeable\n"); - exit(); - } - wait(); - printf(stdout, "stack test OK\n"); + + printf(stdout, "validate ok\n"); } int @@ -1408,8 +1430,8 @@ main(int argc, char *argv[]) } close(open("usertests.ran", O_CREATE)); - stacktest(); sbrktest(); + validatetest(); opentest(); writetest();