summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Morris <[email protected]>2010-08-06 11:12:18 -0400
committerRobert Morris <[email protected]>2010-08-06 11:12:18 -0400
commitc4cc10da7ef6d65f0f654445e0af35b8309f16c2 (patch)
tree771c4791115f945fc86ea9eadc350bb22c518535
parent1afc9d3fcaa7c5992659bb8b69f639b746dda2bc (diff)
downloadxv6-labs-c4cc10da7ef6d65f0f654445e0af35b8309f16c2.tar.gz
xv6-labs-c4cc10da7ef6d65f0f654445e0af35b8309f16c2.tar.bz2
xv6-labs-c4cc10da7ef6d65f0f654445e0af35b8309f16c2.zip
fix corner cases in exec of ELF
put an invalid page below the stack have fork() handle invalid pages
-rw-r--r--defs.h3
-rw-r--r--exec.c7
-rw-r--r--kalloc.c5
-rw-r--r--mmu.h1
-rw-r--r--proc.c10
-rw-r--r--proc.h5
-rw-r--r--usertests.c24
-rw-r--r--vm.c66
8 files changed, 84 insertions, 37 deletions
diff --git a/defs.h b/defs.h
index 4a63154..b691099 100644
--- a/defs.h
+++ b/defs.h
@@ -163,7 +163,8 @@ void freevm(pde_t*);
void inituvm(pde_t*, char*, char*, uint);
int loaduvm(pde_t*, char*, struct inode *ip, uint, uint);
pde_t* copyuvm(pde_t*,uint);
-void loadvm(struct proc*);
+void switchuvm(struct proc*);
+void switchkvm();
// number of elements in fixed-size array
#define NELEM(x) (sizeof(x)/sizeof((x)[0]))
diff --git a/exec.c b/exec.c
index 8a92e99..4f11695 100644
--- a/exec.c
+++ b/exec.c
@@ -43,13 +43,16 @@ exec(char *path, char **argv)
goto bad;
if (!allocuvm(pgdir, (char *)ph.va, ph.memsz))
goto bad;
- sz += PGROUNDUP(ph.memsz);
+ if(ph.va + ph.memsz > sz)
+ sz = ph.va + ph.memsz;
if (!loaduvm(pgdir, (char *)ph.va, ip, ph.offset, ph.filesz))
goto bad;
}
iunlockput(ip);
// 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);
@@ -95,7 +98,7 @@ exec(char *path, char **argv)
proc->tf->eip = elf.entry; // main
proc->tf->esp = sp;
- loadvm(proc);
+ switchuvm(proc);
freevm(oldpgdir);
diff --git a/kalloc.c b/kalloc.c
index 5661105..7695006 100644
--- a/kalloc.c
+++ b/kalloc.c
@@ -1,9 +1,8 @@
// Physical memory allocator, intended to allocate
-// memory for user processes. Allocates in 4096-byte "pages".
+// memory for user processes. Allocates in 4096-byte pages.
// Free list is kept sorted and combines adjacent pages into
// long runs, to make it easier to allocate big segments.
-// One reason the page size is 4k is that the x86 segment size
-// granularity is 4k.
+// This combining is not useful now that xv6 uses paging.
#include "types.h"
#include "defs.h"
diff --git a/mmu.h b/mmu.h
index 76a6f1b..f4fc732 100644
--- a/mmu.h
+++ b/mmu.h
@@ -129,7 +129,6 @@ struct segdesc {
#define PTE_ADDR(pte) ((uint) (pte) & ~0xFFF)
typedef uint pte_t;
-extern pde_t *kpgdir;
// Control Register flags
#define CR0_PE 0x00000001 // Protection Enable
diff --git a/proc.c b/proc.c
index dd6f27e..f799a4d 100644
--- a/proc.c
+++ b/proc.c
@@ -145,7 +145,7 @@ growproc(int n)
if (!allocuvm(proc->pgdir, (char *)proc->sz, n))
return -1;
proc->sz += n;
- loadvm(proc);
+ switchuvm(proc);
return 0;
}
@@ -214,9 +214,10 @@ scheduler(void)
// to release ptable.lock and then reacquire it
// before jumping back to us.
proc = p;
- loadvm(p);
+ switchuvm(p);
p->state = RUNNING;
swtch(&cpu->scheduler, proc->context);
+ switchkvm();
// Process is done running for now.
// It should have changed its p->state before coming back.
@@ -242,7 +243,6 @@ sched(void)
panic("sched running");
if(readeflags()&FL_IF)
panic("sched interruptible");
- lcr3(PADDR(kpgdir)); // Switch to the kernel page table
intena = cpu->intena;
swtch(&proc->context, cpu->scheduler);
cpu->intena = intena;
@@ -414,8 +414,8 @@ wait(void)
// Found one.
pid = p->pid;
kfree(p->kstack, KSTACKSIZE);
- p->kstack = 0;
- freevm(p->pgdir);
+ p->kstack = 0;
+ freevm(p->pgdir);
p->state = UNUSED;
p->pid = 0;
p->parent = 0;
diff --git a/proc.h b/proc.h
index 5e5d031..7d97dfa 100644
--- a/proc.h
+++ b/proc.h
@@ -16,7 +16,7 @@
// Contexts are stored at the bottom of the stack they
// describe; the stack pointer is the address of the context.
// The layout of the context matches the layout of the stack in swtch.S
-// at "Switch stacks" comment. Switch itself doesn't save eip explicitly,
+// at the "Switch stacks" comment. Switch doesn't save eip explicitly,
// but it is on the stack and allocproc() manipulates it.
struct context {
uint edi;
@@ -31,7 +31,7 @@ enum procstate { UNUSED, EMBRYO, SLEEPING, RUNNABLE, RUNNING, ZOMBIE };
// Per-process state
struct proc {
uint sz; // Size of process memory (bytes)
- pde_t* pgdir; // linear address of proc's pgdir
+ pde_t* pgdir; // Linear address of proc's pgdir
char *kstack; // Bottom of kernel stack for this process
enum procstate state; // Process state
volatile int pid; // Process ID
@@ -48,6 +48,7 @@ struct proc {
// Process memory is laid out contiguously, low addresses first:
// text
// original data and bss
+// invalid page
// fixed-size stack
// expandable heap
diff --git a/usertests.c b/usertests.c
index 2bd21ba..247cc95 100644
--- a/usertests.c
+++ b/usertests.c
@@ -1261,6 +1261,29 @@ sbrktest(void)
printf(stdout, "sbrk test OK\n");
}
+void
+stacktest(void)
+{
+ 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();
+ }
+ 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");
+}
+
int
main(int argc, char *argv[])
{
@@ -1272,6 +1295,7 @@ main(int argc, char *argv[])
}
close(open("usertests.ran", O_CREATE));
+ stacktest();
sbrktest();
opentest();
diff --git a/vm.c b/vm.c
index 9ea5e92..6914dd3 100644
--- a/vm.c
+++ b/vm.c
@@ -8,13 +8,20 @@
// The mappings from logical to linear are one to one (i.e.,
// segmentation doesn't do anything).
-// The mapping from linear to physical are one to one for the kernel.
-// The mappings for the kernel include all of physical memory (until
-// PHYSTOP), including the I/O hole, and the top of physical address
-// space, where additional devices are located.
-// The kernel itself is linked to be at 1MB, and its physical memory
-// is also at 1MB.
-// Physical memory for user programs is allocated from physical memory
+// There is one page table per process, plus one that's used
+// when a CPU is not running any process (kpgdir).
+// A user process uses the same page table as the kernel; the
+// page protection bits prevent it from using anything other
+// than its memory.
+//
+// setupkvm() and exec() set up every page table like this:
+// 0..640K : user memory (text, data, stack, heap)
+// 640K..1M : mapped direct (for IO space)
+// 1M..kernend : mapped direct (for the kernel's text and data)
+// kernend..PHYSTOP : mapped direct (kernel heap and user pages)
+// 0xfe000000..0 : mapped direct (devices such as ioapic)
+//
+// The kernel allocates memory for its heap and for user memory
// between kernend and the end of physical memory (PHYSTOP).
// The virtual address space of each user program includes the kernel
// (which is inaccessible in user mode). The user program addresses
@@ -31,7 +38,7 @@ static uint kerndata;
static uint kerndsz;
static uint kernend;
static uint freesz;
-pde_t *kpgdir; // One kernel page table for scheduler procs
+static pde_t *kpgdir; // for use in scheduler()
// return the address of the PTE in page table pgdir
// that corresponds to linear address va. if create!=0,
@@ -114,9 +121,9 @@ ksegment(void)
proc = 0;
}
-// Setup address space and current process task state.
+// Switch h/w page table and TSS registers to point to process p.
void
-loadvm(struct proc *p)
+switchuvm(struct proc *p)
{
pushcli();
@@ -128,14 +135,21 @@ loadvm(struct proc *p)
ltr(SEG_TSS << 3);
if (p->pgdir == 0)
- panic("loadvm: no pgdir\n");
+ panic("switchuvm: no pgdir\n");
lcr3(PADDR(p->pgdir)); // switch to new address space
popcli();
}
-// Setup kernel part of a page table. Linear adresses map one-to-one
-// on physical addresses.
+// Switch h/w page table register to the kernel-only page table, for when
+// no process is running.
+void
+switchkvm()
+{
+ lcr3(PADDR(kpgdir)); // Switch to the kernel page table
+}
+
+// Set up kernel part of a page table.
pde_t*
setupkvm(void)
{
@@ -163,6 +177,10 @@ setupkvm(void)
return pgdir;
}
+// return the physical address that a given user address
+// maps to. the result is also a kernel logical address,
+// since the kernel maps the physical memory allocated to user
+// processes directly.
char*
uva2ka(pde_t *pgdir, char *uva)
{
@@ -266,6 +284,8 @@ inituvm(pde_t *pgdir, char *addr, char *init, uint sz)
}
}
+// given a parent process's page table, create a copy
+// of it for a child.
pde_t*
copyuvm(pde_t *pgdir, uint sz)
{
@@ -278,17 +298,20 @@ copyuvm(pde_t *pgdir, uint sz)
for (i = 0; i < sz; i += PGSIZE) {
if (!(pte = walkpgdir(pgdir, (void *)i, 0)))
panic("copyuvm: pte should exist\n");
- pa = PTE_ADDR(*pte);
- if (!(mem = kalloc(PGSIZE)))
- return 0;
- memmove(mem, (char *)pa, PGSIZE);
- if (!mappages(d, (void *)i, PGSIZE, PADDR(mem), PTE_W|PTE_U))
- return 0;
+ if(*pte & PTE_P){
+ pa = PTE_ADDR(*pte);
+ if (!(mem = kalloc(PGSIZE)))
+ return 0;
+ memmove(mem, (char *)pa, PGSIZE);
+ if (!mappages(d, (void *)i, PGSIZE, PADDR(mem), PTE_W|PTE_U))
+ return 0;
+ }
}
return d;
}
-// Gather about physical memory layout. Called once during boot.
+// Gather information about physical memory layout.
+// Called once during boot.
void
pminit(void)
{
@@ -307,9 +330,6 @@ pminit(void)
kerndsz = ph[1].memsz;
freesz = PHYSTOP - kernend;
- cprintf("kerntext@0x%x(sz=0x%x), kerndata@0x%x(sz=0x%x), kernend 0x%x freesz = 0x%x\n",
- kerntext, kerntsz, kerndata, kerndsz, kernend, freesz);
-
kinit((char *)kernend, freesz);
}