From 0ab3b66ebe89343630658fdf12510a1a77c2aa5d Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Wed, 15 Dec 2021 16:49:21 +0100 Subject: [PATCH 1/9] add support for position independent executables --- Cargo.lock | 9 + Cargo.toml | 1 + src/binary/level_4_entries.rs | 11 +- src/binary/load_kernel.rs | 154 +++++++++++++++++- tests/pie.rs | 32 ++++ tests/test_kernels/pie/.cargo/config.toml | 10 ++ tests/test_kernels/pie/.gitignore | 1 + tests/test_kernels/pie/Cargo.toml | 10 ++ tests/test_kernels/pie/src/bin/basic_boot.rs | 21 +++ .../pie/src/bin/check_boot_info.rs | 68 ++++++++ .../pie/src/bin/global_variable.rs | 38 +++++ .../test_kernels/pie/src/bin/should_panic.rs | 18 ++ tests/test_kernels/pie/src/lib.rs | 27 +++ tests/test_kernels/pie/x86_64-pie.json | 16 ++ 14 files changed, 405 insertions(+), 11 deletions(-) create mode 100644 tests/pie.rs create mode 100644 tests/test_kernels/pie/.cargo/config.toml create mode 100644 tests/test_kernels/pie/.gitignore create mode 100644 tests/test_kernels/pie/Cargo.toml create mode 100644 tests/test_kernels/pie/src/bin/basic_boot.rs create mode 100644 tests/test_kernels/pie/src/bin/check_boot_info.rs create mode 100644 tests/test_kernels/pie/src/bin/global_variable.rs create mode 100644 tests/test_kernels/pie/src/bin/should_panic.rs create mode 100644 tests/test_kernels/pie/src/lib.rs create mode 100644 tests/test_kernels/pie/x86_64-pie.json diff --git a/Cargo.lock b/Cargo.lock index 287e4e8f..b255b139 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -380,6 +380,15 @@ dependencies = [ "x86_64", ] +[[package]] +name = "test_kernel_pie" +version = "0.1.0" +dependencies = [ + "bootloader", + "uart_16550", + "x86_64 0.13.6", +] + [[package]] name = "thiserror" version = "1.0.24" diff --git a/Cargo.toml b/Cargo.toml index 874cd738..991046e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ members = [ "tests/test_kernels/default_settings", "tests/test_kernels/map_phys_mem", "tests/test_kernels/higher_half", + "tests/test_kernels/pie", ] exclude = [ "examples/basic", diff --git a/src/binary/level_4_entries.rs b/src/binary/level_4_entries.rs index c231cae2..447e07a8 100644 --- a/src/binary/level_4_entries.rs +++ b/src/binary/level_4_entries.rs @@ -16,7 +16,10 @@ impl UsedLevel4Entries { /// Initializes a new instance from the given ELF program segments. /// /// Marks the virtual address range of all segments as used. - pub fn new<'a>(segments: impl Iterator>) -> Self { + pub fn new<'a>( + segments: impl Iterator>, + virtual_address_offset: u64, + ) -> Self { let mut used = UsedLevel4Entries { entry_state: [false; 512], }; @@ -24,9 +27,11 @@ impl UsedLevel4Entries { used.entry_state[0] = true; // TODO: Can we do this dynamically? for segment in segments { - let start_page: Page = Page::containing_address(VirtAddr::new(segment.virtual_addr())); + let start_page: Page = Page::containing_address(VirtAddr::new( + segment.virtual_addr() + virtual_address_offset, + )); let end_page: Page = Page::containing_address(VirtAddr::new( - segment.virtual_addr() + segment.mem_size(), + segment.virtual_addr() + virtual_address_offset + segment.mem_size(), )); for p4_index in u64::from(start_page.p4_index())..=u64::from(end_page.p4_index()) { diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 1e0870c5..dacaa3df 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -11,8 +11,9 @@ use x86_64::{ PhysAddr, VirtAddr, }; use xmas_elf::{ - header, - program::{self, ProgramHeader, Type}, + dynamic, header, + program::{self, ProgramHeader, SegmentData, Type}, + sections::Rela, ElfFile, }; @@ -23,6 +24,7 @@ struct Loader<'a, M, F> { struct Inner<'a, M, F> { kernel_offset: PhysAddr, + virtual_address_offset: u64, page_table: &'a mut M, frame_allocator: &'a mut F, } @@ -44,11 +46,22 @@ where } let elf_file = ElfFile::new(bytes)?; + + let virtual_address_offset = match elf_file.header.pt2.type_().as_type() { + header::Type::None => unimplemented!(), + header::Type::Relocatable => unimplemented!(), + header::Type::Executable => 0, + header::Type::SharedObject => 0x400000, + header::Type::Core => unimplemented!(), + header::Type::ProcessorSpecific(_) => unimplemented!(), + }; + header::sanity_check(&elf_file)?; let loader = Loader { elf_file, inner: Inner { kernel_offset, + virtual_address_offset, page_table, frame_allocator, }, @@ -58,9 +71,21 @@ where } fn load_segments(&mut self) -> Result, &'static str> { - let mut tls_template = None; for program_header in self.elf_file.program_iter() { program::sanity_check(program_header, &self.elf_file)?; + } + + // Apply relocations in physical memory. + for program_header in self.elf_file.program_iter() { + if let Type::Dynamic = program_header.get_type()? { + self.inner + .handle_dynamic_segment(program_header, &self.elf_file)? + } + } + + // Load the segments into virtual memory. + let mut tls_template = None; + for program_header in self.elf_file.program_iter() { match program_header.get_type()? { Type::Load => self.inner.handle_load_segment(program_header)?, Type::Tls => { @@ -85,11 +110,14 @@ where } fn entry_point(&self) -> VirtAddr { - VirtAddr::new(self.elf_file.header.pt2.entry_point()) + VirtAddr::new(self.elf_file.header.pt2.entry_point() + self.inner.virtual_address_offset) } fn used_level_4_entries(&self) -> UsedLevel4Entries { - UsedLevel4Entries::new(self.elf_file.program_iter()) + UsedLevel4Entries::new( + self.elf_file.program_iter(), + self.inner.virtual_address_offset, + ) } } @@ -106,7 +134,7 @@ where let end_frame: PhysFrame = PhysFrame::containing_address(phys_start_addr + segment.file_size() - 1u64); - let virt_start_addr = VirtAddr::new(segment.virtual_addr()); + let virt_start_addr = VirtAddr::new(segment.virtual_addr()) + self.virtual_address_offset; let start_page: Page = Page::containing_address(virt_start_addr); let mut segment_flags = Flags::PRESENT; @@ -146,7 +174,7 @@ where ) -> Result<(), &'static str> { log::info!("Mapping bss section"); - let virt_start_addr = VirtAddr::new(segment.virtual_addr()); + let virt_start_addr = VirtAddr::new(segment.virtual_addr()) + self.virtual_address_offset; let phys_start_addr = self.kernel_offset + segment.offset(); let mem_size = segment.mem_size(); let file_size = segment.file_size(); @@ -262,11 +290,121 @@ where fn handle_tls_segment(&mut self, segment: ProgramHeader) -> Result { Ok(TlsTemplate { - start_addr: segment.virtual_addr(), + start_addr: segment.virtual_addr() + self.virtual_address_offset, mem_size: segment.mem_size(), file_size: segment.file_size(), }) } + + fn handle_dynamic_segment( + &mut self, + segment: ProgramHeader, + elf_file: &ElfFile, + ) -> Result<(), &'static str> { + let data = segment.get_data(elf_file)?; + let data = if let SegmentData::Dynamic64(data) = data { + data + } else { + unreachable!() + }; + + // Find the `Rela`, `RelaSize` and `RelaEnt` entries. + let mut rela = None; + let mut rela_size = None; + let mut rela_ent = None; + for rel in data { + let tag = rel.get_tag()?; + match tag { + dynamic::Tag::Rela => { + let ptr = rel.get_ptr()?; + let prev = rela.replace(ptr); + if prev.is_some() { + return Err("Dynamic section contains more than one Rela entry"); + } + } + dynamic::Tag::RelaSize => { + let val = rel.get_val()?; + let prev = rela_size.replace(val); + if prev.is_some() { + return Err("Dynamic section contains more than one RelaSize entry"); + } + } + dynamic::Tag::RelaEnt => { + let val = rel.get_val()?; + let prev = rela_ent.replace(val); + if prev.is_some() { + return Err("Dynamic section contains more than one RelaEnt entry"); + } + } + _ => {} + } + } + let offset = if let Some(rela) = rela { + rela + } else { + // The section doesn't contain any relocations. + + assert_eq!(rela_size, None); + assert_eq!(rela_ent, None); + + return Ok(()); + }; + let total_size = rela_size.ok_or("RelaSize entry is missing")?; + let entry_size = rela_ent.ok_or("RelaEnt entry is missing")?; + + // Apply the mappings. + let entries = total_size / entry_size; + let relas = unsafe { + core::slice::from_raw_parts::>( + elf_file.input.as_ptr().add(offset as usize).cast(), + entries as usize, + ) + }; + for rela in relas { + let idx = rela.get_symbol_table_index(); + assert_eq!( + idx, 0, + "relocations using the symbol table are not supported" + ); + + match rela.get_type() { + 8 => { + let offset_in_file = find_offset(elf_file, rela.get_offset())? + .ok_or("Destination of relocation is not mapped in physical memory")?; + let dest_addr = self.kernel_offset + offset_in_file; + let dest_ptr = dest_addr.as_u64() as *mut u64; + + let value = self + .virtual_address_offset + .checked_add(rela.get_addend()) + .unwrap(); + + unsafe { + // write new value, utilizing that the address identity-mapped + dest_ptr.write(value); + } + } + ty => unimplemented!("relocation type {:x} not supported", ty), + } + } + + Ok(()) + } +} + +/// Locate the offset into the elf file corresponding to a virtual address. +fn find_offset(elf_file: &ElfFile, virt_offset: u64) -> Result, &'static str> { + for program_header in elf_file.program_iter() { + if let Type::Load = program_header.get_type()? { + if program_header.virtual_addr() <= virt_offset { + let offset_in_segment = virt_offset - program_header.virtual_addr(); + if offset_in_segment < program_header.file_size() { + return Ok(Some(program_header.offset() + offset_in_segment)); + } + } + } + } + Ok(None) } /// Loads the kernel ELF file given in `bytes` in the given `page_table`. diff --git a/tests/pie.rs b/tests/pie.rs new file mode 100644 index 00000000..565fb6e4 --- /dev/null +++ b/tests/pie.rs @@ -0,0 +1,32 @@ +use std::process::Command; + +#[test] +fn basic_boot() { + run_test_binary("basic_boot"); +} + +#[test] +fn should_panic() { + run_test_binary("should_panic"); +} + +#[test] +fn check_boot_info() { + run_test_binary("check_boot_info"); +} + +#[test] +fn global_variable() { + run_test_binary("global_variable"); +} + +fn run_test_binary(bin_name: &str) { + let mut cmd = Command::new(env!("CARGO")); + cmd.current_dir("tests/test_kernels/pie"); + cmd.arg("run"); + cmd.arg("--bin").arg(bin_name); + cmd.arg("--target").arg("x86_64-pie.json"); + cmd.arg("-Zbuild-std=core"); + cmd.arg("-Zbuild-std-features=compiler-builtins-mem"); + assert!(cmd.status().unwrap().success()); +} diff --git a/tests/test_kernels/pie/.cargo/config.toml b/tests/test_kernels/pie/.cargo/config.toml new file mode 100644 index 00000000..08a49f5a --- /dev/null +++ b/tests/test_kernels/pie/.cargo/config.toml @@ -0,0 +1,10 @@ +[unstable] +# TODO: Uncomment once https://github.com/rust-lang/cargo/issues/8643 is merged +# build-std = ["core"] + +[build] +# TODO: Uncomment once https://github.com/rust-lang/cargo/issues/8643 is merged +# target = "x86_64-example-kernel.json" + +[target.'cfg(target_os = "none")'] +runner = "cargo run --manifest-path ../../runner/Cargo.toml" \ No newline at end of file diff --git a/tests/test_kernels/pie/.gitignore b/tests/test_kernels/pie/.gitignore new file mode 100644 index 00000000..1de56593 --- /dev/null +++ b/tests/test_kernels/pie/.gitignore @@ -0,0 +1 @@ +target \ No newline at end of file diff --git a/tests/test_kernels/pie/Cargo.toml b/tests/test_kernels/pie/Cargo.toml new file mode 100644 index 00000000..d5c5140c --- /dev/null +++ b/tests/test_kernels/pie/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "test_kernel_pie" +version = "0.1.0" +authors = ["Tom Dohrmann "] +edition = "2018" + +[dependencies] +bootloader = { path = "../../.." } +x86_64 = { version = "0.13.2", default-features = false, features = ["instructions", "inline_asm"] } +uart_16550 = "0.2.10" diff --git a/tests/test_kernels/pie/src/bin/basic_boot.rs b/tests/test_kernels/pie/src/bin/basic_boot.rs new file mode 100644 index 00000000..f480d860 --- /dev/null +++ b/tests/test_kernels/pie/src/bin/basic_boot.rs @@ -0,0 +1,21 @@ +#![no_std] // don't link the Rust standard library +#![no_main] // disable all Rust-level entry points + +use bootloader::{entry_point, BootInfo}; +use core::panic::PanicInfo; +use test_kernel_pie::{exit_qemu, QemuExitCode}; + +entry_point!(kernel_main); + +fn kernel_main(_boot_info: &'static mut BootInfo) -> ! { + exit_qemu(QemuExitCode::Success); +} + +/// This function is called on panic. +#[panic_handler] +fn panic(info: &PanicInfo) -> ! { + use core::fmt::Write; + + let _ = writeln!(test_kernel_pie::serial(), "PANIC: {}", info); + exit_qemu(QemuExitCode::Failed); +} diff --git a/tests/test_kernels/pie/src/bin/check_boot_info.rs b/tests/test_kernels/pie/src/bin/check_boot_info.rs new file mode 100644 index 00000000..909f99c2 --- /dev/null +++ b/tests/test_kernels/pie/src/bin/check_boot_info.rs @@ -0,0 +1,68 @@ +#![no_std] // don't link the Rust standard library +#![no_main] // disable all Rust-level entry points + +use bootloader::{boot_info::PixelFormat, entry_point, BootInfo}; +use core::panic::PanicInfo; +use test_kernel_pie::{exit_qemu, QemuExitCode}; + +entry_point!(kernel_main); + +fn kernel_main(boot_info: &'static mut BootInfo) -> ! { + // check memory regions + assert!(boot_info.memory_regions.len() > 4); + + // check framebuffer + let framebuffer = boot_info.framebuffer.as_ref().unwrap(); + assert_eq!(framebuffer.info().byte_len, framebuffer.buffer().len()); + if ![640, 1024].contains(&framebuffer.info().horizontal_resolution) { + panic!( + "unexpected horizontal_resolution `{}`", + framebuffer.info().horizontal_resolution + ); + } + if ![480, 768].contains(&framebuffer.info().vertical_resolution) { + panic!( + "unexpected vertical_resolution `{}`", + framebuffer.info().vertical_resolution + ); + } + if ![3, 4].contains(&framebuffer.info().bytes_per_pixel) { + panic!( + "unexpected bytes_per_pixel `{}`", + framebuffer.info().bytes_per_pixel + ); + } + if ![640, 1024].contains(&framebuffer.info().stride) { + panic!("unexpected stride `{}`", framebuffer.info().stride); + } + assert_eq!(framebuffer.info().pixel_format, PixelFormat::BGR); + assert_eq!( + framebuffer.buffer().len(), + framebuffer.info().stride + * framebuffer.info().vertical_resolution + * framebuffer.info().bytes_per_pixel + ); + + // check defaults for optional features + assert_eq!(boot_info.physical_memory_offset.into_option(), None); + assert_eq!(boot_info.recursive_index.into_option(), None); + + // check rsdp_addr + let rsdp = boot_info.rsdp_addr.into_option().unwrap(); + assert!(rsdp > 0x000E0000); + assert!(rsdp < 0x000FFFFF); + + // the test kernel has no TLS template + assert_eq!(boot_info.tls_template.into_option(), None); + + exit_qemu(QemuExitCode::Success); +} + +/// This function is called on panic. +#[panic_handler] +fn panic(info: &PanicInfo) -> ! { + use core::fmt::Write; + + let _ = writeln!(test_kernel_pie::serial(), "PANIC: {}", info); + exit_qemu(QemuExitCode::Failed); +} diff --git a/tests/test_kernels/pie/src/bin/global_variable.rs b/tests/test_kernels/pie/src/bin/global_variable.rs new file mode 100644 index 00000000..d6a6f634 --- /dev/null +++ b/tests/test_kernels/pie/src/bin/global_variable.rs @@ -0,0 +1,38 @@ +#![no_std] // don't link the Rust standard library +#![no_main] // disable all Rust-level entry points + +use bootloader::{entry_point, BootInfo}; +use core::{ + panic::PanicInfo, + sync::atomic::{AtomicU64, Ordering}, +}; +use test_kernel_pie::{exit_qemu, QemuExitCode}; + +entry_point!(kernel_main); + +fn kernel_main(_boot_info: &'static mut BootInfo) -> ! { + // Initialize with a value that is unlikely to be anywhere in memory. + // If we can later read out this exact value, we can be sure that we actually + // read from this global variable and not some other location in memory. + static FOO: AtomicU64 = AtomicU64::new(0xdeadbeef); + + // Make sure that relocations are actually applied by referencing a `FOO` + // in `FOO_REF`. `FOO`'s address will be calculated and put into `FOO_REF` + // at load time using a relocation. + static FOO_REF: &AtomicU64 = &FOO; + + // Verify that the memory address pointed to by `FOO_REF` contains our value. + let val = FOO_REF.load(Ordering::Relaxed); + assert_eq!(val, 0xdeadbeef); + + exit_qemu(QemuExitCode::Success); +} + +/// This function is called on panic. +#[panic_handler] +fn panic(info: &PanicInfo) -> ! { + use core::fmt::Write; + + let _ = writeln!(test_kernel_pie::serial(), "PANIC: {}", info); + exit_qemu(QemuExitCode::Failed); +} diff --git a/tests/test_kernels/pie/src/bin/should_panic.rs b/tests/test_kernels/pie/src/bin/should_panic.rs new file mode 100644 index 00000000..a5b0c2fc --- /dev/null +++ b/tests/test_kernels/pie/src/bin/should_panic.rs @@ -0,0 +1,18 @@ +#![no_std] // don't link the Rust standard library +#![no_main] // disable all Rust-level entry points + +use bootloader::{entry_point, BootInfo}; +use core::panic::PanicInfo; +use test_kernel_pie::{exit_qemu, QemuExitCode}; + +entry_point!(kernel_main); + +fn kernel_main(_boot_info: &'static mut BootInfo) -> ! { + panic!(); +} + +/// This function is called on panic. +#[panic_handler] +fn panic(_info: &PanicInfo) -> ! { + exit_qemu(QemuExitCode::Success); +} diff --git a/tests/test_kernels/pie/src/lib.rs b/tests/test_kernels/pie/src/lib.rs new file mode 100644 index 00000000..4e46fdb6 --- /dev/null +++ b/tests/test_kernels/pie/src/lib.rs @@ -0,0 +1,27 @@ +#![no_std] + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u32)] +pub enum QemuExitCode { + Success = 0x10, + Failed = 0x11, +} + +pub fn exit_qemu(exit_code: QemuExitCode) -> ! { + use x86_64::instructions::{nop, port::Port}; + + unsafe { + let mut port = Port::new(0xf4); + port.write(exit_code as u32); + } + + loop { + nop(); + } +} + +pub fn serial() -> uart_16550::SerialPort { + let mut port = unsafe { uart_16550::SerialPort::new(0x3F8) }; + port.init(); + port +} diff --git a/tests/test_kernels/pie/x86_64-pie.json b/tests/test_kernels/pie/x86_64-pie.json new file mode 100644 index 00000000..082ab7ad --- /dev/null +++ b/tests/test_kernels/pie/x86_64-pie.json @@ -0,0 +1,16 @@ +{ + "llvm-target": "x86_64-unknown-none", + "data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128", + "arch": "x86_64", + "target-endian": "little", + "target-pointer-width": "64", + "target-c-int-width": "32", + "os": "none", + "executables": true, + "linker-flavor": "ld.lld", + "linker": "rust-lld", + "panic-strategy": "abort", + "disable-redzone": true, + "features": "-mmx,-sse,+soft-float", + "position-independent-executables": true + } From 002927ba8dd4acf9d8240ec01edea8c67835cc0f Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Wed, 15 Dec 2021 17:19:35 +0100 Subject: [PATCH 2/9] add comment --- src/binary/load_kernel.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index dacaa3df..87084a1c 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -368,6 +368,7 @@ where ); match rela.get_type() { + // R_AMD64_RELATIVE 8 => { let offset_in_file = find_offset(elf_file, rela.get_offset())? .ok_or("Destination of relocation is not mapped in physical memory")?; From fac26c667aa961b9e39ecc18175e093b073fdffa Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Tue, 28 Dec 2021 11:51:04 +0100 Subject: [PATCH 3/9] update x86_64 dep in test --- Cargo.lock | 2 +- tests/test_kernels/pie/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b255b139..ca22de45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -386,7 +386,7 @@ version = "0.1.0" dependencies = [ "bootloader", "uart_16550", - "x86_64 0.13.6", + "x86_64", ] [[package]] diff --git a/tests/test_kernels/pie/Cargo.toml b/tests/test_kernels/pie/Cargo.toml index d5c5140c..dfbb0308 100644 --- a/tests/test_kernels/pie/Cargo.toml +++ b/tests/test_kernels/pie/Cargo.toml @@ -6,5 +6,5 @@ edition = "2018" [dependencies] bootloader = { path = "../../.." } -x86_64 = { version = "0.13.2", default-features = false, features = ["instructions", "inline_asm"] } +x86_64 = { version = "0.14.7", default-features = false, features = ["instructions", "inline_asm"] } uart_16550 = "0.2.10" From 3d4b6e75f0656db2e26cb6253ed21d1b580657e3 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Tue, 28 Dec 2021 12:30:00 +0100 Subject: [PATCH 4/9] add safety checks --- src/binary/load_kernel.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 87084a1c..5fdad948 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -353,13 +353,27 @@ where let entry_size = rela_ent.ok_or("RelaEnt entry is missing")?; // Apply the mappings. - let entries = total_size / entry_size; - let relas = unsafe { - core::slice::from_raw_parts::>( - elf_file.input.as_ptr().add(offset as usize).cast(), - entries as usize, - ) - }; + let entries = (total_size / entry_size) as usize; + let rela_start = elf_file + .input + .as_ptr() + .wrapping_add(offset as usize) + .cast::>(); + + // Make sure the relocations are inside the elf file. + let rela_end = rela_start.wrapping_add(entries); + assert!(rela_start <= rela_end); + let file_ptr_range = elf_file.input.as_ptr_range(); + assert!( + file_ptr_range.start <= rela_start.cast(), + "the relocation table must start in the elf file" + ); + assert!( + rela_end.cast() <= file_ptr_range.end, + "the relocation table must end in the elf file" + ); + + let relas = unsafe { core::slice::from_raw_parts(rela_start, entries) }; for rela in relas { let idx = rela.get_symbol_table_index(); assert_eq!( From 62ad48a7286df8ea19bf6eda5a0b46dc5b255b7c Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Tue, 28 Dec 2021 12:34:51 +0100 Subject: [PATCH 5/9] return error instead of panicking --- src/binary/load_kernel.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 5fdad948..041c8850 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -344,8 +344,9 @@ where } else { // The section doesn't contain any relocations. - assert_eq!(rela_size, None); - assert_eq!(rela_ent, None); + if rela_size.is_some() || rela_ent.is_some() { + return Err("Rela entry is missing but RelaSize or RelaEnt have been provided"); + } return Ok(()); }; From 5d61efd72e201aff401031119ae0662d7a175f72 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Tue, 28 Dec 2021 12:35:08 +0100 Subject: [PATCH 6/9] customize panic message --- src/binary/load_kernel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 041c8850..6688f9c2 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -305,7 +305,7 @@ where let data = if let SegmentData::Dynamic64(data) = data { data } else { - unreachable!() + panic!("expected Dynamic64 segment") }; // Find the `Rela`, `RelaSize` and `RelaEnt` entries. From 2de37158c42e07f1f4d1a5613e3d328fb9fc8c41 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Tue, 28 Dec 2021 15:00:44 +0100 Subject: [PATCH 7/9] fix aliasing issues --- src/binary/load_kernel.rs | 210 ++++++++++++++++++++++++++------------ 1 file changed, 146 insertions(+), 64 deletions(-) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 6688f9c2..0b84fc2b 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -1,3 +1,5 @@ +use core::mem::align_of; + use crate::{ binary::{level_4_entries::UsedLevel4Entries, PAGE_SIZE}, boot_info::TlsTemplate, @@ -5,8 +7,8 @@ use crate::{ use x86_64::{ align_up, structures::paging::{ - mapper::MapperAllSizes, FrameAllocator, Page, PageSize, PageTableFlags as Flags, PhysFrame, - Size4KiB, + mapper::{MappedFrame, MapperAllSizes, TranslateResult}, + FrameAllocator, Page, PageSize, PageTableFlags as Flags, PhysFrame, Size4KiB, Translate, }, PhysAddr, VirtAddr, }; @@ -17,6 +19,9 @@ use xmas_elf::{ ElfFile, }; +/// Used by [`Inner::make_mut`] and [`Inner::clean_copied_flag`]. +const COPIED: Flags = Flags::BIT_9; + struct Loader<'a, M, F> { elf_file: ElfFile<'a>, inner: Inner<'a, M, F>, @@ -31,7 +36,7 @@ struct Inner<'a, M, F> { impl<'a, M, F> Loader<'a, M, F> where - M: MapperAllSizes, + M: MapperAllSizes + Translate, F: FrameAllocator, { fn new( @@ -75,14 +80,6 @@ where program::sanity_check(program_header, &self.elf_file)?; } - // Apply relocations in physical memory. - for program_header in self.elf_file.program_iter() { - if let Type::Dynamic = program_header.get_type()? { - self.inner - .handle_dynamic_segment(program_header, &self.elf_file)? - } - } - // Load the segments into virtual memory. let mut tls_template = None; for program_header in self.elf_file.program_iter() { @@ -106,6 +103,17 @@ where | Type::ProcessorSpecific(_) => {} } } + + // Apply relocations in virtual memory. + for program_header in self.elf_file.program_iter() { + if let Type::Dynamic = program_header.get_type()? { + self.inner + .handle_dynamic_segment(program_header, &self.elf_file)? + } + } + + self.inner.remove_copied_flags(&self.elf_file).unwrap(); + Ok(tls_template) } @@ -123,7 +131,7 @@ where impl<'a, M, F> Inner<'a, M, F> where - M: MapperAllSizes, + M: MapperAllSizes + Translate, F: FrameAllocator, { fn handle_load_segment(&mut self, segment: ProgramHeader) -> Result<(), &'static str> { @@ -175,7 +183,6 @@ where log::info!("Mapping bss section"); let virt_start_addr = VirtAddr::new(segment.virtual_addr()) + self.virtual_address_offset; - let phys_start_addr = self.kernel_offset + segment.offset(); let mem_size = segment.mem_size(); let file_size = segment.file_size(); @@ -220,47 +227,16 @@ where // the remaining part of the frame since the frame is no longer shared with other // segments now. - // calculate the frame where the last segment page is mapped - let orig_frame: PhysFrame = - PhysFrame::containing_address(phys_start_addr + file_size - 1u64); - // allocate a new frame to replace `orig_frame` - let new_frame = self.frame_allocator.allocate_frame().unwrap(); - - // zero new frame, utilizing that it's identity-mapped - { - let new_frame_ptr = new_frame.start_address().as_u64() as *mut PageArray; - unsafe { new_frame_ptr.write(ZERO_ARRAY) }; - } - - // copy the data bytes from orig_frame to new_frame - { - log::info!("Copy contents"); - let orig_bytes_ptr = orig_frame.start_address().as_u64() as *mut u8; - let new_bytes_ptr = new_frame.start_address().as_u64() as *mut u8; - - for offset in 0..(data_bytes_before_zero as isize) { - unsafe { - let orig_byte = orig_bytes_ptr.offset(offset).read(); - new_bytes_ptr.offset(offset).write(orig_byte); - } - } - } - - // remap last page from orig_frame to `new_frame` - log::info!("Remap last page"); let last_page = Page::containing_address(virt_start_addr + file_size - 1u64); - self.page_table - .unmap(last_page.clone()) - .map_err(|_err| "Failed to unmap last segment page because of bss memory")? - .1 - .ignore(); - let flusher = unsafe { - self.page_table - .map_to(last_page, new_frame, segment_flags, self.frame_allocator) + let new_frame = unsafe { self.make_mut(last_page) }; + let new_bytes_ptr = new_frame.start_address().as_u64() as *mut u8; + unsafe { + core::ptr::write_bytes( + new_bytes_ptr, + 0, + (Size4KiB::SIZE - data_bytes_before_zero) as usize, + ); } - .map_err(|_err| "Failed to remap last segment page because of bss memory")?; - // we operate on an inactive page table, so we don't need to flush our changes - flusher.ignore(); } // map additional frames for `.bss` memory that is not present in source file @@ -288,6 +264,104 @@ where Ok(()) } + /// This method is intended for making the memory loaded by a Load segment mutable. + /// + /// All memory from a Load segment starts out by mapped to the same frames that + /// contain the elf file. Thus writing to memory in that state will cause aliasing issues. + /// To avoid that, we allocate a new frame, copy all bytes from the old frame to the new frame, + /// and remap the page to the new frame. At this point the page no longer aliases the elf file + /// and we can write to it. + /// + /// When we map the new frame we also set [`COPIED`] flag in the page table flags, so that + /// we can detect if the frame has already been copied when we try to modify the page again. + /// + /// ## Safety + /// - `page` should be a page mapped by a Load segment. + /// + /// ## Panics + /// Panics if the page is not mapped in `self.page_table`. + unsafe fn make_mut(&mut self, page: Page) -> PhysFrame { + let (frame, flags) = match self.page_table.translate(page.start_address()) { + TranslateResult::Mapped { + frame, + offset: _, + flags, + } => (frame, flags), + TranslateResult::NotMapped => panic!("{:?} is not mapped", page), + TranslateResult::InvalidFrameAddress(_) => unreachable!(), + }; + let frame = if let MappedFrame::Size4KiB(frame) = frame { + frame + } else { + // We only map 4k pages. + unreachable!() + }; + + if flags.contains(COPIED) { + // The frame was already copied, we are free to modify it. + return frame; + } + + // Allocate a new frame and copy the memory, utilizing that both frames are identity mapped. + let new_frame = self.frame_allocator.allocate_frame().unwrap(); + let frame_ptr = frame.start_address().as_u64() as *const u8; + let new_frame_ptr = new_frame.start_address().as_u64() as *mut u8; + unsafe { + core::ptr::copy_nonoverlapping(frame_ptr, new_frame_ptr, Size4KiB::SIZE as usize); + } + + // Replace the underlying frame and update the flags. + self.page_table.unmap(page).unwrap().1.ignore(); + let new_flags = flags | COPIED; + unsafe { + self.page_table + .map_to(page, new_frame, new_flags, self.frame_allocator) + .unwrap() + .ignore(); + } + + new_frame + } + + /// Cleans up the custom flags set by [`Inner::make_mut`]. + fn remove_copied_flags(&mut self, elf_file: &ElfFile) -> Result<(), &'static str> { + for program_header in elf_file.program_iter() { + if let Type::Load = program_header.get_type()? { + let start = self.virtual_address_offset + program_header.virtual_addr(); + let end = start + program_header.mem_size(); + let start = VirtAddr::new(start); + let end = VirtAddr::new(end); + let start_page = Page::containing_address(start); + let end_page = Page::containing_address(end - 1u64); + for page in Page::::range_inclusive(start_page, end_page) { + // Translate the page and get the flags. + let res = self.page_table.translate(page.start_address()); + let flags = match res { + TranslateResult::Mapped { + frame: _, + offset: _, + flags, + } => flags, + TranslateResult::NotMapped | TranslateResult::InvalidFrameAddress(_) => { + unreachable!("has the elf file not been mapped correctly?") + } + }; + + if flags.contains(COPIED) { + // Remove the flag. + unsafe { + self.page_table + .update_flags(page, flags & !COPIED) + .unwrap() + .ignore(); + } + } + } + } + } + Ok(()) + } + fn handle_tls_segment(&mut self, segment: ProgramHeader) -> Result { Ok(TlsTemplate { start_addr: segment.virtual_addr() + self.virtual_address_offset, @@ -385,19 +459,27 @@ where match rela.get_type() { // R_AMD64_RELATIVE 8 => { - let offset_in_file = find_offset(elf_file, rela.get_offset())? - .ok_or("Destination of relocation is not mapped in physical memory")?; - let dest_addr = self.kernel_offset + offset_in_file; - let dest_ptr = dest_addr.as_u64() as *mut u64; - + check_is_in_load(elf_file, rela.get_offset())?; + let addr = self.virtual_address_offset + rela.get_offset(); let value = self .virtual_address_offset .checked_add(rela.get_addend()) .unwrap(); + let ptr = addr as *mut u64; + if ptr as usize % align_of::() != 0 { + return Err("destination of relocation is not aligned"); + } + + let virt_addr = VirtAddr::from_ptr(ptr); + let page = Page::containing_address(virt_addr); + let offset_in_page = virt_addr - page.start_address(); + + let new_frame = unsafe { self.make_mut(page) }; + let phys_addr = new_frame.start_address() + offset_in_page; + let addr = phys_addr.as_u64() as *mut u64; unsafe { - // write new value, utilizing that the address identity-mapped - dest_ptr.write(value); + addr.write(value); } } ty => unimplemented!("relocation type {:x} not supported", ty), @@ -408,19 +490,19 @@ where } } -/// Locate the offset into the elf file corresponding to a virtual address. -fn find_offset(elf_file: &ElfFile, virt_offset: u64) -> Result, &'static str> { +/// Check that the virtual offset belongs to a load segment. +fn check_is_in_load(elf_file: &ElfFile, virt_offset: u64) -> Result<(), &'static str> { for program_header in elf_file.program_iter() { if let Type::Load = program_header.get_type()? { if program_header.virtual_addr() <= virt_offset { let offset_in_segment = virt_offset - program_header.virtual_addr(); if offset_in_segment < program_header.file_size() { - return Ok(Some(program_header.offset() + offset_in_segment)); + return Ok(()); } } } } - Ok(None) + Err("offset is not in load segment") } /// Loads the kernel ELF file given in `bytes` in the given `page_table`. @@ -429,7 +511,7 @@ fn find_offset(elf_file: &ElfFile, virt_offset: u64) -> Result, &'st /// and a structure describing which level 4 page table entries are in use. pub fn load_kernel( bytes: &[u8], - page_table: &mut impl MapperAllSizes, + page_table: &mut (impl MapperAllSizes + Translate), frame_allocator: &mut impl FrameAllocator, ) -> Result<(VirtAddr, Option, UsedLevel4Entries), &'static str> { let mut loader = Loader::new(bytes, page_table, frame_allocator)?; From 3085217fe6e3f7c16f3c88fe17ad4aadbce0b0ea Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 9 Jan 2022 15:10:24 +0100 Subject: [PATCH 8/9] fix offset --- src/binary/load_kernel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 0b84fc2b..42b07e1d 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -232,7 +232,7 @@ where let new_bytes_ptr = new_frame.start_address().as_u64() as *mut u8; unsafe { core::ptr::write_bytes( - new_bytes_ptr, + new_bytes_ptr.add(data_bytes_before_zero), 0, (Size4KiB::SIZE - data_bytes_before_zero) as usize, ); From b9a46099c0d16b4ed82f2d5a3903195a7ee2aa8d Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 9 Jan 2022 16:16:00 +0100 Subject: [PATCH 9/9] fix type error --- src/binary/load_kernel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/binary/load_kernel.rs b/src/binary/load_kernel.rs index 42b07e1d..4be7b8e5 100644 --- a/src/binary/load_kernel.rs +++ b/src/binary/load_kernel.rs @@ -232,7 +232,7 @@ where let new_bytes_ptr = new_frame.start_address().as_u64() as *mut u8; unsafe { core::ptr::write_bytes( - new_bytes_ptr.add(data_bytes_before_zero), + new_bytes_ptr.add(data_bytes_before_zero as usize), 0, (Size4KiB::SIZE - data_bytes_before_zero) as usize, );