From ade8faf0f4abe285c103f8085dd2d8fe22c58197 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:20:09 +0000 Subject: [PATCH 1/6] Update syscall to add more comments explaining the basic handler --- src/userprog/syscall.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 5e53cd3..2926bbd 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -13,24 +13,27 @@ static void syscall_handler (struct intr_frame *); in size. */ typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); -/* System Call Functions */ +/* System call function prototypes */ static void halt (void); static void exit (int status); static void *validate_user_pointer(void *ptr, size_t size); -/* A struct defining a pair of a syscall_function along with its arity. */ +/* A struct defining a syscall_function pointer along with its arity. */ typedef struct { - syscall_function function; - int arity; + syscall_function function; /* Function pointer. */ + int arity; /* Number of arguments of the function. */ } syscall_arguments; -/* A look-up table for system call functions mapped using their numbers. */ +/* A look-up table mapping numbers to system call functions with their number of + arguments. */ static const syscall_arguments syscall_lookup[] = { [SYS_HALT] = {(syscall_function) halt, 0}, [SYS_EXIT] = {(syscall_function) exit, 1}, }; +/* The number of syscall functions (i.e, number of elements) within the + syscall_lookup table. */ static const int lookup_size = sizeof (syscall_lookup) / sizeof (syscall_arguments); @@ -43,20 +46,24 @@ syscall_init (void) static void syscall_handler (struct intr_frame *f) { + /* First, read the system call number from the stack. */ validate_user_pointer(f->esp, 1); int syscall_number = *(int *) f->esp; + /* Ensures the number corresponds to a system call that can be handled. */ if (syscall_number < 0 || syscall_number >= lookup_size) thread_exit (); syscall_arguments syscall = syscall_lookup[syscall_number]; + /* Next, read and copy the arguments from the stack pointer. */ validate_user_pointer (f->esp, syscall.arity); uintptr_t args[3]; - for (int i=0; i < syscall.arity; i++) args[i] = *(uintptr_t *) (f->esp + 1 + i); + /* Call the function that handles this system call with the arguments. When + there is a return value it is stored in f->eax. */ f->eax = syscall.function(args[0], args[1], args[2]); } @@ -67,13 +74,13 @@ halt (void) } static void -exit (int status) +exit (int status UNUSED) { //TODO } -/* A function to validate if a block of memory starting at PTR and of - size SIZE bytes is fully contained within user virtual memory. */ +/* Validates if a block of memory starting at PTR and of size SIZE bytes is + fully contained within user virtual memory. */ static void * validate_user_pointer (void *ptr, size_t size) { From e718159ed89acf205213caf4cb218a52bb5312d0 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:29:07 +0000 Subject: [PATCH 2/6] Update syscall to use screaming uppercase casing for a constant --- src/userprog/syscall.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 2926bbd..ddbbd39 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -34,7 +34,7 @@ static const syscall_arguments syscall_lookup[] = { /* The number of syscall functions (i.e, number of elements) within the syscall_lookup table. */ -static const int lookup_size +static const int LOOKUP_SIZE = sizeof (syscall_lookup) / sizeof (syscall_arguments); void @@ -51,7 +51,7 @@ syscall_handler (struct intr_frame *f) int syscall_number = *(int *) f->esp; /* Ensures the number corresponds to a system call that can be handled. */ - if (syscall_number < 0 || syscall_number >= lookup_size) + if (syscall_number < 0 || syscall_number >= LOOKUP_SIZE) thread_exit (); syscall_arguments syscall = syscall_lookup[syscall_number]; From 3a258cf064c51f6f69df48a5bf37de9097f337c7 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:38:58 +0000 Subject: [PATCH 3/6] Update validate_user_pointer to perform no memory checks when size is 0 --- src/userprog/syscall.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index ddbbd39..fc8c28e 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -80,11 +80,15 @@ exit (int status UNUSED) } /* Validates if a block of memory starting at PTR and of size SIZE bytes is - fully contained within user virtual memory. */ + fully contained within user virtual memory. Kills the thread (by calling + thread_exit) if the memory is invalid. Otherwise, returns the PTR given. + If the size is 0, the function does no checks and returns PTR.*/ static void * validate_user_pointer (void *ptr, size_t size) { - if (ptr == NULL || !is_user_vaddr (ptr) || !is_user_vaddr (ptr + size - 1)) + if (size > 0 && (ptr == NULL || + !is_user_vaddr (ptr) || + !is_user_vaddr (ptr + size - 1))) thread_exit (); return ptr; From 79f6a8e8080dfd2fc15fd9bd31dc0365600c4600 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:44:55 +0000 Subject: [PATCH 4/6] Fix Bug in syscall handler related to pointer arithmetic: add sizeof uintptr_t instead of 1 --- src/userprog/syscall.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index fc8c28e..e2f0163 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -57,10 +57,11 @@ syscall_handler (struct intr_frame *f) syscall_arguments syscall = syscall_lookup[syscall_number]; /* Next, read and copy the arguments from the stack pointer. */ - validate_user_pointer (f->esp, syscall.arity); - uintptr_t args[3]; + validate_user_pointer (f->esp + sizeof (uintptr_t), + syscall.arity * sizeof (uintptr_t)); + uintptr_t args[3] = {0}; for (int i=0; i < syscall.arity; i++) - args[i] = *(uintptr_t *) (f->esp + 1 + i); + 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. */ From 0d057da3dc56dd90fa487df96daf3410ac5ae65d Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:48:36 +0000 Subject: [PATCH 5/6] Refactor syscall to follow PintOS style in adding space after after function name in calls --- src/userprog/syscall.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index e2f0163..f225b41 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -17,17 +17,19 @@ typedef uintptr_t (*syscall_function) (uintptr_t, uintptr_t, uintptr_t); static void halt (void); static void exit (int status); -static void *validate_user_pointer(void *ptr, size_t size); +static void *validate_user_pointer (void *ptr, size_t size); /* A struct defining a syscall_function pointer along with its arity. */ -typedef struct { - syscall_function function; /* Function pointer. */ - int arity; /* Number of arguments of the function. */ -} syscall_arguments; +typedef struct + { + syscall_function function; /* Function pointer. */ + int arity; /* Number of arguments of the function. */ + } syscall_arguments; /* A look-up table mapping numbers to system call functions with their number of arguments. */ -static const syscall_arguments syscall_lookup[] = { +static const syscall_arguments syscall_lookup[] = +{ [SYS_HALT] = {(syscall_function) halt, 0}, [SYS_EXIT] = {(syscall_function) exit, 1}, }; @@ -47,7 +49,7 @@ static void syscall_handler (struct intr_frame *f) { /* First, read the system call number from the stack. */ - validate_user_pointer(f->esp, 1); + validate_user_pointer (f->esp, 1); int syscall_number = *(int *) f->esp; /* Ensures the number corresponds to a system call that can be handled. */ @@ -65,7 +67,7 @@ syscall_handler (struct intr_frame *f) /* Call the function that handles this system call with the arguments. When there is a return value it is stored in f->eax. */ - f->eax = syscall.function(args[0], args[1], args[2]); + f->eax = syscall.function (args[0], args[1], args[2]); } static void From 5e2342fad781a75a70bf6f7bda65d8b2b3d71e1a Mon Sep 17 00:00:00 2001 From: sBubshait Date: Mon, 4 Nov 2024 00:49:47 +0000 Subject: [PATCH 6/6] Update syscall to make syscall_number an unsigned integer instead of an int --- src/userprog/syscall.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index f225b41..2b504ec 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -50,10 +50,10 @@ syscall_handler (struct intr_frame *f) { /* First, read the system call number from the stack. */ validate_user_pointer (f->esp, 1); - int syscall_number = *(int *) f->esp; + unsigned syscall_number = *(int *) f->esp; /* Ensures the number corresponds to a system call that can be handled. */ - if (syscall_number < 0 || syscall_number >= LOOKUP_SIZE) + if (syscall_number >= LOOKUP_SIZE) thread_exit (); syscall_arguments syscall = syscall_lookup[syscall_number];