diff --git a/src/userprog/exception.c b/src/userprog/exception.c index 272325e..3e3b133 100644 --- a/src/userprog/exception.c +++ b/src/userprog/exception.c @@ -146,6 +146,14 @@ page_fault (struct intr_frame *f) write = (f->error_code & PF_W) != 0; user = (f->error_code & PF_U) != 0; + /* Kernel page fault is further handled by the kernel itself. */ + if (kernel) + { + f->eip = (void *)f->eax; + f->eax = 0xffffffff; + return; + } + if (user) { if (try_alloc_new_page (fault_addr, f->esp)) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 26a37e7..da6d77d 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -14,6 +14,7 @@ #include "vm/stackgrowth.h" #endif #include +#include #include #define MAX_SYSCALL_ARGS 3 @@ -49,8 +50,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); -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 @@ -99,8 +103,8 @@ static void syscall_handler (struct intr_frame *f) { /* First, read the system call number from the stack. */ - validate_user_pointer (f->esp, sizeof (uintptr_t)); - uintptr_t syscall_number = *(int *) f->esp; + validate_user_pointer (f->esp, sizeof (uintptr_t), false); + 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. */ @@ -111,11 +115,10 @@ 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)); - - uintptr_t args[MAX_SYSCALL_ARGS] = {0}; + syscall.arity * sizeof (uintptr_t), false); + 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. */ @@ -144,8 +147,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 */ } @@ -164,8 +166,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); @@ -182,8 +183,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); @@ -201,8 +201,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); @@ -268,8 +267,7 @@ 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); + validate_user_pointer (buffer, size, true); if (fd == STDIN_FILENO) { @@ -312,8 +310,7 @@ 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); + validate_user_pointer (buffer, size, false); if (fd == STDOUT_FILENO) { @@ -455,72 +452,76 @@ 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) +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; - - struct thread *t = thread_current (); - - 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)) - syscall_exit (EXIT_FAILURE); - - /* We now need to check if the entire memory block is mapped to physical - memory by the page table. */ - for (const void *ptr = pg_round_down (start); ptr <= end; ptr += PGSIZE) - if (pagedir_get_page (t->pagedir, ptr) == NULL) - { - if (!try_alloc_new_page (ptr, t->curr_esp)) - syscall_exit (EXIT_FAILURE); - } + /* 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)) + thread_exit (); + ptr = pg_round_down (ptr); + while (ptr <= last) + { + int result; + /* Check read access to pointer. */ + if ((result = get_user (ptr)) == -1) + thread_exit (); + /* Check write access to pointer (if required). */ + if (check_write && !put_user (ptr, result)) + thread_exit (); + 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); - - /* Calculate the offset of the string within the (first) page. */ - size_t offset = (uintptr_t) str % PGSIZE; - struct thread *t = thread_current (); - - /* We move page by page, checking if the page is mapped to physical memory. */ - for (;;) - { - void *page = pg_round_down (str); - - /* 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)) - syscall_exit (EXIT_FAILURE); - - if (pagedir_get_page (t->pagedir, page) == NULL) - if (!try_alloc_new_page (str, t->curr_esp)) - syscall_exit (EXIT_FAILURE); - - while (offset < PGSIZE) - { - if (*str == '\0') - return; /* We reached the end of the string without issues. */ - - str++; - offset++; - } - - offset = 0; /* Next page will start at the beginning. */ - } + while (true) + { + if (!is_user_vaddr (ptr)) + thread_exit (); + int result; + if ((result = get_user ((const uint8_t *)ptr)) == -1) + thread_exit (); + if (check_write && !put_user ((uint8_t *)ptr, result)) + thread_exit (); + if (*ptr == '\0') + return; + ptr++; + } } + +/* 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