From 18356dbbe5b7465bc7d3b867c60b91148c57ddc0 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sat, 29 Nov 2025 12:40:05 +0100 Subject: [PATCH 1/4] fix buffer overflow in DiskAccess::read_exact_into DiskAccess::read_exact_into did not work correctly if current_offset wasn't aligned to 512 bytes. For example, if self.current_offset is 3 and len is 1024: - end_addr will be 3+1024=1027 - start_lba will be 3/512=0 - end_lba (1027-1)=2 read_exact_into would then read 3 (!) sectors with lbas 0, 1, and 2 even though the buffer can only hold 2 sectors (1024 bytes). To fix this, only allow seeking to the start of a sector. DiskAccess::read_exact_into works correctly if the offset is a multiple of the sector size. There were few uses of DiskAccess::seek that didn't seek to the start of a sector. All those uses then use DiskAccess::read_exact to read data at the offset. Unlike DiskAccess::read_exact_into, DiskAccess::read_exact can already handle non-aligned offsets. To fix the bad seek calls, we turn read_exact into a combined seek+offset function that works for non-aligned offsets. --- bios/stage-2/src/disk.rs | 8 +++++--- bios/stage-2/src/fat.rs | 20 ++++++++------------ 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/bios/stage-2/src/disk.rs b/bios/stage-2/src/disk.rs index ec83eec3..66880a9b 100644 --- a/bios/stage-2/src/disk.rs +++ b/bios/stage-2/src/disk.rs @@ -8,8 +8,9 @@ pub struct DiskAccess { } impl Read for DiskAccess { - unsafe fn read_exact(&mut self, len: usize) -> &[u8] { - let current_sector_offset = usize::try_from(self.current_offset % 512).unwrap(); + unsafe fn read_exact_at(&mut self, len: usize, offset: u64) -> &[u8] { + self.seek(SeekFrom::Start(offset - (offset % 512))); + let current_sector_offset = usize::try_from(offset % 512).unwrap(); static mut TMP_BUF: AlignedArrayBuffer<1024> = AlignedArrayBuffer { buffer: [0; 512 * 2], @@ -62,6 +63,7 @@ impl Seek for DiskAccess { fn seek(&mut self, pos: SeekFrom) -> u64 { match pos { SeekFrom::Start(offset) => { + assert_eq!(offset % 512, 0); self.current_offset = offset; self.current_offset } @@ -70,7 +72,7 @@ impl Seek for DiskAccess { } pub trait Read { - unsafe fn read_exact(&mut self, len: usize) -> &[u8]; + unsafe fn read_exact_at(&mut self, len: usize, offset: u64) -> &[u8]; fn read_exact_into(&mut self, len: usize, buf: &mut dyn AlignedBuffer); } diff --git a/bios/stage-2/src/fat.rs b/bios/stage-2/src/fat.rs index 79eff1ac..7d828bd6 100644 --- a/bios/stage-2/src/fat.rs +++ b/bios/stage-2/src/fat.rs @@ -32,9 +32,8 @@ struct Bpb { } impl Bpb { - fn parse(disk: &mut D) -> Self { - disk.seek(SeekFrom::Start(0)); - let raw = unsafe { disk.read_exact(512) }; + fn parse(disk: &mut D) -> Self { + let raw = unsafe { disk.read_exact_at(512, 0) }; let bytes_per_sector = u16::from_le_bytes(raw[11..13].try_into().unwrap()); let sectors_per_cluster = raw[13]; @@ -256,7 +255,7 @@ struct Traverser<'a, D> { impl Traverser<'_, D> where - D: Read + Seek, + D: Read, { fn next_cluster(&mut self) -> Result, ()> { let entry = classify_fat_entry( @@ -286,7 +285,7 @@ where impl Iterator for Traverser<'_, D> where - D: Read + Seek, + D: Read, { type Item = Result; @@ -476,28 +475,25 @@ enum FileFatEntry { fn fat_entry_of_nth_cluster(disk: &mut D, fat_type: FatType, fat_start: u64, n: u32) -> u32 where - D: Seek + Read, + D: Read, { debug_assert!(n >= 2); match fat_type { FatType::Fat32 => { let base = n as u64 * 4; - disk.seek(SeekFrom::Start(fat_start + base)); - let buf = unsafe { disk.read_exact(4) }; + let buf = unsafe { disk.read_exact_at(4, fat_start + base) }; let buf: [u8; 4] = buf.try_into().unwrap(); u32::from_le_bytes(buf) & 0x0FFFFFFF } FatType::Fat16 => { let base = n as u64 * 2; - disk.seek(SeekFrom::Start(fat_start + base)); - let buf = unsafe { disk.read_exact(2) }; + let buf = unsafe { disk.read_exact_at(2, fat_start + base) }; let buf: [u8; 2] = buf.try_into().unwrap(); u16::from_le_bytes(buf) as u32 } FatType::Fat12 => { let base = n as u64 + (n as u64 / 2); - disk.seek(SeekFrom::Start(fat_start + base)); - let buf = unsafe { disk.read_exact(2) }; + let buf = unsafe { disk.read_exact_at(2, fat_start + base) }; let buf: [u8; 2] = buf.try_into().unwrap(); let entry16 = u16::from_le_bytes(buf); if n & 1 == 0 { From 9bf2de2738706b2bd622433ed51b7a462d9457ff Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sat, 29 Nov 2025 12:52:32 +0100 Subject: [PATCH 2/4] use opt-level z This generates smaller binaries. --- Cargo.toml | 2 +- bios/stage-2/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3aa07b4a..31764326 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -83,7 +83,7 @@ overflow-checks = false # duplicated from `bios/stage-2/Cargo.toml` [profile.stage-2] inherits = "release" -opt-level = "s" +opt-level = "z" codegen-units = 1 debug = false overflow-checks = true diff --git a/bios/stage-2/Cargo.toml b/bios/stage-2/Cargo.toml index 4e39accb..2487e964 100644 --- a/bios/stage-2/Cargo.toml +++ b/bios/stage-2/Cargo.toml @@ -18,7 +18,7 @@ bootloader-x86_64-bios-common = { workspace = true } # See https://github.com/rust-lang/cargo/issues/8264 for details. [profile.stage-2] inherits = "release" -opt-level = "s" +opt-level = "z" codegen-units = 1 debug = false overflow-checks = true From 809bb7fed1188137db4999901783c84501848878 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sat, 29 Nov 2025 12:52:50 +0100 Subject: [PATCH 3/4] remove symbol mangling hack Now that we fixed the buffer overflow and found another way of generating smaller binaries, we no longer need to use the old symbol mangling scheme. The new symbol mangling was never really at fault. All it did was shuffle some of the symbols around resulting in a larger binary. We just got unlucky there. --- build.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/build.rs b/build.rs index 84663244..c83c6145 100644 --- a/build.rs +++ b/build.rs @@ -215,10 +215,6 @@ fn build_bios_stage_2() -> PathBuf { cmd.env_remove("RUSTFLAGS"); cmd.env_remove("CARGO_ENCODED_RUSTFLAGS"); cmd.env_remove("RUSTC_WORKSPACE_WRAPPER"); // used by clippy - cmd.env( - "RUSTFLAGS", - "-Csymbol-mangling-version=legacy -Zunstable-options", - ); let status = cmd .status() .expect("failed to run cargo install for bios second stage"); From f5b3bdb1417066c7d8f102404cde061b2068adc9 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sat, 29 Nov 2025 13:09:01 +0100 Subject: [PATCH 4/4] update bootloader crates in lockfiles --- examples/basic/Cargo.lock | 12 ++++++------ tests/test_kernels/Cargo.lock | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/basic/Cargo.lock b/examples/basic/Cargo.lock index efe2f2d2..0c13af02 100644 --- a/examples/basic/Cargo.lock +++ b/examples/basic/Cargo.lock @@ -79,9 +79,9 @@ dependencies = [ [[package]] name = "bootloader" -version = "0.11.12" +version = "0.11.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61e418d2f8952e4a6d935d23ea038daed2fe7707b0118923f91e83bf1fbe64b3" +checksum = "269724328c6b8eda6fb9c4f054e1a5937fb27861272dd92a58d06b231f32cd48" dependencies = [ "anyhow", "bootloader-boot-config", @@ -95,18 +95,18 @@ dependencies = [ [[package]] name = "bootloader-boot-config" -version = "0.11.12" +version = "0.11.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cfbe7f4a5d9843a9a310712e64691f3c474546f02bed7a53ed81775a681d18e" +checksum = "8695f73dcd0377cabbc64df31924b92456fde56cb9c2ae217da242e3235a1564" dependencies = [ "serde", ] [[package]] name = "bootloader_api" -version = "0.11.12" +version = "0.11.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba1a4adcd36a49a08c4fb8c13c132fb1ea01a59681b305b46625095318328455" +checksum = "9af2881e494cfadbfd715dc4dd0cd8ff1f3bab70bed96385da250b392f4d91fc" [[package]] name = "bumpalo" diff --git a/tests/test_kernels/Cargo.lock b/tests/test_kernels/Cargo.lock index 26557330..ea6968ed 100644 --- a/tests/test_kernels/Cargo.lock +++ b/tests/test_kernels/Cargo.lock @@ -22,7 +22,7 @@ checksum = "8f68f53c83ab957f72c32642f3868eec03eb974d1fb82e453128456482613d36" [[package]] name = "bootloader_api" -version = "0.11.11" +version = "0.11.13" [[package]] name = "rustversion"