Fix race condition in the passing of data from thread executing process_execute to its child

This commit is contained in:
Themis Demetriades
2024-11-11 21:51:38 +00:00
parent 049fc5559c
commit 52fdd47e0c

View File

@@ -43,7 +43,8 @@ struct process_start_data
char *cmd_saveptr; /* Value pointed to by 'saveptr' argument used by char *cmd_saveptr; /* Value pointed to by 'saveptr' argument used by
successive calls to strtok_r to split 'cmd' into successive calls to strtok_r to split 'cmd' into
tokens while maintaining state. */ tokens while maintaining state. */
char *file_name; /* Name of the file of the process to be started. */ char file_name[FNAME_MAX_LEN]; /* Name of the file of the process to
be started. */
}; };
static thread_func start_process NO_RETURN; static thread_func start_process NO_RETURN;
@@ -58,7 +59,12 @@ process_execute (const char *cmd)
{ {
char *cmd_copy; char *cmd_copy;
tid_t tid; tid_t tid;
struct process_start_data data;
struct process_start_data *data = malloc (sizeof (struct process_start_data));
if (data == NULL)
{
return TID_ERROR;
}
/* 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(). */
@@ -72,17 +78,15 @@ process_execute (const char *cmd)
/* Retrieve first argument of command, which is the file name /* Retrieve first argument of command, which is the file name
of the process. */ of the process. */
char *file_name = strtok_r (cmd_copy, " ", &data.cmd_saveptr); char *file_name = strtok_r (cmd_copy, " ", &data->cmd_saveptr);
if (strlen (file_name) > FNAME_MAX_LEN)
file_name[FNAME_MAX_LEN + 1] = '\n';
/* Create a new thread to execute the command, by initializing /* Create a new thread to execute the command, by initializing
it running the function 'start_process' with the appropriate it running the function 'start_process' with the appropriate
arguments. For details of arguments, see 'start_process'. */ arguments. For details of arguments, see 'start_process'. */
data.cmd = cmd_copy; data->cmd = cmd_copy;
data.file_name = file_name; strlcpy (data->file_name, file_name, FNAME_MAX_LEN);
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);
return tid; return tid;
@@ -105,7 +109,6 @@ start_process (void *proc_start_data)
bool success; bool success;
struct process_start_data *data = proc_start_data; struct process_start_data *data = proc_start_data;
ASSERT (data->cmd_saveptr != NULL);
/* Initialize interrupt frame and load executable. */ /* Initialize interrupt frame and load executable. */
memset (&if_, 0, sizeof if_); memset (&if_, 0, sizeof if_);
@@ -118,7 +121,7 @@ start_process (void *proc_start_data)
if (!success) if (!success)
{ {
palloc_free_page (data->cmd); palloc_free_page (data->cmd);
thread_exit (); goto fail;
} }
/* Initialize user process stack and free page used to store the /* Initialize user process stack and free page used to store the
@@ -130,7 +133,7 @@ start_process (void *proc_start_data)
if (!success) if (!success)
{ {
process_exit (); process_exit ();
thread_exit (); goto fail;
} }
/* Start the user process by simulating a return from an /* Start the user process by simulating a return from an
@@ -141,6 +144,11 @@ start_process (void *proc_start_data)
and jump to it. */ and jump to it. */
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. */
fail:
free (data);
thread_exit ();
} }
/* Helper function that initializes the stack of a newly created /* Helper function that initializes the stack of a newly created