Skip to content

Fix ARM64 user stack frame mapping #404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ryanbreen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix ARM64 user stack frame mapping #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions kernel/src/memory/process_memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2319,20 +2319,29 @@ pub fn map_user_stack_to_process_with_phys(
process_page_table: &mut ProcessPageTable,
user_stack_bottom: VirtAddr,
user_stack_top: VirtAddr,
phys_bottom: u64,
physical_frames: &[u64],
) -> Result<(), &'static str> {
use crate::memory::arch_stub::{Page, PageTableFlags, PhysAddr, PhysFrame, Size4KiB};

let stack_size = user_stack_top.as_u64() - user_stack_bottom.as_u64();
let num_pages = stack_size / 4096;
let num_pages = (stack_size / 4096) as usize;

if physical_frames.len() != num_pages {
log::error!(
"Stack frame count mismatch: stack pages={} physical_frames={}",
num_pages,
physical_frames.len()
);
return Err("Stack frame count mismatch");
}

let flags =
PageTableFlags::PRESENT | PageTableFlags::WRITABLE | PageTableFlags::USER_ACCESSIBLE;

for i in 0..num_pages {
let page_offset = i * 4096;
for (i, frame_phys) in physical_frames.iter().copied().enumerate() {
let page_offset = (i as u64) * 4096;
let user_vaddr = VirtAddr::new(user_stack_bottom.as_u64() + page_offset);
let phys_addr = PhysAddr::new(phys_bottom + page_offset);
let phys_addr = PhysAddr::new(frame_phys);
let page = Page::<Size4KiB>::containing_address(user_vaddr);
let frame = PhysFrame::<Size4KiB>::containing_address(phys_addr);

Expand Down
38 changes: 26 additions & 12 deletions kernel/src/memory/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ pub struct GuardedStack {
/// Privilege level of the stack
#[allow(dead_code)]
privilege: ThreadPrivilege,
/// Physical frames backing each stack page, in ascending virtual-page order.
#[cfg(target_arch = "aarch64")]
physical_frames: alloc::vec::Vec<u64>,
}

impl GuardedStack {
Expand Down Expand Up @@ -87,6 +90,8 @@ impl GuardedStack {
stack_top,
stack_size,
privilege,
#[cfg(target_arch = "aarch64")]
physical_frames: alloc::vec::Vec::new(),
})
}

Expand Down Expand Up @@ -128,6 +133,11 @@ impl GuardedStack {
self.stack_size
}

#[cfg(target_arch = "aarch64")]
pub fn physical_frames(&self) -> &[u64] {
&self.physical_frames
}

/// Find free virtual address space for stack allocation
///
/// This function uses a simple incrementing allocator. The bounds checking is
Expand Down Expand Up @@ -327,19 +337,12 @@ pub fn allocate_stack_with_privilege(

// Allocate physical frames for the stack
// We track all frames and verify they're contiguous
let mut first_frame_phys: Option<u64> = None;
let mut prev_frame_phys: Option<u64> = None;
let mut frames = alloc::vec::Vec::with_capacity(stack_pages);

for i in 0..stack_pages {
for _ in 0..stack_pages {
let frame = allocate_frame().ok_or("out of memory for stack")?;
let phys = frame.start_address().as_u64();

if i == 0 {
first_frame_phys = Some(phys);
}
// Note: We don't log non-contiguous frames here - just accept them
// For simple stacks this works fine even if frames aren't contiguous
prev_frame_phys = Some(phys);
frames.push(phys);

// Zero the frame via HHDM
let virt = HHDM_BASE + phys;
Expand All @@ -348,8 +351,18 @@ pub fn allocate_stack_with_privilege(
}
}

let stack_phys = first_frame_phys.ok_or("no frames allocated")?;
let last_frame_phys = prev_frame_phys.ok_or("no frames allocated")?;
let stack_phys = *frames.first().ok_or("no frames allocated")?;
let last_frame_phys = *frames.last().ok_or("no frames allocated")?;

let contiguous = frames
.windows(2)
.all(|pair| pair[1] == pair[0].saturating_add(4096));
if !contiguous {
crate::serial_println!(
"ARM64 stack allocator: non-contiguous physical frames: {:x?}",
frames
);
}

// Calculate virtual addresses via HHDM
// Layout: [guard page][stack pages...]
Expand All @@ -364,6 +377,7 @@ pub fn allocate_stack_with_privilege(
stack_top,
stack_size: size,
privilege,
physical_frames: frames,
})
}

Expand Down
35 changes: 9 additions & 26 deletions kernel/src/process/manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,18 +435,6 @@ impl ProcessManager {
"Failed to allocate user stack"
})?;

// The kernel_stack.top() is an HHDM address - extract physical address
let kernel_stack_top = kernel_stack.top().as_u64();
let hhdm_base = crate::arch_impl::aarch64::constants::HHDM_BASE;
let stack_phys_top = kernel_stack_top - hhdm_base;
let stack_phys_bottom = stack_phys_top - USER_STACK_SIZE as u64;

crate::serial_println!(
"manager.create_process [ARM64]: Stack physical range {:#x}-{:#x}",
stack_phys_bottom,
stack_phys_top
);

// Calculate userspace stack addresses
// Stack grows down, so stack_top is the highest address
let user_stack_top = USER_STACK_REGION_START;
Expand All @@ -462,28 +450,25 @@ impl ProcessManager {
process.user_stack_top = user_stack_top;
process.user_stack_bottom = user_stack_bottom;

// Store the kernel-accessible stack (for potential kernel access later)
process.stack = Some(Box::new(kernel_stack));

// Map the physical stack frames into the process page table at USERSPACE addresses
log::debug!("ARM64: Mapping user stack pages into process page table...");
crate::serial_println!(
"manager.create_process [ARM64]: Mapping user stack into process page table"
);
let initial_tpidr_el0 = if let Some(ref mut page_table) = process.page_table {
crate::serial_println!(
"manager.create_process [ARM64]: map_user_stack_to_process user_bottom={:#x} user_top={:#x} phys_bottom={:#x}",
"manager.create_process [ARM64]: map_user_stack_to_process user_bottom={:#x} user_top={:#x} frames={}",
user_stack_bottom,
user_stack_top,
stack_phys_bottom
kernel_stack.physical_frames().len()
);

// Map physical frames to userspace addresses
crate::memory::process_memory::map_user_stack_to_process_with_phys(
page_table,
VirtAddr::new(user_stack_bottom),
VirtAddr::new(user_stack_top),
stack_phys_bottom,
kernel_stack.physical_frames(),
)
.map_err(|e| {
crate::serial_println!(
Expand All @@ -506,6 +491,9 @@ impl ProcessManager {
return Err("Process page table not available for stack mapping");
};

// Store the kernel-accessible stack (for potential kernel access later)
process.stack = Some(Box::new(kernel_stack));

// Create the main thread with USERSPACE stack top
crate::serial_println!("manager.create_process [ARM64]: Creating main thread");
let user_stack_top_vaddr = VirtAddr::new(user_stack_top);
Expand Down Expand Up @@ -633,12 +621,6 @@ impl ProcessManager {
stack::allocate_stack_with_privilege(USER_STACK_SIZE, StackPrivilege::User)
.map_err(|_| "Failed to allocate user stack")?;

// Extract physical address from HHDM address
let kernel_stack_top = kernel_stack.top().as_u64();
let hhdm_base = crate::arch_impl::aarch64::constants::HHDM_BASE;
let stack_phys_top = kernel_stack_top - hhdm_base;
let stack_phys_bottom = stack_phys_top - USER_STACK_SIZE as u64;

// Calculate userspace stack addresses
let user_stack_top = USER_STACK_REGION_START;
let user_stack_bottom = user_stack_top - USER_STACK_SIZE as u64;
Expand All @@ -652,15 +634,14 @@ impl ProcessManager {
process.memory_usage.stack_size = USER_STACK_SIZE;
process.user_stack_top = user_stack_top;
process.user_stack_bottom = user_stack_bottom;
process.stack = Some(Box::new(kernel_stack));

// Map the physical stack frames into the process page table
let initial_tpidr_el0 = if let Some(ref mut page_table) = process.page_table {
crate::memory::process_memory::map_user_stack_to_process_with_phys(
page_table,
VirtAddr::new(user_stack_bottom),
VirtAddr::new(user_stack_top),
stack_phys_bottom,
kernel_stack.physical_frames(),
)
.map_err(|e| {
log::error!(
Expand All @@ -675,6 +656,8 @@ impl ProcessManager {
return Err("Process page table not available for stack mapping");
};

process.stack = Some(Box::new(kernel_stack));

// Set up argc/argv/envp/auxv on the stack following Linux ABI
// The stack is now mapped, so we can write to it via physical addresses
let default_env: [&[u8]; 5] = [
Expand Down