From 5808a2564db6a55d5f630fe17f1dddbf6c50dfbf Mon Sep 17 00:00:00 2001 From: Ali Mashtizadeh Date: Mon, 19 Jan 2015 17:57:20 -0800 Subject: [PATCH] Implemented CopyStrIn/Out to fix bugginess of using CopyIn/Out wrapping passed a mapped page. --- include/errno.h | 1 + sys/amd64/support.S | 24 ++++++++++++++++++++++++ sys/amd64/trap.c | 13 +++++++++++++ sys/include/thread.h | 2 ++ sys/kern/copy.c | 43 +++++++++++++++++++++++++++++++++++++++++++ sys/kern/syscall.c | 9 +++------ 6 files changed, 86 insertions(+), 6 deletions(-) diff --git a/include/errno.h b/include/errno.h index 41c6d4b..e6f26a1 100644 --- a/include/errno.h +++ b/include/errno.h @@ -9,6 +9,7 @@ #define ENOMEM 0xBAD5 #define ENOENT 0xBAD6 #define ENOTDIR 0xBAD7 +#define ENAMETOOLONG 0xBAD8 #endif /* __ERRNO_H__ */ diff --git a/sys/amd64/support.S b/sys/amd64/support.S index 6380ded..8eb612f 100644 --- a/sys/amd64/support.S +++ b/sys/amd64/support.S @@ -21,3 +21,27 @@ copy_unsafe_fault: retq FUNC_END(copy_unsafe) +// copystr_unsafe(to, from, len) +FUNC_BEGIN(copystr_unsafe) + movq %rdx, %rcx +1: + decq %rcx + jz copystr_unsafe_toolong + lodsb + stosb + orb %al, %al + jnz 1b +.globl copystr_unsafe_done +copystr_unsafe_done: + xorq %rax, %rax + retq +.globl copystr_unsafe_fault +copystr_unsafe_fault: + movq $EFAULT, %rax + retq +.globl copystr_unsafe_toolong + copystr_unsafe_toolong: + movq $ENAMETOOLONG, %rax + retq +FUNC_END(copystr_unsafe) + diff --git a/sys/amd64/trap.c b/sys/amd64/trap.c index 999c2b4..9772ac8 100644 --- a/sys/amd64/trap.c +++ b/sys/amd64/trap.c @@ -115,6 +115,10 @@ extern int copy_unsafe(void *to, void *from, uintptr_t len); extern void copy_unsafe_done(void); extern void copy_unsafe_fault(void); +extern int copystr_unsafe(void *to, void *from, uintptr_t len); +extern void copystr_unsafe_done(void); +extern void copystr_unsafe_fault(void); + void trap_entry(TrapFrame *tf) { @@ -140,6 +144,15 @@ trap_entry(TrapFrame *tf) return; } + // User IO + if ((tf->vector == T_PF) && + (tf->rip >= (uint64_t)©str_unsafe) && + (tf->rip <= (uint64_t)©str_unsafe_done)) { + kprintf("Faulted in copystr_unsafe\n"); + tf->rip = (uint64_t)©str_unsafe_fault; + return; + } + // Halt on kernel errors if (tf->vector <= T_CPU_LAST) { diff --git a/sys/include/thread.h b/sys/include/thread.h index d5ea482..2685dd6 100644 --- a/sys/include/thread.h +++ b/sys/include/thread.h @@ -107,6 +107,8 @@ Handle *Handle_Lookup(Process *proc, uint64_t fd); // CopyIn/CopyOut Functions int CopyIn(uintptr_t fromuser, void *tokernel, uintptr_t len); int CopyOut(void *fromkernel, uintptr_t touser, uintptr_t len); +int CopyStrIn(uintptr_t fromuser, void *tokernel, uintptr_t len); +int CopyStrOut(void *fromkernel, uintptr_t touser, uintptr_t len); #endif /* __SYS_THREAD_H__ */ diff --git a/sys/kern/copy.c b/sys/kern/copy.c index 62da717..454fc88 100644 --- a/sys/kern/copy.c +++ b/sys/kern/copy.c @@ -11,6 +11,7 @@ #include extern int copy_unsafe(void *to_addr, void *from_addr, uintptr_t len); +extern int copystr_unsafe(void *to_addr, void *from_addr, uintptr_t len); int CopyIn(uintptr_t fromuser, void *tokernel, uintptr_t len) @@ -54,3 +55,45 @@ CopyOut(void *fromkernel, uintptr_t touser, uintptr_t len) return copy_unsafe((void *)touser, fromkernel, len); } +int +CopyStrIn(uintptr_t fromuser, void *tokernel, uintptr_t len) +{ + if (len == 0) + return 0; + + // Kernel space + if (fromuser >= MEM_USERSPACE_TOP) { + kprintf("CopyStrIn: address exceeds userspace top\n"); + return EFAULT; + } + + // Wrap around + if (len > (MEM_USERSPACE_TOP - fromuser)) { + kprintf("CopyStrIn: length exceeds userspace top\n"); + return EFAULT; + } + + return copystr_unsafe(tokernel, (void *)fromuser, len); +} + +int +CopyStrOut(void *fromkernel, uintptr_t touser, uintptr_t len) +{ + if (len == 0) + return 0; + + // Kernel space + if (touser >= MEM_USERSPACE_TOP) { + kprintf("CopyStrOut: address exceeds userspace top\n"); + return EFAULT; + } + + // Wrap around + if (len > (MEM_USERSPACE_TOP - touser)) { + kprintf("CopyStrOut: length exceeds userspace top\n"); + return EFAULT; + } + + return copystr_unsafe((void *)touser, fromkernel, len); +} + diff --git a/sys/kern/syscall.c b/sys/kern/syscall.c index e86eda7..d85807b 100644 --- a/sys/kern/syscall.c +++ b/sys/kern/syscall.c @@ -55,8 +55,7 @@ Syscall_Spawn(uint64_t user_path) Process *proc; Thread *thr; - // XXX: Use CopyInStr - status = CopyIn(user_path, &path, sizeof(path)); + status = CopyStrIn(user_path, &path, sizeof(path)); if (status != 0) return status; @@ -200,8 +199,7 @@ Syscall_Open(uint64_t user_path, uint64_t flags) int status; char path[256]; - // XXX: Use CopyInStr - status = CopyIn(user_path, &path, sizeof(path)); + status = CopyStrIn(user_path, &path, sizeof(path)); if (status != 0) return status; @@ -241,8 +239,7 @@ Syscall_Stat(uint64_t user_path, uint64_t user_stat) char path[256]; struct stat sb; - // XXX: Use CopyInStr - status = CopyIn(user_path, &path, sizeof(path)); + status = CopyStrIn(user_path, &path, sizeof(path)); if (status != 0) { return status; }