Skip to content

aarch64: Restrict address mappings in loader.rs #464

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
bruelc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

aarch64: Restrict address mappings in loader.rs #464

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
1 change: 1 addition & 0 deletions loader/src/aarch64/mmu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ uint64_t boot_lvl2_upper[1 << 9] ALIGN(1 << 12);
/* Paging structures for identity mapping */
uint64_t boot_lvl0_lower[1 << 9] ALIGN(1 << 12);
uint64_t boot_lvl1_lower[1 << 9] ALIGN(1 << 12);
uint64_t boot_lvl2_lower[1 << 9] ALIGN(1 << 12);

int arch_mmu_enable(int logical_cpu)
{
Expand Down
2 changes: 2 additions & 0 deletions loader/src/uart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ void putc(uint8_t ch)
#error Board not defined
#endif

uint32_t *uart_addr = (uint32_t *)UART_BASE;

void puts(const char *s)
{
while (*s) {
Expand Down
52 changes: 49 additions & 3 deletions tool/microkit/src/loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,20 +485,65 @@ impl<'a> Loader<'a> {
let (boot_lvl0_upper_addr, boot_lvl0_upper_size) = elf
.find_symbol("boot_lvl0_upper")
.expect("Could not find 'boot_lvl0_upper' symbol");
let (boot_lvl2_lower_addr, boot_lvl2_lower_size) = elf
.find_symbol("boot_lvl2_lower")
.expect("Could not find 'boot_lvl2_lower' symbol");

let (text_addr, _) = elf
.find_symbol("_text")
.expect("Could not find 'text' symbol");
let (end_addr, _) = elf
.find_symbol("_bss_end")
.expect("Could not find 'end' symbol");

Copy link
Copy Markdown
Contributor

@midnightveil midnightveil Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a brief comment just saying what this is doing.

Specifically: it's mapping in just the loader itself; not all of RAM.

It might make sense to add extra linker script symbols for 'loader_start' and 'loader_end' rather than relying on text/bss end implicitly being at start/end.

Also text_addr -> start_addr

if Aarch64::lvl1_index(text_addr) != Aarch64::lvl1_index(end_addr) {
eprintln!("ERROR: We only map 1GiB, but elfloader paddr range covers multiple GiB");
Copy link
Copy Markdown
Contributor

@midnightveil midnightveil Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we tend to use panic!("INTERNAL: <message>") here.

I'd also reword this: specifically the issue is the microkit loader (not elfloader) crosses a GiB boundary [or really that it requires multiple level 1 page tables].

It likely makes sense to do this change to both AArch64 and Riscv64, no sense it being different for either.

std::process::exit(1);
}

let mut boot_lvl0_lower: [u8; PAGE_TABLE_SIZE] = [0; PAGE_TABLE_SIZE];
boot_lvl0_lower[..8].copy_from_slice(&(boot_lvl1_lower_addr | 3).to_le_bytes());

let mut boot_lvl1_lower: [u8; PAGE_TABLE_SIZE] = [0; PAGE_TABLE_SIZE];
for i in 0..512 {

// map optional UART MMIO in l1 1GB page, only available if CONFIG_PRINTING
if let Ok((uart_addr, _)) = elf.find_symbol("uart_addr") {
let data = elf
.get_data(uart_addr, 8)
.expect("uart_addr not initialized");
let uart_base = u64::from_le_bytes(data[0..8].try_into().unwrap());
Comment on lines +510 to +514
Copy link
Copy Markdown
Contributor

@midnightveil midnightveil Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Not something you need to fix, just making this comment so I can link to it later):

It'd be nice if the kernel exposed the virtual address of where it wants its UART so we can map that in and get this feature working again.


let lvl1_idx = Aarch64::lvl1_index(uart_base);
#[allow(clippy::identity_op)] // keep the (0 << 2) for clarity
let pt_entry: u64 = ((i as u64) << AARCH64_1GB_BLOCK_BITS) |
let pt_entry: u64 = ((lvl1_idx as u64) << AARCH64_1GB_BLOCK_BITS) |
(1 << 10) | // access flag
(0 << 2) | // strongly ordered memory
(1); // 1G block
let start = 8 * lvl1_idx;
let end = 8 * (lvl1_idx + 1);
boot_lvl1_lower[start..end].copy_from_slice(&pt_entry.to_le_bytes());
}

let mut boot_lvl2_lower: [u8; PAGE_TABLE_SIZE] = [0; PAGE_TABLE_SIZE];

let pt_entry = (boot_lvl2_lower_addr | 3).to_le_bytes();
Copy link
Copy Markdown
Contributor

@midnightveil midnightveil Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we be consistent with the other examples and split out the meanings of the 3 flags into the individual bits?

let lvl1_idx = Aarch64::lvl1_index(text_addr);
let start = 8 * lvl1_idx;
let end = 8 * (lvl1_idx + 1);
boot_lvl1_lower[start..end].copy_from_slice(&pt_entry);

let lvl2_idx = Aarch64::lvl2_index(text_addr);

for i in lvl2_idx ..= Aarch64::lvl2_index(end_addr) {
let entry_idx = (i - Aarch64::lvl2_index(text_addr)) << AARCH64_2MB_BLOCK_BITS;
let pt_entry: u64 = (entry_idx as u64 + text_addr) |
(1 << 10) | // access flag
(3 << 8) | // inner sharable
(3 << 2) | // MT_NORMAL memory
Copy link
Copy Markdown
Contributor

@midnightveil midnightveil Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment here and elsewhere is off

(1 << 0); // 2M block
let start = 8 * i;
let end = 8 * (i + 1);
boot_lvl1_lower[start..end].copy_from_slice(&pt_entry.to_le_bytes());
boot_lvl2_lower[start..end].copy_from_slice(&pt_entry.to_le_bytes());
}

let boot_lvl0_upper: [u8; PAGE_TABLE_SIZE] = [0; PAGE_TABLE_SIZE];
Expand Down Expand Up @@ -536,6 +581,7 @@ impl<'a> Loader<'a> {
(boot_lvl0_upper_addr, boot_lvl0_upper_size, boot_lvl0_upper),
(boot_lvl1_upper_addr, boot_lvl1_upper_size, boot_lvl1_upper),
(boot_lvl2_upper_addr, boot_lvl2_upper_size, boot_lvl2_upper),
(boot_lvl2_lower_addr, boot_lvl2_lower_size, boot_lvl2_lower),
]
}
}
Loading