Skip to content

Commit 18356db

Browse files
committed
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.
1 parent 9a5cf6b commit 18356db

File tree

2 files changed

+13
-15
lines changed

2 files changed

+13
-15
lines changed

bios/stage-2/src/disk.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ pub struct DiskAccess {
88
}
99

1010
impl Read for DiskAccess {
11-
unsafe fn read_exact(&mut self, len: usize) -> &[u8] {
12-
let current_sector_offset = usize::try_from(self.current_offset % 512).unwrap();
11+
unsafe fn read_exact_at(&mut self, len: usize, offset: u64) -> &[u8] {
12+
self.seek(SeekFrom::Start(offset - (offset % 512)));
13+
let current_sector_offset = usize::try_from(offset % 512).unwrap();
1314

1415
static mut TMP_BUF: AlignedArrayBuffer<1024> = AlignedArrayBuffer {
1516
buffer: [0; 512 * 2],
@@ -62,6 +63,7 @@ impl Seek for DiskAccess {
6263
fn seek(&mut self, pos: SeekFrom) -> u64 {
6364
match pos {
6465
SeekFrom::Start(offset) => {
66+
assert_eq!(offset % 512, 0);
6567
self.current_offset = offset;
6668
self.current_offset
6769
}
@@ -70,7 +72,7 @@ impl Seek for DiskAccess {
7072
}
7173

7274
pub trait Read {
73-
unsafe fn read_exact(&mut self, len: usize) -> &[u8];
75+
unsafe fn read_exact_at(&mut self, len: usize, offset: u64) -> &[u8];
7476
fn read_exact_into(&mut self, len: usize, buf: &mut dyn AlignedBuffer);
7577
}
7678

bios/stage-2/src/fat.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ struct Bpb {
3232
}
3333

3434
impl Bpb {
35-
fn parse<D: Read + Seek>(disk: &mut D) -> Self {
36-
disk.seek(SeekFrom::Start(0));
37-
let raw = unsafe { disk.read_exact(512) };
35+
fn parse<D: Read>(disk: &mut D) -> Self {
36+
let raw = unsafe { disk.read_exact_at(512, 0) };
3837

3938
let bytes_per_sector = u16::from_le_bytes(raw[11..13].try_into().unwrap());
4039
let sectors_per_cluster = raw[13];
@@ -256,7 +255,7 @@ struct Traverser<'a, D> {
256255

257256
impl<D> Traverser<'_, D>
258257
where
259-
D: Read + Seek,
258+
D: Read,
260259
{
261260
fn next_cluster(&mut self) -> Result<Option<Cluster>, ()> {
262261
let entry = classify_fat_entry(
@@ -286,7 +285,7 @@ where
286285

287286
impl<D> Iterator for Traverser<'_, D>
288287
where
289-
D: Read + Seek,
288+
D: Read,
290289
{
291290
type Item = Result<Cluster, ()>;
292291

@@ -476,28 +475,25 @@ enum FileFatEntry {
476475

477476
fn fat_entry_of_nth_cluster<D>(disk: &mut D, fat_type: FatType, fat_start: u64, n: u32) -> u32
478477
where
479-
D: Seek + Read,
478+
D: Read,
480479
{
481480
debug_assert!(n >= 2);
482481
match fat_type {
483482
FatType::Fat32 => {
484483
let base = n as u64 * 4;
485-
disk.seek(SeekFrom::Start(fat_start + base));
486-
let buf = unsafe { disk.read_exact(4) };
484+
let buf = unsafe { disk.read_exact_at(4, fat_start + base) };
487485
let buf: [u8; 4] = buf.try_into().unwrap();
488486
u32::from_le_bytes(buf) & 0x0FFFFFFF
489487
}
490488
FatType::Fat16 => {
491489
let base = n as u64 * 2;
492-
disk.seek(SeekFrom::Start(fat_start + base));
493-
let buf = unsafe { disk.read_exact(2) };
490+
let buf = unsafe { disk.read_exact_at(2, fat_start + base) };
494491
let buf: [u8; 2] = buf.try_into().unwrap();
495492
u16::from_le_bytes(buf) as u32
496493
}
497494
FatType::Fat12 => {
498495
let base = n as u64 + (n as u64 / 2);
499-
disk.seek(SeekFrom::Start(fat_start + base));
500-
let buf = unsafe { disk.read_exact(2) };
496+
let buf = unsafe { disk.read_exact_at(2, fat_start + base) };
501497
let buf: [u8; 2] = buf.try_into().unwrap();
502498
let entry16 = u16::from_le_bytes(buf);
503499
if n & 1 == 0 {

0 commit comments

Comments
 (0)