Compare commits

...

18 Commits

Author SHA1 Message Date
149b0a9d87 Remove filesys CI for gitlab 2024-11-14 03:25:33 +00:00
Saleh Bubshait
79b3b8fda7 Merge branch 'userprog-oom' into 'master'
Fix multi-oom

See merge request lab2425_autumn/pintos_22!39
2024-11-13 22:09:11 +00:00
f5e498e0a9 explicit thread_exit () when process_start () fails 2024-11-13 21:58:41 +00:00
d02b956221 Merge branch 'system-calls' into 'userprog-oom'
Add Fixes to Memory Leaks, Memory Access Validation, Synchronised Processes and Refactoring

See merge request lab2425_autumn/pintos_22!38
2024-11-13 19:20:19 +00:00
Saleh Bubshait
927c376d02 Merge branch 'task2/system-calls/saleh' into 'system-calls'
Fix Memory Leaks, Synchronisation in Processes, and Refactoring

See merge request lab2425_autumn/pintos_22!37
2024-11-13 18:51:51 +00:00
sBubshait
fa2fb4a711 Refactor system call comments for accuracy and grammar 2024-11-13 18:48:23 +00:00
sBubshait
31ea215805 Refactor validate_user_string to remove unnecessary variable to track length of str 2024-11-13 18:30:24 +00:00
e7cb16b301 Fix child_results loop accessing next after free() 2024-11-13 18:29:05 +00:00
sBubshait
9549ca28e5 Refactor syscall: Use EXIT_FAILURE instead of magic numbers & close files on failure 2024-11-13 18:18:41 +00:00
sBubshait
b1c5819469 Avoid masking the struct syscall_arguments using typedef for consistency 2024-11-13 18:06:51 +00:00
sBubshait
287130ca7b Update syscall.c to use syscall_exit on failure instead of calling thread_exit directly 2024-11-13 18:02:08 +00:00
sBubshait
4f586bb4da Fix Bug: Free all entries in the fd hashtable when the process exits, w/ E 2024-11-13 17:42:25 +00:00
sBubshait
d890c2353e Add constant MAX_SYSCALL_ARGS to avoid magic numbers [Gleb] 2024-11-13 17:16:15 +00:00
sBubshait
eb4f23c290 Add validate_user_string helper function to validate that a string is fully conatined within user vm 2024-11-13 17:11:37 +00:00
sBubshait
26de38cdba Update validate_user_pointer to check if the memory block is mapped into a physical address 2024-11-13 16:39:45 +00:00
sBubshait
6e59e8c9f3 Update validate_user_pointer to be a void function 2024-11-13 16:22:16 +00:00
sBubshait
30e49846b5 Add more tests for system calls to deal with bad buffers given to read and write 2024-11-13 16:21:12 +00:00
005791edd2 Synchronise process_execute return with child process load 2024-11-13 11:05:09 +00:00
16 changed files with 235 additions and 82 deletions

View File

@@ -23,11 +23,6 @@ test_devices:
variables: variables:
DIR: devices DIR: devices
test_filesys:
extends: .pintos_tests
variables:
DIR: filesys
test_threads: test_threads:
extends: .pintos_tests extends: .pintos_tests
variables: variables:

View File

