Skip to content

Commit

Permalink
task/schedule: Split up create_user_task()
Browse files Browse the repository at this point in the history
The create_user_task() function did the task creation and already put
the new task on the global TASKLIST and the cpu run-queue.

This is problematic because the task returned by create_user_task() is
not ready to run yet. When the ELF loading fails and exec_user()
returns an error, the task remains on the run-queue and TASKLIST. The
next call to schedule() will then try to run it, which can cause
undefined behavior in user-space.

So split the function up and only add a new user task to the run-queue
when it is fully initialized.

Signed-off-by: Joerg Roedel <[email protected]>
  • Loading branch information
joergroedel committed Nov 27, 2024
1 parent 5cdac82 commit 3ba293f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
12 changes: 11 additions & 1 deletion kernel/src/task/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::error::SvsmError;
use crate::fs::open;
use crate::mm::vm::VMFileMappingFlags;
use crate::mm::USER_MEM_END;
use crate::task::{create_user_task, current_task, schedule};
use crate::task::{create_user_task, current_task, finish_user_task, schedule};
use crate::types::PAGE_SIZE;
use crate::utils::align_up;
use elf::{Elf64File, Elf64PhdrFlags};
Expand All @@ -28,6 +28,15 @@ fn convert_elf_phdr_flags(flags: Elf64PhdrFlags) -> VMFileMappingFlags {
vm_flags
}

/// Loads and executes an ELF binary in user-mode.
///
/// # Arguments
///
/// * binary: Path to file in the file-system
///
/// # Returns
///
/// `()` on success, [`SvsmError`] on failure.
pub fn exec_user(binary: &str) -> Result<(), SvsmError> {
let fh = open(binary)?;
let file_size = fh.size();
Expand Down Expand Up @@ -88,6 +97,7 @@ pub fn exec_user(binary: &str) -> Result<(), SvsmError> {
let stack_addr = USER_MEM_END - user_stack_size;
task.mmap_user(stack_addr, None, 0, user_stack_size, stack_flags)?;

finish_user_task(task);
schedule();

Ok(())
Expand Down
4 changes: 2 additions & 2 deletions kernel/src/task/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ mod tasks;
mod waiting;

pub use schedule::{
create_user_task, current_task, current_task_terminated, is_current_task, schedule,
schedule_init, schedule_task, start_kernel_task, terminate, RunQueue, TASKLIST,
create_user_task, current_task, current_task_terminated, finish_user_task, is_current_task,
schedule, schedule_init, schedule_task, start_kernel_task, terminate, RunQueue, TASKLIST,
};

pub use tasks::{
Expand Down
26 changes: 22 additions & 4 deletions kernel/src/task/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,33 @@ pub fn start_kernel_task(entry: extern "C" fn()) -> Result<TaskPointer, SvsmErro
Ok(task)
}

/// Creates and initializes the kernel state of a new user task. The task is
/// not added to the TASKLIST or run-queue yet.
///
/// # Arguments
///
/// * user_entry: The user-space entry point.
///
/// # Returns
///
/// A new instance of [`TaskPointer`] on success, [`SvsmError`] on failure.
pub fn create_user_task(user_entry: usize) -> Result<TaskPointer, SvsmError> {
let cpu = this_cpu();
let task = Task::create_user(cpu, user_entry)?;
Task::create_user(cpu, user_entry)
}

/// Finished user-space task creation by putting the task on the global
/// TASKLIST and adding it to the run-queue.
///
/// # Arguments
///
/// * task: Pointer to user task
pub fn finish_user_task(task: TaskPointer) {
// Add task to global TASKLIST
TASKLIST.lock().list().push_back(task.clone());

// Put task on the runqueue of this CPU
cpu.runqueue().lock_write().handle_task(task.clone());

Ok(task)
this_cpu().runqueue().lock_write().handle_task(task);
}

pub fn current_task() -> TaskPointer {
Expand Down

0 comments on commit 3ba293f

Please sign in to comment.