Imperial-MEng/pintos_22
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'system-calls' into 'userprog-oom'

Browse Source 
Add Fixes to Memory Leaks, Memory Access Validation, Synchronised Processes and Refactoring

See merge request lab2425_autumn/pintos_22!38
This commit is contained in:
Gleb Koval
2024-11-13 19:20:19 +00:00
parent e7cb16b301 927c376d02
commit d02b956221
15 changed files with 208 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/userprog/process.c
View File

@@ -343,6 +343,11 @@ process_exit (void)
uint32_t *pd;
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)
{
lock_acquire (&filesys_lock);

142
src/userprog/syscall.c
View File

@@ -13,6 +13,9 @@
#include <stdio.h>
#include <syscall-nr.h>
#define MAX_SYSCALL_ARGS 3
#define EXIT_FAILURE -1
static unsigned fd_counter = MIN_USER_FD;
struct open_file
@@ -45,18 +48,19 @@ 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 *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. */
typedef struct
struct syscall_arguments
{
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 struct syscall_arguments syscall_lookup[] =
{
[SYS_HALT] = {(syscall_function) syscall_halt, 0},
[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
syscall_lookup table. */
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
synchronise all file access between processes. */
@@ -88,28 +91,29 @@ syscall_init (void)
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
high-level system call function, storing its output (if any) in f->eax */
static void
syscall_handler (struct intr_frame *f)
{
/* First, read the system call number from the stack. */
validate_user_pointer (f->esp, 1);
unsigned syscall_number = *(int *) f->esp;
validate_user_pointer (f->esp, sizeof (uintptr_t));
uintptr_t syscall_number = *(int *) f->esp;
/* Ensures the number corresponds to a system call that can be handled. */
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. */
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 + sizeof (uintptr_t) * (i + 1));
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));
/* Call the function that handles this system call with the arguments. When
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.
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
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 pid;
return process_execute (cmd_line); /* Returns the PID of the new process */
}
/* 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
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
pointer. Acquires the file system lock to prevent synchronisation issues,
and then uses FILESYS_CREATE to create the file, returning the same status */
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);
bool status = filesys_create (file, initial_size);
@@ -176,7 +176,7 @@ syscall_create (const char *file UNUSED, unsigned initial_size UNUSED)
static bool
syscall_remove (const char *file)
{
validate_user_pointer (file, 1);
validate_user_string (file);
lock_acquire (&filesys_lock);
bool status = filesys_remove (file);
@@ -192,20 +192,23 @@ syscall_remove (const char *file)
static int
syscall_open (const char *file)
{
validate_user_pointer (file, 1);
validate_user_string (file);
lock_acquire (&filesys_lock);
struct file *ptr = filesys_open (file);
lock_release (&filesys_lock);
if (ptr == NULL)
return -1;
return EXIT_FAILURE;
/* Allocate space for a struct representing a mapping from an FD to a struct
file. */
struct open_file *file_info
= (struct open_file*) malloc (sizeof (struct open_file));
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 */
file_info->fd = fd_counter++;
@@ -226,7 +229,7 @@ syscall_filesize (int fd)
{
struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL)
return -1;
return EXIT_FAILURE;
lock_acquire (&filesys_lock);
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
allowed. */
if (fd < 0 || fd == STDOUT_FILENO)
return -1;
return EXIT_FAILURE;
validate_user_pointer (buffer, size);
@@ -263,7 +266,7 @@ syscall_read (int fd, void *buffer, unsigned size)
/* Reading from a file. */
struct open_file *file_info = fd_get_file (fd);
if (file_info == NULL)
return -1;
return EXIT_FAILURE;
lock_acquire (&filesys_lock);
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;
}
/* 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
FD it returns NULL. */
static struct open_file *
@@ -397,18 +414,57 @@ fd_get_file (int fd)
return hash_entry (e, struct open_file, elem);
}
/* Validates if a block of memory starting at PTR and of size SIZE bytes is
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 (const void *ptr, size_t size)
/* 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. */
static void
validate_user_pointer (const void *start, size_t size)
{
if (size > 0 && (ptr == NULL ||
!is_user_vaddr (ptr) ||
!is_user_vaddr (ptr + size - 1) ||
pagedir_get_page (thread_current()->pagedir, ptr) == NULL))
thread_exit ();
if (size == 0)
return;
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. */
}
}

1
src/userprog/syscall.h
View File

@@ -14,5 +14,6 @@ void syscall_init (void);
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);
void fd_cleanup (struct hash_elem *e, void *aux);
#endif /* userprog/syscall.h */