@@ -9,14 +9,14 @@ sc-bad-arg sc-bad-num sc-boundary sc-boundary-2 halt exit create-normal \
create-empty create-null create-bad-ptr create-long create-exists \ create-empty create-null create-bad-ptr create-long create-exists \
create-bound open-normal open-missing open-boundary open-empty \ create-bound open-normal open-missing open-boundary open-empty \
open-null open-bad-ptr open-twice close-normal close-twice close-stdin \ open-null open-bad-ptr open-twice close-normal close-twice close-stdin \
close-stdout close-bad-fd read-normal read-bad-ptr read-boundary \ close-stdout close-bad-fd read-normal read-bad-ptr read-bad-buf read-boundary \
read-zero read-stdout read-bad-fd write-normal write-bad-ptr \ read-zero read-stdout read-bad-fd write-normal write-bad-ptr write-bad-buf \
write-boundary write-zero write-stdin write-bad-fd exec-once exec-arg \ write-boundary write-zero write-stdin write-bad-fd exec-once exec-arg \
exec-large-arg exec-multiple exec-missing exec-over-arg exec-over-args \ exec-large-arg exec-multiple exec-missing exec-over-arg exec-over-args \
exec-bad-ptr wait-simple wait-twice wait-killed wait-load-kill \ exec-bad-ptr wait-simple wait-twice wait-killed wait-load-kill \
wait-bad-pid wait-bad-child multi-recurse multi-child-fd rox-simple \ wait-bad-pid wait-bad-child multi-recurse multi-child-fd rox-simple \
rox-child rox-multichild bad-read bad-write bad-read2 bad-write2 \ rox-child rox-multichild bad-read bad-write bad-read2 bad-write2 \
bad-jump bad-jump2 bad-maths) bad-jump bad-jump2 bad-maths overflow-stack)
tests/userprog_PROGS = $(tests/userprog_TESTS) $(addprefix \ tests/userprog_PROGS = $(tests/userprog_TESTS) $(addprefix \
tests/userprog/,child-simple child-args child-bad child-close child-rox exec-exit) tests/userprog/,child-simple child-args child-bad child-close child-rox exec-exit)
@@ -36,6 +36,7 @@ tests/userprog/bad-read2_SRC = tests/userprog/bad-read2.c tests/main.c
tests/userprog/bad-write2_SRC = tests/userprog/bad-write2.c tests/main.c tests/userprog/bad-write2_SRC = tests/userprog/bad-write2.c tests/main.c
tests/userprog/bad-jump2_SRC = tests/userprog/bad-jump2.c tests/main.c tests/userprog/bad-jump2_SRC = tests/userprog/bad-jump2.c tests/main.c
tests/userprog/bad-maths_SRC = tests/userprog/bad-maths.c tests/main.c tests/userprog/bad-maths_SRC = tests/userprog/bad-maths.c tests/main.c
tests/userprog/overflow-stack_SRC = tests/userprog/overflow-stack.c tests/main.c
tests/userprog/sc-boundary_SRC = tests/userprog/sc-boundary.c \ tests/userprog/sc-boundary_SRC = tests/userprog/sc-boundary.c \
tests/userprog/boundary.c tests/main.c tests/userprog/boundary.c tests/main.c
tests/userprog/sc-boundary-2_SRC = tests/userprog/sc-boundary-2.c \ tests/userprog/sc-boundary-2_SRC = tests/userprog/sc-boundary-2.c \
@@ -66,6 +67,7 @@ tests/userprog/close-stdout_SRC = tests/userprog/close-stdout.c tests/main.c
tests/userprog/close-bad-fd_SRC = tests/userprog/close-bad-fd.c tests/main.c tests/userprog/close-bad-fd_SRC = tests/userprog/close-bad-fd.c tests/main.c
tests/userprog/read-normal_SRC = tests/userprog/read-normal.c tests/main.c tests/userprog/read-normal_SRC = tests/userprog/read-normal.c tests/main.c
tests/userprog/read-bad-ptr_SRC = tests/userprog/read-bad-ptr.c tests/main.c tests/userprog/read-bad-ptr_SRC = tests/userprog/read-bad-ptr.c tests/main.c
tests/userprog/read-bad-buf_SRC = tests/userprog/read-bad-buf.c tests/main.c
tests/userprog/read-boundary_SRC = tests/userprog/read-boundary.c \ tests/userprog/read-boundary_SRC = tests/userprog/read-boundary.c \
tests/userprog/boundary.c tests/main.c tests/userprog/boundary.c tests/main.c
tests/userprog/read-zero_SRC = tests/userprog/read-zero.c tests/main.c tests/userprog/read-zero_SRC = tests/userprog/read-zero.c tests/main.c
@@ -73,6 +75,7 @@ tests/userprog/read-stdout_SRC = tests/userprog/read-stdout.c tests/main.c
tests/userprog/read-bad-fd_SRC = tests/userprog/read-bad-fd.c tests/main.c tests/userprog/read-bad-fd_SRC = tests/userprog/read-bad-fd.c tests/main.c
tests/userprog/write-normal_SRC = tests/userprog/write-normal.c tests/main.c tests/userprog/write-normal_SRC = tests/userprog/write-normal.c tests/main.c
tests/userprog/write-bad-ptr_SRC = tests/userprog/write-bad-ptr.c tests/main.c tests/userprog/write-bad-ptr_SRC = tests/userprog/write-bad-ptr.c tests/main.c
tests/userprog/write-bad-buf_SRC = tests/userprog/write-bad-buf.c tests/main.c
tests/userprog/write-boundary_SRC = tests/userprog/write-boundary.c \ tests/userprog/write-boundary_SRC = tests/userprog/write-boundary.c \
tests/userprog/boundary.c tests/main.c tests/userprog/boundary.c tests/main.c
tests/userprog/write-zero_SRC = tests/userprog/write-zero.c tests/main.c tests/userprog/write-zero_SRC = tests/userprog/write-zero.c tests/main.c
@@ -122,10 +125,12 @@ tests/userprog/close-normal_PUTFILES += tests/userprog/sample.txt
tests/userprog/close-twice_PUTFILES += tests/userprog/sample.txt tests/userprog/close-twice_PUTFILES += tests/userprog/sample.txt
tests/userprog/read-normal_PUTFILES += tests/userprog/sample.txt tests/userprog/read-normal_PUTFILES += tests/userprog/sample.txt
tests/userprog/read-bad-ptr_PUTFILES += tests/userprog/sample.txt tests/userprog/read-bad-ptr_PUTFILES += tests/userprog/sample.txt
tests/userprog/read-bad-buf_PUTFILES += tests/userprog/sample.txt
tests/userprog/read-boundary_PUTFILES += tests/userprog/sample.txt tests/userprog/read-boundary_PUTFILES += tests/userprog/sample.txt
tests/userprog/read-zero_PUTFILES += tests/userprog/sample.txt tests/userprog/read-zero_PUTFILES += tests/userprog/sample.txt
tests/userprog/write-normal_PUTFILES += tests/userprog/sample.txt tests/userprog/write-normal_PUTFILES += tests/userprog/sample.txt
tests/userprog/write-bad-ptr_PUTFILES += tests/userprog/sample.txt tests/userprog/write-bad-ptr_PUTFILES += tests/userprog/sample.txt
tests/userprog/write-bad-buf_PUTFILES += tests/userprog/sample.txt
tests/userprog/write-boundary_PUTFILES += tests/userprog/sample.txt tests/userprog/write-boundary_PUTFILES += tests/userprog/sample.txt
tests/userprog/write-zero_PUTFILES += tests/userprog/sample.txt tests/userprog/write-zero_PUTFILES += tests/userprog/sample.txt
tests/userprog/multi-child-fd_PUTFILES += tests/userprog/sample.txt tests/userprog/multi-child-fd_PUTFILES += tests/userprog/sample.txt

