From cef1b57d4a97f15e9858804e368f7928b579b88e Mon Sep 17 00:00:00 2001 From: Frans Kaashoek Date: Mon, 15 Aug 2022 19:02:19 -0400 Subject: [PATCH] Compile user binary to map text without W and data without X Use the flags in elf header to set vm permissions Modify pgbug() so that usertests text segment is without W Add test to check app cannot write text segment --- Makefile | 2 +- kernel/exec.c | 27 ++++++++++++++++++++------- user/usertests.c | 31 ++++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 6f0f40e..d7de428 100644 --- a/Makefile +++ b/Makefile @@ -90,7 +90,7 @@ tags: $(OBJS) _init ULIB = $U/ulib.o $U/usys.o $U/printf.o $U/umalloc.o _%: %.o $(ULIB) - $(LD) $(LDFLAGS) -N -e _main -Ttext 0 -o $@ $^ + $(LD) $(LDFLAGS) -verbose -e _main -Ttext 0 -o $@ $^ $(OBJDUMP) -S $@ > $*.asm $(OBJDUMP) -t $@ | sed '1,/SYMBOL TABLE/d; s/ .* / /; /^$$/d' > $*.sym diff --git a/kernel/exec.c b/kernel/exec.c index 3c468e1..aee9345 100644 --- a/kernel/exec.c +++ b/kernel/exec.c @@ -7,7 +7,17 @@ #include "defs.h" #include "elf.h" -static int loadseg(pde_t *pgdir, uint64 addr, struct inode *ip, uint offset, uint sz); +static int loadseg(pde_t *, uint64, uint, struct inode *, uint, uint); + +int flags2perm(int flags) +{ + int perm = 0; + if(flags & 0x1) + perm = PTE_X; + if(flags & 0x2) + perm |= PTE_W; + return perm; +} int exec(char *path, char **argv) @@ -32,6 +42,7 @@ exec(char *path, char **argv) // Check ELF header if(readi(ip, 0, (uint64)&elf, 0, sizeof(elf)) != sizeof(elf)) goto bad; + if(elf.magic != ELF_MAGIC) goto bad; @@ -48,13 +59,15 @@ exec(char *path, char **argv) goto bad; if(ph.vaddr + ph.memsz < ph.vaddr) goto bad; + if(ph.align != PGSIZE) + goto bad; + uint64 e = PGROUNDUP(ph.vaddr + ph.memsz); uint64 sz1; - if((sz1 = uvmalloc(pagetable, sz, ph.vaddr + ph.memsz, PTE_X|PTE_W)) == 0) + if((sz1 = uvmalloc(pagetable, sz, e, flags2perm(ph.flags))) == 0) goto bad; sz = sz1; - if((ph.vaddr % PGSIZE) != 0) - goto bad; - if(loadseg(pagetable, ph.vaddr, ip, ph.off, ph.filesz) < 0) + uint64 s = PGROUNDDOWN(ph.vaddr); + if(loadseg(pagetable, s, ph.vaddr - s, ip, ph.off, ph.filesz) < 0) goto bad; } iunlockput(ip); @@ -134,7 +147,7 @@ exec(char *path, char **argv) // and the pages from va to va+sz must already be mapped. // Returns 0 on success, -1 on failure. static int -loadseg(pagetable_t pagetable, uint64 va, struct inode *ip, uint offset, uint sz) +loadseg(pagetable_t pagetable, uint64 va, uint poff, struct inode *ip, uint offset, uint sz) { uint i, n; uint64 pa; @@ -147,7 +160,7 @@ loadseg(pagetable_t pagetable, uint64 va, struct inode *ip, uint offset, uint sz n = sz - i; else n = PGSIZE; - if(readi(ip, 0, (uint64)pa, offset+i, n) != n) + if(readi(ip, 0, (uint64)pa+poff, offset+i, n) != n) return -1; } diff --git a/user/usertests.c b/user/usertests.c index aec19dd..0a84ef9 100644 --- a/user/usertests.c +++ b/user/usertests.c @@ -2508,17 +2508,40 @@ stacktest(char *s) exit(xstatus); } +// check that writes to text segment fault +void +texttest(char *s) +{ + int pid; + int xstatus; + + pid = fork(); + if(pid == 0) { + volatile int *addr = (int *) 0; + *addr = 10; + exit(1); + } else if(pid < 0){ + printf("%s: fork failed\n", s); + exit(1); + } + wait(&xstatus); + if(xstatus == -1) // kernel killed child? + exit(0); + else + exit(xstatus); +} + // regression test. copyin(), copyout(), and copyinstr() used to cast // the virtual page address to uint, which (with certain wild system // call arguments) resulted in a kernel page faults. +void *big = (void*) 0xeaeb0b5b00002f5e; void pgbug(char *s) { char *argv[1]; argv[0] = 0; - exec((char*)0xeaeb0b5b00002f5e, argv); - - pipe((int*)0xeaeb0b5b00002f5e); + exec(big, argv); + pipe(big); exit(0); } @@ -2607,6 +2630,7 @@ sbrklast(char *s) exit(1); } + // does sbrk handle signed int32 wrap-around with // negative arguments? void @@ -2617,6 +2641,7 @@ sbrk8000(char *s) *(top-1) = *(top-1) + 1; } + // regression test. does write() with an invalid buffer pointer cause // a block to be allocated for a file that is then not freed when the // file is deleted? if the kernel has this bug, it will panic: balloc: