Merge pull request #524 from rust-osdev/fix/buffer-overflow

fix buffer overflow in `DiskAccess::read_exact_into`

Author: phil-opp

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

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

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);
 }
 

bios/stage-2/src/fat.rs

@@ -32,9 +32,8 @@ struct Bpb {
 }
 
 impl Bpb {
-    fn parse<D: Read + Seek>(disk: &mut D) -> Self {
-        disk.seek(SeekFrom::Start(0));
-        let raw = unsafe { disk.read_exact(512) };
+    fn parse<D: Read>(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<D> Traverser<'_, D>
 where
-    D: Read + Seek,
+    D: Read,
 {
     fn next_cluster(&mut self) -> Result<Option<Cluster>, ()> {
         let entry = classify_fat_entry(
@@ -286,7 +285,7 @@ where
 
 impl<D> Iterator for Traverser<'_, D>
 where
-    D: Read + Seek,
+    D: Read,
 {
     type Item = Result<Cluster, ()>;
 
@@ -476,28 +475,25 @@ enum FileFatEntry {
 
 fn fat_entry_of_nth_cluster<D>(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 {

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");