View File

@@ -1,5 +1,9 @@
Full robustness of argument passing code: Full robustness of argument passing and syscall handling code:
- Test user stack overflow robustness of "exec" system calls. - Test user stack overflow robustness of "exec" system calls and user code.
5 exec-over-arg 5 exec-over-arg
5 exec-over-args 5 exec-over-args
5 overflow-stack
- Test syscall user provided buffer validity checks.
5 read-bad-buf
5 write-bad-buf

View File

@@ -2,11 +2,7 @@
use strict; use strict;
use warnings; use warnings;
use tests::tests; use tests::tests;
check_expected ([<<'EOF', <<'EOF']); check_expected ([<<'EOF']);
(exec-bad-ptr) begin
(exec-bad-ptr) end
exec-bad-ptr: exit(0)
EOF
(exec-bad-ptr) begin (exec-bad-ptr) begin
exec-bad-ptr: exit(-1) exec-bad-ptr: exit(-1)
EOF EOF

View File

@@ -2,11 +2,7 @@
use strict; use strict;
use warnings; use warnings;
use tests::tests; use tests::tests;
check_expected ([<<'EOF', <<'EOF']); check_expected ([<<'EOF']);
(open-bad-ptr) begin
(open-bad-ptr) end
open-bad-ptr: exit(0)
EOF
(open-bad-ptr) begin (open-bad-ptr) begin
open-bad-ptr: exit(-1) open-bad-ptr: exit(-1)
EOF EOF

