From 4655d42e3b65f906eae8c815fb78331790f6e423 Mon Sep 17 00:00:00 2001 From: Robert Morris Date: Mon, 27 Sep 2010 16:14:33 -0400 Subject: [PATCH] copyout() copies data to a va in a pagetable, for exec() &c usertest that passes too many arguments, break exec --- defs.h | 1 + exec.c | 85 ++++++++++++++++++++++++++++++++++------------------- param.h | 2 ++ sysfile.c | 2 +- usertests.c | 10 +++---- vm.c | 32 +++++++++++++++++--- 6 files changed, 91 insertions(+), 41 deletions(-) diff --git a/defs.h b/defs.h index 7408d4d..d08609d 100644 --- a/defs.h +++ b/defs.h @@ -165,6 +165,7 @@ int loaduvm(pde_t*, char*, struct inode *, uint, uint); pde_t* copyuvm(pde_t*,uint); void switchuvm(struct proc*); void switchkvm(); +int copyout(pde_t *pgdir, uint va, void *buf, uint len); // number of elements in fixed-size array #define NELEM(x) (sizeof(x)/sizeof((x)[0])) diff --git a/exec.c b/exec.c index 0a9ca59..2e2ced4 100644 --- a/exec.c +++ b/exec.c @@ -9,16 +9,13 @@ int exec(char *path, char **argv) { - char *mem, *s, *last; - int i, argc, arglen, len, off; - uint sz, sp, spbottom, argp; + char *s, *last; + int i, off; + uint sz = 0; struct elfhdr elf; - struct inode *ip; + struct inode *ip = 0; struct proghdr ph; - pde_t *pgdir, *oldpgdir; - - pgdir = 0; - sz = 0; + pde_t *pgdir = 0, *oldpgdir; if((ip = namei(path)) == 0) return -1; @@ -48,40 +45,65 @@ exec(char *path, char **argv) } iunlockput(ip); - // Allocate and initialize stack at sz - sz = spbottom = PGROUNDUP(sz); + // Allocate a one-page stack at the next page boundary + sz = PGROUNDUP(sz); if(!(sz = allocuvm(pgdir, sz, sz + PGSIZE))) goto bad; - mem = uva2ka(pgdir, (char *)spbottom); - arglen = 0; - for(argc=0; argv[argc]; argc++) - arglen += strlen(argv[argc]) + 1; - arglen = (arglen+3) & ~3; + // initialize stack content: - sp = sz; - argp = sz - arglen - 4*(argc+1); + // "argumentN" -- nul-terminated string + // ... + // "argument0" + // 0 -- argv[argc] + // address of argumentN + // ... + // address of argument0 -- argv[0] + // address of address of argument0 -- argv argument to main() + // argc -- argc argument to main() + // ffffffff -- return PC for main() call - // XXX rtm: does the following code work if the - // arguments &c do not fit in one page? + uint sp = sz; - // Copy argv strings and pointers to stack. - *(uint*)(mem+argp-spbottom + 4*argc) = 0; // argv[argc] - for(i=argc-1; i>=0; i--){ - len = strlen(argv[i]) + 1; - sp -= len; - memmove(mem+sp-spbottom, argv[i], len); - *(uint*)(mem+argp-spbottom + 4*i) = sp; // argv[i] + // count arguments + int argc; + for(argc = 0; argv[argc]; argc++) + ; + if(argc >= MAXARG) + goto bad; + + // push strings and remember where they are + uint strings[MAXARG]; + for(i = argc - 1; i >= 0; --i){ + sp -= strlen(argv[i]) + 1; + strings[i] = sp; + copyout(pgdir, sp, argv[i], strlen(argv[i]) + 1); } - // Stack frame for main(argc, argv), below arguments. - sp = argp; + // push 0 for argv[argc] sp -= 4; - *(uint*)(mem+sp-spbottom) = argp; + int zero = 0; + copyout(pgdir, sp, &zero, 4); + + // push argv[] elements + for(i = argc - 1; i >= 0; --i){ + sp -= 4; + copyout(pgdir, sp, &strings[i], 4); + } + + // push argv + uint argvaddr = sp; sp -= 4; - *(uint*)(mem+sp-spbottom) = argc; + copyout(pgdir, sp, &argvaddr, 4); + + // push argc sp -= 4; - *(uint*)(mem+sp-spbottom) = 0xffffffff; // fake return pc + copyout(pgdir, sp, &argc, 4); + + // push 0 in case main returns + sp -= 4; + uint ffffffff = 0xffffffff; + copyout(pgdir, sp, &ffffffff, 4); // Save program name for debugging. for(last=s=path; *s; s++) @@ -103,6 +125,7 @@ exec(char *path, char **argv) return 0; bad: + cprintf("kernel: exec failed\n"); if(pgdir) freevm(pgdir); iunlockput(ip); return -1; diff --git a/param.h b/param.h index 48c3352..70f88e8 100644 --- a/param.h +++ b/param.h @@ -7,4 +7,6 @@ #define NINODE 50 // maximum number of active i-nodes #define NDEV 10 // maximum major device number #define ROOTDEV 1 // device number of file system root disk +#define USERTOP 0xA0000 // end of user address space #define PHYSTOP 0x1000000 // use phys mem up to here as free pool +#define MAXARG 32 // max exec arguments diff --git a/sysfile.c b/sysfile.c index 6b8eef4..0b42920 100644 --- a/sysfile.c +++ b/sysfile.c @@ -344,7 +344,7 @@ sys_chdir(void) int sys_exec(void) { - char *path, *argv[20]; + char *path, *argv[MAXARG]; int i; uint uargv, uarg; diff --git a/usertests.c b/usertests.c index 177ffba..5d1d8ea 100644 --- a/usertests.c +++ b/usertests.c @@ -1445,11 +1445,11 @@ bigargtest(void) ppid = getpid(); pid = fork(); if(pid == 0){ - char *args[100]; + char *args[32]; int i; - for(i = 0; i < 99; i++) - args[i] = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; - args[99] = 0; + for(i = 0; i < 32-1; i++) + args[i] = "bigargs test: failed\n "; + args[32-1] = 0; printf(stdout, "bigarg test\n"); exec("echo", args); printf(stdout, "bigarg test ok\n"); @@ -1472,7 +1472,7 @@ main(int argc, char *argv[]) } close(open("usertests.ran", O_CREATE)); - // bigargtest(); + bigargtest(); bsstest(); sbrktest(); validatetest(); diff --git a/vm.c b/vm.c index cd6b255..551efbf 100644 --- a/vm.c +++ b/vm.c @@ -6,8 +6,6 @@ #include "proc.h" #include "elf.h" -#define USERTOP 0xA0000 - static pde_t *kpgdir; // for use in scheduler() // Set up CPU's kernel segment descriptors. @@ -126,7 +124,7 @@ setupkvm(void) { pde_t *pgdir; extern char etext[]; - char *rwstart = PGROUNDDOWN(etext) - PGSIZE; + char *rwstart = PGROUNDDOWN(etext); uint rwlen = (uint)rwstart - 0x100000; // Allocate page directory @@ -193,7 +191,10 @@ char* uva2ka(pde_t *pgdir, char *uva) { pte_t *pte = walkpgdir(pgdir, uva, 0); - if(pte == 0) return 0; + if((*pte & PTE_P) == 0) + return 0; + if((*pte & PTE_U) == 0) + return 0; uint pa = PTE_ADDR(*pte); return (char *)pa; } @@ -326,3 +327,26 @@ bad: return 0; } +// copy some data to user address va in page table pgdir. +// most useful when pgdir is not the current page table. +// returns 1 if everthing OK, 0 on error. +// uva2ka ensures this only works for PTE_U pages. +int +copyout(pde_t *pgdir, uint va, void *xbuf, uint len) +{ + char *buf = (char *) xbuf; + while(len > 0){ + uint va0 = (uint)PGROUNDDOWN(va); + char *pa0 = uva2ka(pgdir, (char*) va0); + if(pa0 == 0) + return 0; + uint n = PGSIZE - (va - va0); + if(n > len) + n = len; + memmove(pa0 + (va - va0), buf, n); + len -= n; + buf += n; + va = va0 + PGSIZE; + } + return 1; +}