Update syscall handling of invalid pointers by preloading pages using the helper try_fetch_page

src/userprog/exception.c

@@ -13,7 +13,7 @@ static long long page_fault_cnt;
 
 static void kill (struct intr_frame *);
 static void page_fault (struct intr_frame *);
-static bool try_fetch_page (void *upage, bool write);
+bool try_fetch_page (void *upage, bool write);
 
 /* Registers handlers for interrupts that can be caused by user
    programs.
@@ -149,6 +149,14 @@ page_fault (struct intr_frame *f)
   write = (f->error_code & PF_W) != 0;
   user = (f->error_code & PF_U) != 0;
 
+  if (!user)
+  {
+    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
      is expected to have data loaded in memory. */
@@ -170,7 +178,7 @@ page_fault (struct intr_frame *f)
   kill (f);
 }
 
-static bool
+bool
 try_fetch_page (void *upage, bool write)
 {
   /* Check if the page is in the supplemental page table. That is, it is a page

src/userprog/exception.h

@@ -1,6 +1,8 @@
 #ifndef USERPROG_EXCEPTION_H
 #define USERPROG_EXCEPTION_H
 
+#include <stdbool.h>
+
 /* Page fault error code bits that describe the cause of the exception.  */
 #define PF_P 0x1    /* 0: not-present page. 1: access rights violation. */
 #define PF_W 0x2    /* 0: read, 1: write. */
@@ -8,5 +10,7 @@
 
 void exception_init (void);
 void exception_print_stats (void);
+bool
+try_fetch_page (void *upage, bool write);
 
 #endif /* userprog/exception.h */

src/userprog/syscall.c

@@ -1,4 +1,5 @@
 #include "userprog/syscall.h"
+#include "userprog/exception.h"
 #include "devices/shutdown.h"
 #include "devices/input.h"
 #include "filesys/file.h"
@@ -46,7 +47,7 @@ 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_pointer (const void *start, size_t size, bool write);
 static void validate_user_string (const char *str);
 
 /* A struct defining a syscall_function pointer along with its arity. */
@@ -96,7 +97,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));
+  validate_user_pointer (f->esp, sizeof (uintptr_t), false);
   uintptr_t syscall_number = *(int *) f->esp;
 
   /* Ensures the number corresponds to a system call that can be handled. */
@@ -107,7 +108,7 @@ 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));
+                         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++)
@@ -265,7 +266,7 @@ syscall_read (int fd, void *buffer, unsigned size)
     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)
     {
@@ -309,7 +310,7 @@ syscall_write (int fd, const void *buffer, unsigned size)
     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)
     {
@@ -456,7 +457,7 @@ fd_get_file (int fd)
    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. */
 static void
-validate_user_pointer (const void *start, size_t size)
+validate_user_pointer (const void *start, size_t size, bool write)
 {
   /* If the size is 0, we do not need to check anything. */
   if (size == 0)
@@ -468,10 +469,13 @@ validate_user_pointer (const void *start, size_t size)
   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 (thread_current ()->pagedir, ptr) == NULL)
+  /* 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);
 }
 
@@ -496,7 +500,8 @@ validate_user_string (const char *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) ||
-        pagedir_get_page (thread_current ()->pagedir, page) == NULL)
+        (pagedir_get_page (thread_current ()->pagedir, page) == NULL &&
+         !try_fetch_page (page, false)))
       syscall_exit (EXIT_FAILURE);
 
     while (offset < PGSIZE)