View File

@@ -0,0 +1,17 @@
/* Attempt to overflow the user stack by allocating a 4kB buffer and writing into it.
The process must be terminated with -1 exit code until stack growth has been implemented in Task 3
*/
#include <string.h>
#include <syscall.h>
#include "tests/lib.h"
#include "tests/main.h"
void
test_main (void)
{
char stack_obj[4096];
memset (stack_obj, 'a', sizeof stack_obj);
memset (stack_obj+10, '\0', 1);
msg ("buffer: %s", stack_obj);
}

View File

@@ -0,0 +1,14 @@
# -*- perl -*-
use strict;
use warnings;
use tests::tests;
check_expected (IGNORE_USER_FAULTS => 1, [<<'EOF',<<'EOF']);
(overflow-stack) begin
overflow-stack: exit(-1)
EOF
(overflow-stack) begin
(overflow-stack) buffer: aaaaaaaaaa
(overflow-stack) end
overflow-stack: exit(0)
EOF
pass;

View File

@@ -0,0 +1,17 @@
/* Passes a buffer to the read system call that starts in valid memory, but runs into kernel space.
The process must be terminated with -1 exit code.
*/
#include <syscall.h>
#include "tests/lib.h"
#include "tests/main.h"
void
test_main (void)
{
int handle;
CHECK ((handle = open ("sample.txt")) > 1, "open \"sample.txt\"");
read (handle, (char *) 0xbfffffe0, 100);
fail ("should not have survived read()");
}

View File

@@ -0,0 +1,10 @@
# -*- perl -*-
use strict;
use warnings;
use tests::tests;
check_expected (IGNORE_KERNEL_FAULTS => 1, [<<'EOF']);
(read-bad-buf) begin
(read-bad-buf) open "sample.txt"
read-bad-buf: exit(-1)
EOF
pass;

View File

@@ -2,12 +2,7 @@
use strict; use strict;
use warnings; use warnings;
use tests::tests; use tests::tests;
check_expected ([<<'EOF', <<'EOF']); check_expected ([<<'EOF']);
(read-bad-ptr) begin
(read-bad-ptr) open "sample.txt"
(read-bad-ptr) end
read-bad-ptr: exit(0)
EOF
(read-bad-ptr) begin (read-bad-ptr) begin
(read-bad-ptr) open "sample.txt" (read-bad-ptr) open "sample.txt"
read-bad-ptr: exit(-1) read-bad-ptr: exit(-1)

View File

@@ -0,0 +1,17 @@
/* Passes a buffer to the write system call that starts in valid memory, but runs into kernel space.
The process must be terminated with -1 exit code.
*/
#include <syscall.h>
#include "tests/lib.h"
#include "tests/main.h"
void
test_main (void)
{
int handle;
CHECK ((handle = open ("sample.txt")) > 1, "open \"sample.txt\"");
write (handle, (char *) 0xbffffff0, 32);
fail ("should have exited with -1");
}

View File

@@ -0,0 +1,10 @@
# -*- perl -*-
use strict;
use warnings;
use tests::tests;
check_expected (IGNORE_KERNEL_FAULTS => 1, [<<'EOF']);
(write-bad-buf) begin
(write-bad-buf) open "sample.txt"
write-bad-buf: exit(-1)
EOF
pass;

View File

@@ -2,12 +2,7 @@
use strict; use strict;
use warnings; use warnings;
use tests::tests; use tests::tests;
check_expected ([<<'EOF', <<'EOF']); check_expected ([<<'EOF']);
(write-bad-ptr) begin
(write-bad-ptr) open "sample.txt"
(write-bad-ptr) end
write-bad-ptr: exit(0)
EOF
(write-bad-ptr) begin (write-bad-ptr) begin
(write-bad-ptr) open "sample.txt" (write-bad-ptr) open "sample.txt"
write-bad-ptr: exit(-1) write-bad-ptr: exit(-1)

View File

