diff --git a/src/threads/thread.h b/src/threads/thread.h index bdfad35..e157ce3 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -145,6 +145,8 @@ struct thread struct hash open_files; /* Hash Table of FD -> Struct File. */ #endif + void *curr_esp; + /* Owned by thread.c. */ unsigned magic; /* Detects stack overflow. */ }; diff --git a/src/userprog/exception.c b/src/userprog/exception.c index 1e3ccc0..e0d5c64 100644 --- a/src/userprog/exception.c +++ b/src/userprog/exception.c @@ -3,16 +3,24 @@ #include #include "userprog/gdt.h" #include "userprog/pagedir.h" +#include "userprog/process.h" #include "threads/interrupt.h" +#include "threads/palloc.h" #include "threads/thread.h" #include "threads/vaddr.h" #include "vm/page.h" +#define MAX_STACK_SIZE (8 * 1024 * 1024) // 8MB +#define MAX_STACK_OFFSET 32 // 32 bytes offset below stack pointer (ESP) + /* Number of page faults processed. */ static long long page_fault_cnt; static void kill (struct intr_frame *); static void page_fault (struct intr_frame *); + +static bool is_valid_stack_access (const void *fault_addr, const void *esp); +static bool grow_stack (void *upage); bool try_fetch_page (void *upage, bool write); /* Registers handlers for interrupts that can be caused by user @@ -149,20 +157,26 @@ page_fault (struct intr_frame *f) write = (f->error_code & PF_W) != 0; user = (f->error_code & PF_U) != 0; - if (!user) + if (!user || !not_present) { f->eip = (void *)f->eax; f->eax = 0xffffffff; return; } - /* If the fault address is in a user page that is not present, then it might - just need to be lazily loaded. So, we check our SPT to see if the page + be just that the stack needs to grow or that it needs to be lazily loaded. + So we attempt to grow the stack. If this does not work, we check our SPT to see if the page is expected to have data loaded in memory. */ void *upage = pg_round_down (fault_addr); if (not_present && is_user_vaddr (upage) && upage != NULL) { + if (is_valid_stack_access (fault_addr, f->esp)) + { + if (grow_stack (upage)) + return; + } + if (try_fetch_page (upage, write)) return; } @@ -178,6 +192,54 @@ page_fault (struct intr_frame *f) kill (f); } +/* Validates whether the fault address is a valid stack access. Access is a + valid stack access under the following two conditions: + 1. The fault address must be within MAX_STACK_OFFSET (32) bytes below + the current stack pointer. (Accounts for both PUSH and PUSHA instructions) + 2. Growing this stack to this address does not cause it to exceed the + MAX_STACK_SIZE (8MB) limit. + + Returns true if both conditions are met, false otherwise. + + Pre: fault_addr is a valid user virtual address (so also not NULL). */ +static bool +is_valid_stack_access (const void *fault_addr, const void *esp) +{ + uint32_t new_stack_size = PHYS_BASE - pg_round_down (fault_addr); + + uint32_t *lowest_valid_push_addr = (uint32_t *)esp - MAX_STACK_OFFSET; + bool is_within_push_range = (uint32_t *)fault_addr >= lowest_valid_push_addr; + + return is_within_push_range && new_stack_size <= MAX_STACK_SIZE; +} + +/* Attempts to grow the stack by allocating and mapping a new page. + This involves: + 1. Allocating a zeroed page from the user pool + 2. Installing it into the page table with write permissions + + Returns true if the stack was successfully grown, false if either + allocation or installation fails. + + Pre: upage is a valid page-aligned address (so also not NULL). */ +static bool +grow_stack (void *upage) +{ + /* Allocate new page for stack */ + void *kpage = palloc_get_page (PAL_USER | PAL_ZERO); + if (kpage == NULL) + return false; + + /* Install the page into user page table */ + if (!install_page (upage, kpage, true)) + { + palloc_free_page (kpage); + return false; + } + + return true; +} + bool try_fetch_page (void *upage, bool write) { diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index c6d4259..4bc34ca 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,5 +1,4 @@ #include "userprog/syscall.h" -#include "userprog/exception.h" #include "devices/shutdown.h" #include "devices/input.h" #include "filesys/file.h" @@ -12,6 +11,7 @@ #include "userprog/process.h" #include "userprog/pagedir.h" #include +#include #include #define MAX_SYSCALL_ARGS 3 @@ -47,8 +47,11 @@ static unsigned syscall_tell (int fd); static void syscall_close (int fd); static struct open_file *fd_get_file (int fd); -static void validate_user_pointer (const void *start, size_t size, bool write); -static void validate_user_string (const char *str); +static void validate_user_pointer (const void *ptr, size_t size, + bool check_write); +static void validate_user_string (const char *str, bool check_write); +static int get_user (const uint8_t *); +static bool put_user (uint8_t *, uint8_t); /* A struct defining a syscall_function pointer along with its arity. */ struct syscall_arguments @@ -98,7 +101,8 @@ syscall_handler (struct intr_frame *f) { /* First, read the system call number from the stack. */ validate_user_pointer (f->esp, sizeof (uintptr_t), false); - uintptr_t syscall_number = *(int *) f->esp; + uintptr_t syscall_number = *(int *)f->esp; + thread_current ()->curr_esp = f->esp; /* Ensures the number corresponds to a system call that can be handled. */ if (syscall_number >= LOOKUP_SIZE) @@ -109,10 +113,9 @@ syscall_handler (struct intr_frame *f) /* Next, read and copy the arguments from the stack pointer. */ validate_user_pointer (f->esp + sizeof (uintptr_t), syscall.arity * sizeof (uintptr_t), false); - - uintptr_t args[MAX_SYSCALL_ARGS] = {0}; + uintptr_t args[MAX_SYSCALL_ARGS] = { 0 }; for (int i = 0; i < syscall.arity && i < MAX_SYSCALL_ARGS; i++) - args[i] = *(uintptr_t *) (f->esp + sizeof (uintptr_t) * (i + 1)); + args[i] = *(uintptr_t *)(f->esp + sizeof (uintptr_t) * (i + 1)); /* Call the function that handles this system call with the arguments. When there is a return value it is stored in f->eax. */ @@ -141,8 +144,7 @@ syscall_exit (int status) static pid_t syscall_exec (const char *cmd_line) { - /* Validate the user string before executing the process. */ - validate_user_string (cmd_line); + validate_user_string (cmd_line, false); return process_execute (cmd_line); /* Returns the PID of the new process */ } @@ -161,8 +163,7 @@ syscall_wait (pid_t pid) static bool syscall_create (const char *file, unsigned initial_size) { - /* Validate the user string before creating the file. */ - validate_user_string (file); + validate_user_string (file, false); /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); @@ -179,8 +180,7 @@ syscall_create (const char *file, unsigned initial_size) static bool syscall_remove (const char *file) { - /* Validate the user string before removing the file. */ - validate_user_string (file); + validate_user_string (file, false); /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); @@ -198,8 +198,7 @@ syscall_remove (const char *file) static int syscall_open (const char *file) { - /* Validate the user string before opening the file. */ - validate_user_string (file); + validate_user_string (file, false); /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); @@ -265,7 +264,6 @@ syscall_read (int fd, void *buffer, unsigned size) if (fd < STDIN_FILENO || fd == STDOUT_FILENO) return EXIT_FAILURE; - /* Validate the user buffer for the provided size before reading. */ validate_user_pointer (buffer, size, true); if (fd == STDIN_FILENO) @@ -309,7 +307,6 @@ syscall_write (int fd, const void *buffer, unsigned size) if (fd <= 0) return 0; - /* Validate the user buffer for the provided size before writing. */ validate_user_pointer (buffer, size, false); if (fd == STDOUT_FILENO) @@ -452,67 +449,91 @@ fd_get_file (int fd) return hash_entry (e, struct open_file, elem); } -/* Validates if a block of memory starting at START and of size SIZE bytes is - fully contained within user virtual memory. Kills the thread (by exiting with - failure) if the memory is invalid. Otherwise, returns (nothing) normally. - If the size is 0, the function does no checks and returns the given ptr. */ +/* Validates if a block of memory starting at PTR and of size SIZE bytes is + fully contained within valid user virtual memory. thread_exit () if the + memory is invalid. + If the size is 0, the function does no checks and returns PTR. */ static void -validate_user_pointer (const void *start, size_t size, bool write) +validate_user_pointer (const void *ptr, size_t size, bool check_write) { - /* If the size is 0, we do not need to check anything. */ if (size == 0) return; - - const void *end = start + size - 1; - - /* Check if the start and end pointers are valid user virtual addresses. */ - if (start == NULL || !is_user_vaddr (start) || !is_user_vaddr (end)) + /* ptr < ptr + size - 1, so sufficient to check that (ptr + size -1) is a + valid user virtual memory address. */ + void *last = ptr + size - 1; + if (!is_user_vaddr (last)) syscall_exit (EXIT_FAILURE); - - /* We no longer check if the memory is mapped to physical memory. This is - because the data may not necessarily be there at the time of the syscall, - but it may be lazily loaded later. In such case, we try to preload the - page. If that fails, we exit the thread. */ - for (void *ptr = pg_round_down (start); ptr <= end; ptr += PGSIZE) - if (pagedir_get_page (thread_current ()->pagedir, ptr) == NULL && - !try_fetch_page (ptr, write)) - syscall_exit (EXIT_FAILURE); + ptr = pg_round_down (ptr); + while (ptr <= last) + { + int result; + /* Check read access to pointer. */ + if ((result = get_user (ptr)) == -1) + syscall_exit (EXIT_FAILURE); + /* Check write access to pointer (if required). */ + if (check_write && !put_user (ptr, result)) + syscall_exit (EXIT_FAILURE); + ptr += PGSIZE; + } } -/* Validates if a string is fully contained within user virtual memory. Kills - the thread (by exiting with failure) if the memory is invalid. Otherwise, - returns (nothing) normally. */ +/* Validates of a C-string starting at ptr is fully contained within valid + user virtual memory. thread_exit () if the memory is invalid. */ static void -validate_user_string (const char *str) +validate_user_string (const char *ptr, bool check_write) { - /* Check if the string pointer is a valid user virtual address. */ - if (str == NULL || !is_user_vaddr (str)) - syscall_exit (EXIT_FAILURE); + size_t offset = (uintptr_t) ptr % PGSIZE; - /* Calculate the offset of the string within the (first) page. */ - size_t offset = (uintptr_t) str % PGSIZE; - - /* We move page by page, checking if the page is mapped to physical memory. */ for (;;) - { - void *page = pg_round_down (str); + { + void *page = pg_round_down (ptr); - /* If we reach addresses that are not mapped to physical memory before the - end of the string, the thread is terminated. */ - if (!is_user_vaddr(page) || - (pagedir_get_page (thread_current ()->pagedir, page) == NULL && - !try_fetch_page (page, false))) - syscall_exit (EXIT_FAILURE); + if (!is_user_vaddr (page)) + syscall_exit (EXIT_FAILURE); + if (!is_user_vaddr (ptr)) + syscall_exit (EXIT_FAILURE); + int result; + if ((result = get_user ((const uint8_t *)ptr)) == -1) + syscall_exit (EXIT_FAILURE); + if (check_write && !put_user ((uint8_t *)ptr, result)) + syscall_exit (EXIT_FAILURE); - while (offset < PGSIZE) + while (offset < PGSIZE) { - if (*str == '\0') + if (*ptr == '\0') return; /* We reached the end of the string without issues. */ - str++; + ptr++; offset++; } - offset = 0; /* Next page will start at the beginning. */ - } + offset = 0; + + } } + +/* PROVIDED BY SPEC. + Reads a byte at user virtual address UADDR. + UADDR must be below PHYS_BASE. + Returns the byte value if successful, -1 if a segfault occurred. */ +static int +get_user (const uint8_t *uaddr) +{ + int result; + asm ("movl $1f, %0; movzbl %1, %0; 1:" : "=&a"(result) : "m"(*uaddr)); + return result; +} + +/* PROVIDED BY SPEC. + Writes BYTE to user address UDST. + UDST must be below PHYS_BASE. + Returns true if successful, false if a segfault occurred. */ +static bool +put_user (uint8_t *udst, uint8_t byte) +{ + int error_code; + asm ("movl $1f, %0; movb %b2, %1; 1:" + : "=&a"(error_code), "=m"(*udst) + : "q"(byte)); + return error_code != -1; +} \ No newline at end of file