diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index c3caba3..c55c090 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -10,6 +10,7 @@ #include "threads/synch.h" #include "userprog/process.h" #include "userprog/pagedir.h" +#include "vm/frame.h" #include "vm/page.h" #include "vm/mmap.h" #include @@ -52,9 +53,14 @@ static mapid_t syscall_mmap (int fd, void *addr); static void syscall_munmap (mapid_t mapping); static struct open_file *fd_get_file (int fd); -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 void validate_user_ptr (const void *start, size_t size, + bool write); +static void validate_and_pin_user_ptr (const void *start, size_t size, + bool write); +static void validate_and_pin_user_str (const char *ptr); +static void unpin_user_ptr (const void *start, size_t size); + +static void unpin_user_str (const char *ptr); static int get_user (const uint8_t *); static bool put_user (uint8_t *, uint8_t); @@ -107,7 +113,7 @@ 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), false); + validate_user_ptr (f->esp, sizeof (uintptr_t), false); uintptr_t syscall_number = *(int *)f->esp; thread_current ()->curr_esp = f->esp; @@ -118,7 +124,7 @@ syscall_handler (struct intr_frame *f) struct syscall_arguments syscall = syscall_lookup[syscall_number]; /* Next, read and copy the arguments from the stack pointer. */ - validate_user_pointer (f->esp + sizeof (uintptr_t), + validate_user_ptr (f->esp + sizeof (uintptr_t), 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++) @@ -151,9 +157,11 @@ syscall_exit (int status) static pid_t syscall_exec (const char *cmd_line) { - validate_user_string (cmd_line, false); + validate_and_pin_user_str (cmd_line); + pid_t pid = process_execute (cmd_line); + unpin_user_str (cmd_line); - return process_execute (cmd_line); /* Returns the PID of the new process */ + return pid; } /* Handles the syscall of wait. Effectively a wrapper for process_wait as the @@ -170,13 +178,15 @@ syscall_wait (pid_t pid) static bool syscall_create (const char *file, unsigned initial_size) { - validate_user_string (file, false); + validate_and_pin_user_str (file); /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); bool status = filesys_create (file, initial_size); lock_release (&filesys_lock); + unpin_user_str (file); + /* Return the status of the file creation. */ return status; } @@ -187,13 +197,15 @@ syscall_create (const char *file, unsigned initial_size) static bool syscall_remove (const char *file) { - validate_user_string (file, false); + validate_and_pin_user_str (file); /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); bool status = filesys_remove (file); lock_release (&filesys_lock); + unpin_user_str (file); + /* Return the status of the file removal. */ return status; } @@ -205,13 +217,15 @@ syscall_remove (const char *file) static int syscall_open (const char *file) { - validate_user_string (file, false); + validate_and_pin_user_str (file); /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); struct file *ptr = filesys_open (file); lock_release (&filesys_lock); + unpin_user_str (file); + /* If the file could not be opened, return failure. */ if (ptr == NULL) return EXIT_FAILURE; @@ -271,10 +285,11 @@ syscall_read (int fd, void *buffer, unsigned size) if (fd < STDIN_FILENO || fd == STDOUT_FILENO) return EXIT_FAILURE; - validate_user_pointer (buffer, size, true); - if (fd == STDIN_FILENO) { + /* Validate the user buffer. */ + validate_user_ptr (buffer, size, true); + /* Reading from the console. */ char *write_buffer = buffer; for (unsigned i = 0; i < size; i++) @@ -292,13 +307,19 @@ syscall_read (int fd, void *buffer, unsigned size) if (file_info == NULL) return EXIT_FAILURE; + /* Validate the user buffer, and pin the pages to prevent eviction. */ + validate_and_pin_user_ptr (buffer, size, true); + /* Acquire the file system lock to prevent race-conditions. */ lock_acquire (&filesys_lock); - int bytes_written = file_read (file_info->file, buffer, size); + int bytes_read = file_read (file_info->file, buffer, size); lock_release (&filesys_lock); + /* Unpin the pages to allow eviction. */ + unpin_user_ptr (buffer, size); + /* Return the number of bytes read. */ - return bytes_written; + return bytes_read; } } @@ -314,10 +335,11 @@ syscall_write (int fd, const void *buffer, unsigned size) if (fd <= 0) return 0; - validate_user_pointer (buffer, size, false); - if (fd == STDOUT_FILENO) { + /* Validate the user buffer. */ + validate_user_ptr (buffer, size, false); + /* Writing to the console. */ putbuf (buffer, size); @@ -333,13 +355,19 @@ syscall_write (int fd, const void *buffer, unsigned size) if (file_info == NULL) return 0; + /* Validate the user buffer, and pin the pages to prevent eviction. */ + validate_and_pin_user_ptr (buffer, size, false); + /* Acquire the file system lock to prevent race conditions. */ lock_acquire (&filesys_lock); - int bytes = file_write (file_info->file, buffer, size); + int bytes_written = file_write (file_info->file, buffer, size); lock_release (&filesys_lock); + /* Unpin the pages to allow eviction. */ + unpin_user_ptr (buffer, size); + /* Return the number of bytes written. */ - return bytes; + return bytes_written; } } @@ -460,7 +488,7 @@ syscall_mmap (int fd, void *addr) /* Handles the syscall for unmapping a memory mapped file. - Pre: mapping is a valid mapping identifier returned by mmap syscall. */ + Pre: mapping is a valid mapping identifier returned by mmap syscall. */ static void syscall_munmap (mapid_t mapping) { @@ -533,69 +561,171 @@ fd_get_file (int fd) return hash_entry (e, struct open_file, elem); } +/* Helper function that validates a block of memory and optionally pins frames. + thread_exit() if the memory is invalid. Used only by the two helper functions + validate_user_ptr and validate_and_pin_user_ptr. See the comments for those + functions for more details on each. */ +static void +validate_user_ptr_helper (const void *start, size_t size, bool write, bool pin) +{ + if (size == 0) + return; + + /* ptr < ptr + size - 1, so sufficient to check that (ptr + size -1) is a + valid user virtual memory address. */ + void *end = start + size - 1; + if (!is_user_vaddr (end)) + syscall_exit (EXIT_FAILURE); + + for (const void *ptr = pg_round_down (start); ptr <= end; ptr += PGSIZE) + { + 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 (write && !put_user ((uint8_t *)ptr, result)) + syscall_exit (EXIT_FAILURE); + + /* If pin is set, pin the frame to prevent eviction. */ + if (pin) + { + void *kpage = pagedir_get_page(thread_current()->pagedir, ptr); + if (kpage == NULL) + { + // If it was evicted, try to load it back in. + ptr -= PGSIZE; + continue; + } + + frame_pin(kpage); + } + } +} + /* 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 *ptr, size_t size, bool check_write) +validate_user_ptr (const void *start, size_t size, bool write) { - if (size == 0) - return; - /* 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); - 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; - } + validate_user_ptr_helper (start, size, write, false); +} + +/* 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. The function also checks if the memory is writable if + WRITE flag is set. + + The function attempts to preload the pages in case they are not in memory + yet (e.g., in a swap, lazy loading). If this is successful, the frame pages + are pinned to prevent eviction prior to access. + + As such, a call to this function MUST be followed by a call to + unpin_user_ptr (START, SIZE) to unpin the pages and allow eviction. + + If the size is 0, the function does no checks and returns PTR. */ +static void +validate_and_pin_user_ptr (const void *start, size_t size, bool write) +{ + validate_user_ptr_helper (start, size, write, true); +} + +/* Unpins all the pages containing a block of memory starting at START and of + size SIZE bytes. + + Pre: The pages were previously pinned by validate_and_pin_user_ptr (START, + SIZE). */ +static void +unpin_user_ptr (const void *start, size_t size) +{ + void *end = start + size - 1; + + /* We don't need to do any checks as this function is always called after + validate_and_pin_user_ptr. */ + /* Go through all pages in the block range, unpinning the frames. */ + for (void *ptr = pg_round_down (start); ptr <= end; ptr += PGSIZE) + { + void *kpage = pagedir_get_page (thread_current ()->pagedir, ptr); + ASSERT (kpage != NULL); + + frame_unpin (kpage); + } } /* 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 *ptr, bool check_write) +validate_and_pin_user_str (const char *ptr) { size_t offset = (uintptr_t) ptr % PGSIZE; for (;;) { - void *page = pg_round_down (ptr); - - 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)) + + if (get_user ((const uint8_t *)ptr) == -1) syscall_exit (EXIT_FAILURE); + /* Pin the frame to prevent eviction. */ + void *page = pg_round_down (ptr); + void *kpage = pagedir_get_page (thread_current ()->pagedir, page); + if (kpage == NULL) + { + // If it was evicted, attempt to reload. + ptr -= PGSIZE; + continue; + } + + frame_pin (kpage); + while (offset < PGSIZE) - { - if (*ptr == '\0') - return; /* We reached the end of the string without issues. */ + { + if (*ptr == '\0') + return; /* We reached the end of the string without issues. */ - ptr++; - offset++; - } + ptr++; + offset++; + } offset = 0; - } } +/* Unpins all the pages containing a C-string starting at PTR. + + Pre: The pages were previously pinned by validate_and_pin_user_str (PTR). + PTR points to a valid C string that ends with '\0'. */ +static void +unpin_user_str (const char *ptr) +{ + size_t offset = (uintptr_t)ptr % PGSIZE; + const char *str_ptr = ptr; + + for (;;) + { + void *page = pg_round_down(str_ptr); + void *kpage = pagedir_get_page(thread_current()->pagedir, page); + ASSERT(kpage != NULL); + frame_unpin (kpage); + + /* Scan until end of string or page */ + while (offset < PGSIZE) + { + if (*str_ptr == '\0') + return; /* Found end of string */ + str_ptr++; + offset++; + } + + offset = 0; + } +} + /* PROVIDED BY SPEC. Reads a byte at user virtual address UADDR. UADDR must be below PHYS_BASE.