@@ -47,6 +47,9 @@ struct process_start_data
tokens while maintaining state. */ tokens while maintaining state. */
char file_name[FNAME_MAX_LEN + 1]; /* Name of the file of the process to char file_name[FNAME_MAX_LEN + 1]; /* Name of the file of the process to
be started. */ be started. */
bool success; /* Indicates whether the process was successfully loaded. */
struct semaphore loaded; /* Semaphore used to signal that the process has
been loaded. */
}; };
static thread_func start_process NO_RETURN; static thread_func start_process NO_RETURN;
@@ -67,6 +70,8 @@ process_execute (const char *cmd)
{ {
return TID_ERROR; return TID_ERROR;
} }
sema_init (&data->loaded, 0);
data->success = false;
/* Make a copy of command. /* Make a copy of command.
Otherwise there's a race between the caller and load(). */ Otherwise there's a race between the caller and load(). */
@@ -100,7 +105,14 @@ process_execute (const char *cmd)
tid = thread_create (file_name, PRI_DEFAULT, start_process, data); tid = thread_create (file_name, PRI_DEFAULT, start_process, data);
if (tid == TID_ERROR) if (tid == TID_ERROR)
palloc_free_page (cmd_copy); palloc_free_page (cmd_copy);
else
{
sema_down (&data->loaded);
if (!data->success)
tid = TID_ERROR;
}
free (data);
return tid; return tid;
} }
@@ -132,6 +144,11 @@ start_process (void *proc_start_data)
/* Prevent writing to the file being executed. */ /* Prevent writing to the file being executed. */
struct file *exec_file = filesys_open (data->file_name); struct file *exec_file = filesys_open (data->file_name);
if (exec_file == NULL)
{
lock_release (&filesys_lock);
goto fail;
}
thread_current ()->exec_file = exec_file; thread_current ()->exec_file = exec_file;
file_deny_write (exec_file); file_deny_write (exec_file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
@@ -153,10 +170,11 @@ start_process (void *proc_start_data)
/* If stack initialization failed, free resources and quit. */ /* If stack initialization failed, free resources and quit. */
if (!success) if (!success)
{ {
process_exit ();
goto fail; goto fail;
} }
data->success = true;
sema_up (&data->loaded);
/* Start the user process by simulating a return from an /* Start the user process by simulating a return from an
interrupt, implemented by intr_exit (in interrupt, implemented by intr_exit (in
threads/intr-stubs.S). Because intr_exit takes all of its threads/intr-stubs.S). Because intr_exit takes all of its
@@ -166,9 +184,10 @@ start_process (void *proc_start_data)
asm volatile ("movl %0, %%esp; jmp intr_exit" : : "g" (&if_) : "memory"); asm volatile ("movl %0, %%esp; jmp intr_exit" : : "g" (&if_) : "memory");
NOT_REACHED (); NOT_REACHED ();
/* If starting the process failed, free its common resources and exit. */ /* If starting the process failed, exit. */
fail: fail:
free (data); data->success = false;
sema_up (&data->loaded);
thread_exit (); thread_exit ();
} }
@@ -325,10 +344,14 @@ process_exit (void)
uint32_t *pd; uint32_t *pd;
printf ("%s: exit(%d)\n", cur->name, cur->exit_status); printf ("%s: exit(%d)\n", cur->name, cur->exit_status);
/* Clean up all open files */
hash_destroy (&cur->open_files, fd_cleanup);
/* Close the executable file. */
if (cur->exec_file != NULL) if (cur->exec_file != NULL)
{ {
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
file_allow_write (cur->exec_file);
file_close (cur->exec_file); file_close (cur->exec_file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
} }
@@ -356,10 +379,11 @@ process_exit (void)
/* Free child process results or signal parent's death. */ /* Free child process results or signal parent's death. */
struct list_elem *e; struct list_elem *e;
for (e = list_begin (&cur->child_results); for (e = list_begin (&cur->child_results);
e != list_end (&cur->child_results); e = list_next (e)) e != list_end (&cur->child_results);)
{ {
struct process_result *result struct process_result *result
= list_entry (e, struct process_result, elem); = list_entry (e, struct process_result, elem);
struct list_elem *next = list_next (e);
lock_acquire (&result->lock); lock_acquire (&result->lock);
/* Child has died (and was not waited for). Free the result. */ /* Child has died (and was not waited for). Free the result. */
if (sema_try_down (&result->sema)) if (sema_try_down (&result->sema))
@@ -373,6 +397,7 @@ process_exit (void)
sema_up (&result->sema); sema_up (&result->sema);
lock_release (&result->lock); lock_release (&result->lock);
} }
e = next;
} }
/* Destroy the current process's page directory and switch back /* Destroy the current process's page directory and switch back

View File

@@ -13,6 +13,9 @@
#include <stdio.h> #include <stdio.h>
#include <syscall-nr.h> #include <syscall-nr.h>
#define MAX_SYSCALL_ARGS 3
#define EXIT_FAILURE -1
static unsigned fd_counter = MIN_USER_FD; static unsigned fd_counter = MIN_USER_FD;
struct open_file struct open_file
@@ -45,18 +48,19 @@ static unsigned syscall_tell (int fd);
static void syscall_close (int fd); static void syscall_close (int fd);
static struct open_file *fd_get_file (int fd); static struct open_file *fd_get_file (int fd);
static void *validate_user_pointer (const void *ptr, size_t size); static void validate_user_pointer (const void *start, size_t size);
static void validate_user_string (const char *str);
/* A struct defining a syscall_function pointer along with its arity. */ /* A struct defining a syscall_function pointer along with its arity. */
typedef struct struct syscall_arguments
{ {
syscall_function function; /* Function pointer. */ syscall_function function; /* Function pointer. */
int arity; /* Number of arguments of the function. */ int arity; /* Number of arguments of the function. */
} syscall_arguments; };
/* A look-up table mapping numbers to system call functions with their number of /* A look-up table mapping numbers to system call functions with their number of
arguments. */ arguments. */
static const syscall_arguments syscall_lookup[] = static const struct syscall_arguments syscall_lookup[] =
{ {
[SYS_HALT] = {(syscall_function) syscall_halt, 0}, [SYS_HALT] = {(syscall_function) syscall_halt, 0},
[SYS_EXIT] = {(syscall_function) syscall_exit, 1}, [SYS_EXIT] = {(syscall_function) syscall_exit, 1},
@@ -76,8 +80,7 @@ static const syscall_arguments syscall_lookup[] =
/* The number of syscall functions (i.e, number of elements) within the /* The number of syscall functions (i.e, number of elements) within the
syscall_lookup table. */ syscall_lookup table. */
static const int LOOKUP_SIZE static const int LOOKUP_SIZE
= sizeof (syscall_lookup) / sizeof (syscall_arguments); = sizeof (syscall_lookup) / sizeof (struct syscall_arguments);
/* Initialises the syscall handling system, as well as a global lock to /* Initialises the syscall handling system, as well as a global lock to
synchronise all file access between processes. */ synchronise all file access between processes. */
@@ -88,28 +91,29 @@ syscall_init (void)
lock_init (&filesys_lock); lock_init (&filesys_lock);
} }
/* Function that takes a interrupt frame containing a syscall and its args. /* Function that takes an interrupt frame containing a syscall and its args.
Validates the arguments and pointers before calling the relevant Validates the arguments and pointers before calling the relevant
high-level system call function, storing its output (if any) in f->eax */ high-level system call function, storing its output (if any) in f->eax */
static void static void
syscall_handler (struct intr_frame *f) syscall_handler (struct intr_frame *f)
{ {
/* First, read the system call number from the stack. */ /* First, read the system call number from the stack. */
validate_user_pointer (f->esp, 1); validate_user_pointer (f->esp, sizeof (uintptr_t));
unsigned syscall_number = *(int *) f->esp; uintptr_t syscall_number = *(int *) f->esp;
/* Ensures the number corresponds to a system call that can be handled. */ /* Ensures the number corresponds to a system call that can be handled. */
if (syscall_number >= LOOKUP_SIZE) if (syscall_number >= LOOKUP_SIZE)
thread_exit (); syscall_exit (EXIT_FAILURE);
syscall_arguments syscall = syscall_lookup[syscall_number]; struct syscall_arguments syscall = syscall_lookup[syscall_number];
/* Next, read and copy the arguments from the stack pointer. */ /* Next, read and copy the arguments from the stack pointer. */
validate_user_pointer (f->esp + sizeof (uintptr_t), validate_user_pointer (f->esp + sizeof (uintptr_t),
syscall.arity * sizeof (uintptr_t)); syscall.arity * sizeof (uintptr_t));
uintptr_t args[3] = {0};
for (int i=0; i < syscall.arity; i++) uintptr_t args[MAX_SYSCALL_ARGS] = {0};
args[i] = *(uintptr_t *) (f->esp + sizeof (uintptr_t) * (i + 1)); for (int i = 0; i < syscall.arity && i < MAX_SYSCALL_ARGS; i++)
args[i] = *(uintptr_t *) (f->esp + sizeof (uintptr_t) * (i + 1));
/* Call the function that handles this system call with the arguments. When /* Call the function that handles this system call with the arguments. When
there is a return value it is stored in f->eax. */ there is a return value it is stored in f->eax. */
@@ -134,17 +138,13 @@ syscall_exit (int status)
} }
/* Executes a given command with the relevant args, by calling process_execute. /* Executes a given command with the relevant args, by calling process_execute.
Acquires the filesystem lock as process_execute accesses the file system. Returns PID for the process that is running the CMD_LINE. */
Returns PID for the process that is running the CMD_LINE
*/
static pid_t static pid_t
syscall_exec (const char *cmd_line) syscall_exec (const char *cmd_line)
{ {
validate_user_pointer (cmd_line, 1); validate_user_string (cmd_line);
pid_t pid = process_execute(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 /* Handles the syscall of wait. Effectively a wrapper for process_wait as the
@@ -152,16 +152,16 @@ syscall_exec (const char *cmd_line)
static int static int
syscall_wait (pid_t pid) syscall_wait (pid_t pid)
{ {
return process_wait (pid); return process_wait (pid); /* Returns the exit status of the waited process */
} }
/* Handles the syscall for file creation. First validates the user file /* Handles the syscall for file creation. First validates the user file
pointer. Acquires the file system lock to prevent synchronisation issues, pointer. Acquires the file system lock to prevent synchronisation issues,
and then uses FILESYS_CREATE to create the file, returning the same status */ and then uses FILESYS_CREATE to create the file, returning the same status */
static bool static bool
syscall_create (const char *file UNUSED, unsigned initial_size UNUSED) syscall_create (const char *file, unsigned initial_size)
{ {
validate_user_pointer (file, 1); validate_user_string (file);
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
bool status = filesys_create (file, initial_size); bool status = filesys_create (file, initial_size);
@@ -176,7 +176,7 @@ syscall_create (const char *file UNUSED, unsigned initial_size UNUSED)
static bool static bool
syscall_remove (const char *file) syscall_remove (const char *file)
{ {
validate_user_pointer (file, 1); validate_user_string (file);
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
bool status = filesys_remove (file); bool status = filesys_remove (file);
@@ -192,20 +192,23 @@ syscall_remove (const char *file)
static int static int
syscall_open (const char *file) syscall_open (const char *file)
{ {
validate_user_pointer (file, 1); validate_user_string (file);
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
struct file *ptr = filesys_open (file); struct file *ptr = filesys_open (file);
lock_release (&filesys_lock); lock_release (&filesys_lock);
if (ptr == NULL) if (ptr == NULL)
return -1; return EXIT_FAILURE;
/* Allocate space for a struct representing a mapping from an FD to a struct /* Allocate space for a struct representing a mapping from an FD to a struct
file. */ file. */
struct open_file *file_info struct open_file *file_info
= (struct open_file*) malloc (sizeof (struct open_file)); = (struct open_file*) malloc (sizeof (struct open_file));
if (file_info == NULL) if (file_info == NULL)
return -1; {
file_close (ptr);
return EXIT_FAILURE;
}
/* Populate the above struct, with a unique FD and the current open file */ /* Populate the above struct, with a unique FD and the current open file */
file_info->fd = fd_counter++; file_info->fd = fd_counter++;
@@ -226,7 +229,7 @@ syscall_filesize (int fd)
{ {
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL) if (file_info == NULL)
return -1; return EXIT_FAILURE;
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
int bytes = file_length (file_info->file); int bytes = file_length (file_info->file);
@@ -245,7 +248,7 @@ syscall_read (int fd, void *buffer, unsigned size)
/* Only console (fd = 0) or other files, not including STDOUT, (fd > 1) are /* Only console (fd = 0) or other files, not including STDOUT, (fd > 1) are
allowed. */ allowed. */
if (fd < 0 || fd == STDOUT_FILENO) if (fd < 0 || fd == STDOUT_FILENO)
return -1; return EXIT_FAILURE;
validate_user_pointer (buffer, size); validate_user_pointer (buffer, size);
@@ -263,7 +266,7 @@ syscall_read (int fd, void *buffer, unsigned size)
/* Reading from a file. */ /* Reading from a file. */
struct open_file *file_info = fd_get_file (fd); struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL) if (file_info == NULL)
return -1; return EXIT_FAILURE;
lock_acquire (&filesys_lock); lock_acquire (&filesys_lock);
int bytes_written = file_read (file_info->file, buffer, size); int bytes_written = file_read (file_info->file, buffer, size);
@@ -378,6 +381,20 @@ fd_less (const struct hash_elem *a_, const struct hash_elem *b_,
return a->fd < b->fd; return a->fd < b->fd;
} }
/* Function to clean up an open file entry. Closes the file and frees the
associated memory. */
void
fd_cleanup (struct hash_elem *e, void *aux UNUSED)
{
struct open_file *file_info = hash_entry (e, struct open_file, elem);
lock_acquire (&filesys_lock);
file_close (file_info->file);
lock_release (&filesys_lock);
free (file_info);
}
/* Gets a file from its descriptor (FD number). If there is no file with the fd /* Gets a file from its descriptor (FD number). If there is no file with the fd
FD it returns NULL. */ FD it returns NULL. */
static struct open_file * static struct open_file *
@@ -397,18 +414,57 @@ fd_get_file (int fd)
return hash_entry (e, struct open_file, elem); return hash_entry (e, struct open_file, elem);
} }
/* Validates if a block of memory starting at PTR and of size SIZE bytes is /* 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 calling fully contained within user virtual memory. Kills the thread (by exiting with
thread_exit) if the memory is invalid. Otherwise, returns the PTR given. failure) if the memory is invalid. Otherwise, returns (nothing) normally.
If the size is 0, the function does no checks and returns PTR.*/ If the size is 0, the function does no checks and returns the given ptr. */
static void * static void
validate_user_pointer (const void *ptr, size_t size) validate_user_pointer (const void *start, size_t size)
{ {
if (size > 0 && (ptr == NULL || if (size == 0)
!is_user_vaddr (ptr) || return;
!is_user_vaddr (ptr + size - 1) ||
pagedir_get_page (thread_current()->pagedir, ptr) == NULL))
thread_exit ();
return (void *) ptr; const void *end = start + size - 1;
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 = start; ptr <= end; ptr += PGSIZE)
if (pagedir_get_page (thread_current ()->pagedir, ptr) == NULL)
syscall_exit (EXIT_FAILURE);
}
/* 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. */
static void
validate_user_string (const char *str)
{
if (str == NULL || !is_user_vaddr (str))
syscall_exit (EXIT_FAILURE);
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);
if (!is_user_vaddr(page) ||
pagedir_get_page (thread_current ()->pagedir, page) == NULL)
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. */
}
} }

View File

@@ -14,5 +14,6 @@ void syscall_init (void);
unsigned fd_hash (const struct hash_elem *element, void *aux); unsigned fd_hash (const struct hash_elem *element, void *aux);
bool fd_less (const struct hash_elem *a, const struct hash_elem *b, void *aux); bool fd_less (const struct hash_elem *a, const struct hash_elem *b, void *aux);
void fd_cleanup (struct hash_elem *e, void *aux);
#endif /* userprog/syscall.h */ #endif /* userprog/syscall.h */