diff --git a/crates/servers/display-server/src/server.rs b/crates/servers/display-server/src/server.rs index 32470bd..72deecb 100644 --- a/crates/servers/display-server/src/server.rs +++ b/crates/servers/display-server/src/server.rs @@ -1,7 +1,8 @@ use server_framework::Server; -use port_framebuffer::Framebuffer; use crate::protocol::{DisplayRequest, DisplayResponse}; +use port_framebuffer::Framebuffer; +use types::display::Dimension; pub struct DisplayServer { fb: F, @@ -24,7 +25,15 @@ impl Server for DisplayServer { fn handle(&mut self, request: DisplayRequest) -> DisplayResponse { match request { DisplayRequest::DrawRect { x, y, w, h, color } => { - self.fb.fill_rect(x, y, w, h, color); + // Clients choose every coordinate; clip to the screen so a + // huge or off-screen rectangle costs nothing and coordinates + // never overflow in the driver. + let (fw, fh) = (self.fb.width().raw(), self.fb.height().raw()); + if x.raw() < fw && y.raw() < fh { + let w = Dimension::new(w.raw().min(fw - x.raw())); + let h = Dimension::new(h.raw().min(fh - y.raw())); + self.fb.fill_rect(x, y, w, h, color); + } DisplayResponse::Ok } DisplayRequest::Clear { color } => { diff --git a/crates/servers/display-server/src/tests/server.rs b/crates/servers/display-server/src/tests/server.rs index 63cac28..6e6e4de 100644 --- a/crates/servers/display-server/src/tests/server.rs +++ b/crates/servers/display-server/src/tests/server.rs @@ -1,11 +1,14 @@ -use driver_framebuffer_mock::MockFramebuffer; use crate::protocol::{DisplayRequest, DisplayResponse}; use crate::server::DisplayServer; +use driver_framebuffer_mock::MockFramebuffer; use server_framework::Server; use types::display::{Color, Dimension, ScreenCoord}; fn test_server() -> DisplayServer { - DisplayServer::new(MockFramebuffer::new(Dimension::new(320), Dimension::new(240))) + DisplayServer::new(MockFramebuffer::new( + Dimension::new(320), + Dimension::new(240), + )) } #[test] @@ -32,7 +35,9 @@ fn clear_resets_pixels() { h: Dimension::new(10), color: Color::from_raw(0xFF), }); - server.handle(DisplayRequest::Clear { color: Color::from_raw(0) }); + server.handle(DisplayRequest::Clear { + color: Color::from_raw(0), + }); assert_eq!(server.framebuffer().pixel_count(), 0); } @@ -69,8 +74,67 @@ fn draw_rect_correct_pixel_positions() { }); let fb = server.framebuffer(); assert_eq!(fb.pixel_count(), 4); - assert_eq!(fb.pixel_at(0), (ScreenCoord::new(5), ScreenCoord::new(10), Color::from_raw(0xAB))); - assert_eq!(fb.pixel_at(1), (ScreenCoord::new(6), ScreenCoord::new(10), Color::from_raw(0xAB))); - assert_eq!(fb.pixel_at(2), (ScreenCoord::new(5), ScreenCoord::new(11), Color::from_raw(0xAB))); - assert_eq!(fb.pixel_at(3), (ScreenCoord::new(6), ScreenCoord::new(11), Color::from_raw(0xAB))); + assert_eq!( + fb.pixel_at(0), + ( + ScreenCoord::new(5), + ScreenCoord::new(10), + Color::from_raw(0xAB) + ) + ); + assert_eq!( + fb.pixel_at(1), + ( + ScreenCoord::new(6), + ScreenCoord::new(10), + Color::from_raw(0xAB) + ) + ); + assert_eq!( + fb.pixel_at(2), + ( + ScreenCoord::new(5), + ScreenCoord::new(11), + Color::from_raw(0xAB) + ) + ); + assert_eq!( + fb.pixel_at(3), + ( + ScreenCoord::new(6), + ScreenCoord::new(11), + Color::from_raw(0xAB) + ) + ); +} + +#[test] +fn a_huge_rect_is_clipped_to_the_screen() { + let mut server = test_server(); + let resp = server.handle(DisplayRequest::DrawRect { + x: ScreenCoord::new(310), + y: ScreenCoord::new(230), + w: Dimension::new(u16::MAX), + h: Dimension::new(u16::MAX), + color: Color::from_raw(0xFF), + }); + assert_eq!(resp, DisplayResponse::Ok); + assert_eq!(server.framebuffer().pixel_count(), 10 * 10); + let (x, y, _) = server.framebuffer().pixel_at(99); + assert_eq!((x.raw(), y.raw()), (319, 239)); +} + +#[test] +fn an_off_screen_rect_draws_nothing() { + let mut server = test_server(); + for (x, y) in [(320, 0), (0, 240), (u16::MAX, u16::MAX)] { + server.handle(DisplayRequest::DrawRect { + x: ScreenCoord::new(x), + y: ScreenCoord::new(y), + w: Dimension::new(u16::MAX), + h: Dimension::new(u16::MAX), + color: Color::from_raw(0xFF), + }); + } + assert_eq!(server.framebuffer().pixel_count(), 0); } diff --git a/crates/servers/session-server/src/lib.rs b/crates/servers/session-server/src/lib.rs index c496ce4..62a9b95 100644 --- a/crates/servers/session-server/src/lib.rs +++ b/crates/servers/session-server/src/lib.rs @@ -8,9 +8,10 @@ pub mod server; mod tests; pub fn run() -> ! { - let create_cap = types::bounded::CapSlot::new(0).unwrap(); - let mut srv = server::SessionServer::new(create_cap); - srv.register_app("snake", "/apps/snake.elf"); + // Slot 0 holds the endpoint init delegated. Init has no process + // creation authority to hand on yet, so Launch reports an error. + let mut srv = server::SessionServer::new(None); + let _ = srv.register_app("snake", "/apps/snake.elf"); let ep = types::bounded::CapSlot::new(0).unwrap(); server_framework::serve(&mut srv, ep) } diff --git a/crates/servers/session-server/src/server.rs b/crates/servers/session-server/src/server.rs index b904f9c..6ac9eb5 100644 --- a/crates/servers/session-server/src/server.rs +++ b/crates/servers/session-server/src/server.rs @@ -1,6 +1,5 @@ use server_framework::Server; use types::bounded::CapSlot; -use types::id::ProcessId; use crate::protocol::{AppId, SessionRequest, SessionResponse}; @@ -15,8 +14,8 @@ pub struct AppEntry { #[derive(Clone, Copy)] struct RunningApp { - app: AppId, - pid: ProcessId, + /// The capability `Kill` is invoked on. + process: CapSlot, } pub struct SessionServer { @@ -24,11 +23,12 @@ pub struct SessionServer { app_count: usize, running: [Option; MAX_APPS], active: Option, - create_cap: CapSlot, + /// Lets the server create processes; without one, Launch fails. + create_cap: Option, } impl SessionServer { - pub fn new(create_cap: CapSlot) -> Self { + pub fn new(create_cap: Option) -> Self { Self { apps: [const { None }; MAX_APPS], app_count: 0, @@ -38,11 +38,17 @@ impl SessionServer { } } - pub fn register_app(&mut self, name: &'static str, path: &'static str) -> AppId { + /// Adds an app to the menu; `None` once the table is full. + pub fn register_app(&mut self, name: &'static str, path: &'static str) -> Option { + let slot = self.apps.get_mut(self.app_count)?; + *slot = Some(AppEntry { + name, + path, + registered: true, + }); let id = AppId::new(self.app_count as u16); - self.apps[self.app_count] = Some(AppEntry { name, path, registered: true }); self.app_count += 1; - id + Some(id) } pub fn app_count(&self) -> usize { @@ -53,8 +59,10 @@ impl SessionServer { self.active } - fn is_running(&self, app: AppId) -> bool { - self.running[app.raw() as usize].is_some() + /// The running-table entry for `app`; `None` for ids past the table, + /// which come straight from clients. + fn running_mut(&mut self, app: AppId) -> Option<&mut Option> { + self.running.get_mut(app.raw() as usize) } fn launch_app(&mut self, app: AppId) -> SessionResponse { @@ -63,31 +71,27 @@ impl SessionServer { return SessionResponse::NotFound; } - if self.is_running(app) { + if self.apps.get(idx).is_none_or(Option::is_none) { + return SessionResponse::NotFound; + } + if self.running_mut(app).is_none_or(|r| r.is_some()) { return SessionResponse::Error; } - - let _app_entry = match &self.apps[idx] { - Some(entry) => entry, - None => return SessionResponse::NotFound, + let Some(process) = self.create_cap.and_then(libkos::syscall::create_process) else { + return SessionResponse::Error; }; - - let process_cap = match libkos::syscall::create_process(self.create_cap) { - Some(cap) => cap, - None => return SessionResponse::Error, - }; - - let pid = ProcessId::from_raw(process_cap.value() as u16); - self.running[idx] = Some(RunningApp { app, pid }); + if let Some(r) = self.running_mut(app) { + *r = Some(RunningApp { process }); + } self.active = Some(app); SessionResponse::Launched { app } } fn kill_app(&mut self, app: AppId) -> SessionResponse { - let idx = app.raw() as usize; - match self.running[idx].take() { - Some(_running) => { + match self.running_mut(app).and_then(Option::take) { + Some(running) => { + let _ = libkos::syscall::kill(running.process); if self.active == Some(app) { self.active = None; } @@ -106,7 +110,9 @@ impl Server for SessionServer { match request { SessionRequest::Launch { app } => self.launch_app(app), SessionRequest::Kill { app } => self.kill_app(app), - SessionRequest::ListApps => SessionResponse::AppList { count: self.app_count }, + SessionRequest::ListApps => SessionResponse::AppList { + count: self.app_count, + }, SessionRequest::ActiveApp => match self.active { Some(app) => SessionResponse::Active { app }, None => SessionResponse::NotFound, diff --git a/crates/servers/session-server/src/tests/server.rs b/crates/servers/session-server/src/tests/server.rs index 06cb882..516dd5b 100644 --- a/crates/servers/session-server/src/tests/server.rs +++ b/crates/servers/session-server/src/tests/server.rs @@ -4,9 +4,9 @@ use server_framework::Server; fn test_server() -> SessionServer { let cap = types::bounded::CapSlot::new(0).unwrap(); - let mut s = SessionServer::new(cap); - s.register_app("snake", "/apps/snake.elf"); - s.register_app("clock", "/apps/clock.elf"); + let mut s = SessionServer::new(Some(cap)); + s.register_app("snake", "/apps/snake.elf").unwrap(); + s.register_app("clock", "/apps/clock.elf").unwrap(); s } @@ -35,7 +35,9 @@ fn launch_sets_active() { #[test] fn launch_invalid_app_returns_not_found() { let mut s = test_server(); - let resp = s.handle(SessionRequest::Launch { app: AppId::new(99) }); + let resp = s.handle(SessionRequest::Launch { + app: AppId::new(99), + }); assert_eq!(resp, SessionResponse::NotFound); } @@ -77,3 +79,36 @@ fn no_active_app_initially() { let resp = s.handle(SessionRequest::ActiveApp); assert_eq!(resp, SessionResponse::NotFound); } + +#[test] +fn ids_past_the_table_are_rejected_not_indexed() { + let mut server = test_server(); + for raw in [2, 16, 17, u16::MAX] { + let app = AppId::new(raw); + assert_eq!( + server.handle(SessionRequest::Kill { app }), + SessionResponse::NotFound + ); + assert_eq!( + server.handle(SessionRequest::Launch { app }), + SessionResponse::NotFound + ); + } +} + +#[test] +fn the_app_table_fills_up_without_panicking() { + let mut server = SessionServer::new(None); + let added = (0..20) + .filter(|_| server.register_app("x", "/x").is_some()) + .count(); + assert_eq!(added, 16); +} + +#[test] +fn launching_needs_process_creation_authority() { + let mut server = SessionServer::new(None); + server.register_app("snake", "/apps/snake.elf").unwrap(); + let resp = server.handle(SessionRequest::Launch { app: AppId::new(0) }); + assert_eq!(resp, SessionResponse::Error); +} diff --git a/crates/servers/storage-server/src/fs/fat16.rs b/crates/servers/storage-server/src/fs/fat16.rs index be1e20c..814697c 100644 --- a/crates/servers/storage-server/src/fs/fat16.rs +++ b/crates/servers/storage-server/src/fs/fat16.rs @@ -1,5 +1,5 @@ -use port_block_device::{BlockDevice, SectorId}; use crate::filesystem::{DirEntry, FileHandle, FileName, FileSystem}; +use port_block_device::{BlockDevice, SectorId}; const SECTOR_SIZE: usize = 512; const MAX_OPEN_FILES: usize = 8; @@ -15,10 +15,16 @@ struct Bpb { } impl Bpb { + /// Reads the BPB, refusing geometry the rest of this module cannot + /// work with: the disk is untrusted input like any client request. fn parse(sector: &[u8]) -> Option { - if sector.len() < SECTOR_SIZE { return None; } - if sector[510] != 0x55 || sector[511] != 0xAA { return None; } - Some(Self { + if sector.len() < SECTOR_SIZE { + return None; + } + if sector[510] != 0x55 || sector[511] != 0xAA { + return None; + } + let bpb = Self { bytes_per_sector: u16::from_le_bytes([sector[11], sector[12]]), sectors_per_cluster: sector[13], reserved_sectors: u16::from_le_bytes([sector[14], sector[15]]), @@ -26,10 +32,17 @@ impl Bpb { root_entry_count: u16::from_le_bytes([sector[17], sector[18]]), total_sectors: u16::from_le_bytes([sector[19], sector[20]]), sectors_per_fat: u16::from_le_bytes([sector[22], sector[23]]), - }) + }; + let usable = bpb.bytes_per_sector as usize == SECTOR_SIZE + && bpb.sectors_per_cluster.is_power_of_two() + && bpb.num_fats > 0 + && bpb.sectors_per_fat > 0; + usable.then_some(bpb) } - fn fat_start(&self) -> u32 { self.reserved_sectors as u32 } + fn fat_start(&self) -> u32 { + self.reserved_sectors as u32 + } fn root_dir_start(&self) -> u32 { self.fat_start() + (self.num_fats as u32) * (self.sectors_per_fat as u32) @@ -44,8 +57,11 @@ impl Bpb { self.root_dir_start() + self.root_dir_sectors() } - fn cluster_to_sector(&self, cluster: u16) -> u32 { - self.data_start() + ((cluster as u32) - 2) * (self.sectors_per_cluster as u32) + /// The first sector of data cluster `cluster`. Clusters 0 and 1 are + /// reserved and hold no data. + fn cluster_to_sector(&self, cluster: u16) -> Option { + let index = (cluster as u32).checked_sub(2)?; + Some(self.data_start() + index * (self.sectors_per_cluster as u32)) } fn cluster_size(&self) -> usize { @@ -70,9 +86,15 @@ pub struct Fat16 { impl Fat16 { pub fn mount(mut device: B) -> Option { let mut buf = [0u8; SECTOR_SIZE]; - if !device.read_sector(SectorId::new(0), &mut buf) { return None; } + if !device.read_sector(SectorId::new(0), &mut buf) { + return None; + } let bpb = Bpb::parse(&buf)?; - Some(Self { device, bpb, open_files: [const { None }; MAX_OPEN_FILES] }) + Some(Self { + device, + bpb, + open_files: [const { None }; MAX_OPEN_FILES], + }) } fn read_fat_entry(&mut self, cluster: u16) -> u16 { @@ -80,7 +102,9 @@ impl Fat16 { let sector = self.bpb.fat_start() + fat_offset / (SECTOR_SIZE as u32); let offset = (fat_offset % (SECTOR_SIZE as u32)) as usize; let mut buf = [0u8; SECTOR_SIZE]; - if !self.device.read_sector(SectorId::new(sector), &mut buf) { return 0xFFFF; } + if !self.device.read_sector(SectorId::new(sector), &mut buf) { + return 0xFFFF; + } u16::from_le_bytes([buf[offset], buf[offset + 1]]) } @@ -109,7 +133,9 @@ impl Fat16 { fn free_chain(&mut self, start: u16) { let mut cluster = start; loop { - if cluster < 2 || cluster >= 0xFFF8 { break; } + if cluster < 2 || cluster >= 0xFFF8 { + break; + } let next = self.read_fat_entry(cluster); self.write_fat_entry(cluster, 0); cluster = next; @@ -120,13 +146,25 @@ impl Fat16 { let mut buf = [0u8; SECTOR_SIZE]; let fat_name = to_fat_name(name); for s in 0..self.bpb.root_dir_sectors() { - if !self.device.read_sector(SectorId::new(self.bpb.root_dir_start() + s), &mut buf) { continue; } + if !self + .device + .read_sector(SectorId::new(self.bpb.root_dir_start() + s), &mut buf) + { + continue; + } for i in 0..(SECTOR_SIZE / 32) { let off = i * 32; - if buf[off] == 0x00 || buf[off] == 0xE5 { continue; } + if buf[off] == 0x00 || buf[off] == 0xE5 { + continue; + } if buf[off..off + 11] == fat_name { let cluster = u16::from_le_bytes([buf[off + 26], buf[off + 27]]); - let size = u32::from_le_bytes([buf[off + 28], buf[off + 29], buf[off + 30], buf[off + 31]]); + let size = u32::from_le_bytes([ + buf[off + 28], + buf[off + 29], + buf[off + 30], + buf[off + 31], + ]); return Some((cluster, size as usize)); } } @@ -139,16 +177,22 @@ impl Fat16 { let fat_name = to_fat_name(name); for s in 0..self.bpb.root_dir_sectors() { let sector = self.bpb.root_dir_start() + s; - if !self.device.read_sector(SectorId::new(sector), &mut buf) { continue; } + if !self.device.read_sector(SectorId::new(sector), &mut buf) { + continue; + } for i in 0..(SECTOR_SIZE / 32) { let off = i * 32; if buf[off] == 0x00 || buf[off] == 0xE5 { buf[off..off + 11].copy_from_slice(&fat_name); buf[off + 11] = 0x20; - for b in &mut buf[off + 12..off + 26] { *b = 0; } + for b in &mut buf[off + 12..off + 26] { + *b = 0; + } buf[off + 26] = cluster as u8; buf[off + 27] = (cluster >> 8) as u8; - for b in &mut buf[off + 28..off + 32] { *b = 0; } + for b in &mut buf[off + 28..off + 32] { + *b = 0; + } self.device.write_sector(SectorId::new(sector), &buf); return true; } @@ -162,7 +206,9 @@ impl Fat16 { let fat_name = to_fat_name(name); for s in 0..self.bpb.root_dir_sectors() { let sector = self.bpb.root_dir_start() + s; - if !self.device.read_sector(SectorId::new(sector), &mut buf) { continue; } + if !self.device.read_sector(SectorId::new(sector), &mut buf) { + continue; + } for i in 0..(SECTOR_SIZE / 32) { let off = i * 32; if buf[off..off + 11] == fat_name { @@ -181,7 +227,9 @@ impl Fat16 { let fat_name = to_fat_name(name); for s in 0..self.bpb.root_dir_sectors() { let sector = self.bpb.root_dir_start() + s; - if !self.device.read_sector(SectorId::new(sector), &mut buf) { continue; } + if !self.device.read_sector(SectorId::new(sector), &mut buf) { + continue; + } for i in 0..(SECTOR_SIZE / 32) { let off = i * 32; if buf[off..off + 11] == fat_name { @@ -199,12 +247,20 @@ impl Fat16 { let mut cluster = start; for _ in 0..skip_clusters { let next = self.read_fat_entry(cluster); - if next >= 0xFFF8 { return cluster; } + if next >= 0xFFF8 { + return cluster; + } cluster = next; } cluster } + /// The open-file slot `handle` names; `None` for handles past the + /// table, which come straight from clients. + fn slot(&mut self, handle: FileHandle) -> Option<&mut Option> { + self.open_files.get_mut(handle.raw() as usize) + } + fn alloc_handle(&self) -> Option { for i in 0..MAX_OPEN_FILES { if self.open_files[i].is_none() { @@ -219,15 +275,20 @@ impl FileSystem for Fat16 { fn open(&mut self, path: &[u8]) -> Option { let (cluster, size) = self.find_in_root_dir(path)?; let handle = self.alloc_handle()?; - self.open_files[handle.raw() as usize] = Some(OpenFile { - first_cluster: cluster, size, offset: 0, name: FileName::from_bytes(path), + *self.slot(handle)? = Some(OpenFile { + first_cluster: cluster, + size, + offset: 0, + name: FileName::from_bytes(path), }); Some(handle) } fn read(&mut self, handle: FileHandle, buf: &mut [u8]) -> Option { - let file = *self.open_files[handle.raw() as usize].as_ref()?; - if file.offset >= file.size { return Some(0); } + let file = (*self.slot(handle)?)?; + if file.offset >= file.size { + return Some(0); + } let remaining = file.size - file.offset; let to_read = buf.len().min(remaining); @@ -242,28 +303,41 @@ impl FileSystem for Fat16 { while read < to_read { let sector_in_cluster = skip / SECTOR_SIZE; let byte_in_sector = skip % SECTOR_SIZE; - let sector = self.bpb.cluster_to_sector(cluster) + sector_in_cluster as u32; - if !self.device.read_sector(SectorId::new(sector), &mut sector_buf) { break; } + let Some(base) = self.bpb.cluster_to_sector(cluster) else { + break; + }; + let sector = base + sector_in_cluster as u32; + if !self + .device + .read_sector(SectorId::new(sector), &mut sector_buf) + { + break; + } let chunk = (SECTOR_SIZE - byte_in_sector).min(to_read - read); - buf[read..read + chunk].copy_from_slice(§or_buf[byte_in_sector..byte_in_sector + chunk]); + buf[read..read + chunk] + .copy_from_slice(§or_buf[byte_in_sector..byte_in_sector + chunk]); read += chunk; skip += chunk; if skip >= cs { let next = self.read_fat_entry(cluster); - if next >= 0xFFF8 { break; } + if next >= 0xFFF8 { + break; + } cluster = next; skip = 0; } } - self.open_files[handle.raw() as usize].as_mut().map(|f| f.offset += read); + if let Some(Some(f)) = self.slot(handle) { + f.offset += read; + } Some(read) } fn write(&mut self, handle: FileHandle, buf: &[u8]) -> Option { - let mut file = *self.open_files[handle.raw() as usize].as_ref()?; + let mut file = (*self.slot(handle)?)?; let cs = self.bpb.cluster_size(); if file.first_cluster < 2 { @@ -280,11 +354,16 @@ impl FileSystem for Fat16 { while written < buf.len() { let sector_in_cluster = pos / SECTOR_SIZE; let byte_in_sector = pos % SECTOR_SIZE; - let sector = self.bpb.cluster_to_sector(cluster) + sector_in_cluster as u32; - self.device.read_sector(SectorId::new(sector), &mut sector_buf); + let Some(base) = self.bpb.cluster_to_sector(cluster) else { + break; + }; + let sector = base + sector_in_cluster as u32; + self.device + .read_sector(SectorId::new(sector), &mut sector_buf); let chunk = (SECTOR_SIZE - byte_in_sector).min(buf.len() - written); - sector_buf[byte_in_sector..byte_in_sector + chunk].copy_from_slice(&buf[written..written + chunk]); + sector_buf[byte_in_sector..byte_in_sector + chunk] + .copy_from_slice(&buf[written..written + chunk]); self.device.write_sector(SectorId::new(sector), §or_buf); written += chunk; @@ -297,23 +376,28 @@ impl FileSystem for Fat16 { if let Some(new_c) = self.alloc_cluster() { self.write_fat_entry(cluster, new_c); cluster = new_c; - } else { break; } + } else { + break; + } } - } else { cluster = next; } + } else { + cluster = next; + } pos = 0; } } file.offset += written; - if file.offset > file.size { file.size = file.offset; } - self.open_files[handle.raw() as usize] = Some(file); + if file.offset > file.size { + file.size = file.offset; + } + *self.slot(handle)? = Some(file); Some(written) } fn close(&mut self, handle: FileHandle) { - let file = match self.open_files[handle.raw() as usize].take() { - Some(f) => f, - None => return, + let Some(file) = self.slot(handle).and_then(Option::take) else { + return; }; let mut name_buf = [0u8; 32]; let name_bytes = file.name.as_bytes(); @@ -326,13 +410,29 @@ impl FileSystem for Fat16 { let mut buf = [0u8; SECTOR_SIZE]; let mut count = 0; for s in 0..self.bpb.root_dir_sectors() { - if !self.device.read_sector(SectorId::new(self.bpb.root_dir_start() + s), &mut buf) { continue; } + if !self + .device + .read_sector(SectorId::new(self.bpb.root_dir_start() + s), &mut buf) + { + continue; + } for i in 0..(SECTOR_SIZE / 32) { - if count >= entries.len() { return Some(count); } + if count >= entries.len() { + return Some(count); + } let off = i * 32; - if buf[off] == 0x00 { return Some(count); } - if buf[off] == 0xE5 || buf[off + 11] & 0x08 != 0 { continue; } - let size = u32::from_le_bytes([buf[off + 28], buf[off + 29], buf[off + 30], buf[off + 31]]); + if buf[off] == 0x00 { + return Some(count); + } + if buf[off] == 0xE5 || buf[off + 11] & 0x08 != 0 { + continue; + } + let size = u32::from_le_bytes([ + buf[off + 28], + buf[off + 29], + buf[off + 30], + buf[off + 31], + ]); entries[count] = DirEntry { name: from_fat_name(&buf[off..off + 11]), size: size as usize, @@ -345,15 +445,20 @@ impl FileSystem for Fat16 { } fn create(&mut self, path: &[u8]) -> Option { - if self.find_in_root_dir(path).is_some() { return None; } + if self.find_in_root_dir(path).is_some() { + return None; + } let cluster = self.alloc_cluster()?; if !self.create_dir_entry(path, cluster) { self.write_fat_entry(cluster, 0); return None; } let handle = self.alloc_handle()?; - self.open_files[handle.raw() as usize] = Some(OpenFile { - first_cluster: cluster, size: 0, offset: 0, name: FileName::from_bytes(path), + *self.slot(handle)? = Some(OpenFile { + first_cluster: cluster, + size: 0, + offset: 0, + name: FileName::from_bytes(path), }); Some(handle) } @@ -362,20 +467,26 @@ impl FileSystem for Fat16 { if let Some(cluster) = self.delete_dir_entry(path) { self.free_chain(cluster); true - } else { false } + } else { + false + } } fn file_size(&mut self, handle: FileHandle) -> Option { - self.open_files[handle.raw() as usize].as_ref().map(|f| f.size) + self.slot(handle)?.as_ref().map(|f| f.size) } - fn mkdir(&mut self, _path: &[u8]) -> bool { false } + fn mkdir(&mut self, _path: &[u8]) -> bool { + false + } } fn to_fat_name(name: &[u8]) -> [u8; 11] { let mut fat = [b' '; 11]; let dot_pos = name.iter().position(|&b| b == b'.').unwrap_or(name.len()); - for i in 0..dot_pos.min(8) { fat[i] = name[i].to_ascii_uppercase(); } + for i in 0..dot_pos.min(8) { + fat[i] = name[i].to_ascii_uppercase(); + } if dot_pos < name.len() { for i in 0..(name.len() - dot_pos - 1).min(3) { fat[8 + i] = name[dot_pos + 1 + i].to_ascii_uppercase(); @@ -387,12 +498,27 @@ fn to_fat_name(name: &[u8]) -> [u8; 11] { fn from_fat_name(raw: &[u8]) -> FileName { let mut buf = [0u8; 32]; let mut pos = 0; - let name_end = raw[..8].iter().rposition(|&b| b != b' ').map(|i| i + 1).unwrap_or(0); - for i in 0..name_end { buf[pos] = raw[i].to_ascii_lowercase(); pos += 1; } - let ext_end = raw[8..11].iter().rposition(|&b| b != b' ').map(|i| i + 1).unwrap_or(0); + let name_end = raw[..8] + .iter() + .rposition(|&b| b != b' ') + .map(|i| i + 1) + .unwrap_or(0); + for i in 0..name_end { + buf[pos] = raw[i].to_ascii_lowercase(); + pos += 1; + } + let ext_end = raw[8..11] + .iter() + .rposition(|&b| b != b' ') + .map(|i| i + 1) + .unwrap_or(0); if ext_end > 0 { - buf[pos] = b'.'; pos += 1; - for i in 0..ext_end { buf[pos] = raw[8 + i].to_ascii_lowercase(); pos += 1; } + buf[pos] = b'.'; + pos += 1; + for i in 0..ext_end { + buf[pos] = raw[8 + i].to_ascii_lowercase(); + pos += 1; + } } FileName::from_bytes(&buf[..pos]) } diff --git a/crates/servers/storage-server/src/tests/fat16.rs b/crates/servers/storage-server/src/tests/fat16.rs index eafccba..39ea62b 100644 --- a/crates/servers/storage-server/src/tests/fat16.rs +++ b/crates/servers/storage-server/src/tests/fat16.rs @@ -1,21 +1,25 @@ use crate::adapters::mock_block::MockBlockDevice; -use port_block_device::{BlockDevice, SectorId}; use crate::filesystem::{DirEntry, FileSystem}; use crate::fs::fat16::Fat16; +use port_block_device::{BlockDevice, SectorId}; fn format_fat16_disk(sectors: usize) -> MockBlockDevice { let mut dev = MockBlockDevice::new(sectors); let mut boot = [0u8; 512]; // BPB - boot[11] = 0x00; boot[12] = 0x02; // bytes_per_sector = 512 - boot[13] = 1; // sectors_per_cluster = 1 - boot[14] = 1; boot[15] = 0; // reserved_sectors = 1 - boot[16] = 1; // num_fats = 1 - boot[17] = 16; boot[18] = 0; // root_entry_count = 16 - boot[19] = sectors as u8; // total_sectors + boot[11] = 0x00; + boot[12] = 0x02; // bytes_per_sector = 512 + boot[13] = 1; // sectors_per_cluster = 1 + boot[14] = 1; + boot[15] = 0; // reserved_sectors = 1 + boot[16] = 1; // num_fats = 1 + boot[17] = 16; + boot[18] = 0; // root_entry_count = 16 + boot[19] = sectors as u8; // total_sectors boot[20] = (sectors >> 8) as u8; - boot[22] = 1; boot[23] = 0; // sectors_per_fat = 1 + boot[22] = 1; + boot[23] = 0; // sectors_per_fat = 1 // boot signature boot[510] = 0x55; @@ -110,3 +114,75 @@ fn create_duplicate_returns_none() { fs.create(b"dup.txt").map(|h| fs.close(h)); assert!(fs.create(b"dup.txt").is_none()); } + +// ---- hostile input: handles from clients, geometry and chains from disk ---- + +fn with_boot_byte(sectors: usize, index: usize, value: u8) -> MockBlockDevice { + let mut dev = format_fat16_disk(sectors); + let mut boot = [0u8; 512]; + dev.read_sector(SectorId::new(0), &mut boot); + boot[index] = value; + dev.write_sector(SectorId::new(0), &boot); + dev +} + +#[test] +fn mount_rejects_geometry_the_driver_cannot_use() { + // sectors_per_cluster 0 or 3, bytes_per_sector 513 or 0, no FATs, no + // FAT sectors + for (index, value) in [(13, 0), (13, 3), (11, 1), (12, 0), (16, 0), (22, 0)] { + let dev = with_boot_byte(32, index, value); + assert!( + Fat16::mount(dev).is_none(), + "byte {index} = {value} accepted" + ); + } +} + +#[test] +fn handles_past_the_table_are_rejected_not_indexed() { + use crate::filesystem::FileHandle; + let mut fs = mount_test_disk(); + let mut buf = [0u8; 16]; + for raw in [8, 9, u16::MAX] { + let h = FileHandle::new(raw); + assert_eq!(fs.read(h, &mut buf), None); + assert_eq!(fs.write(h, b"x"), None); + assert_eq!(fs.file_size(h), None); + fs.close(h); + } +} + +/// Points a root-directory entry for `name` at `cluster` with `size`. +fn plant_entry(dev: &mut MockBlockDevice, name: &[u8; 11], cluster: u16, size: u32) { + // root dir starts after 1 reserved + 1 FAT sector + let mut dir = [0u8; 512]; + dir[..11].copy_from_slice(name); + dir[11] = 0x20; + dir[26..28].copy_from_slice(&cluster.to_le_bytes()); + dir[28..32].copy_from_slice(&size.to_le_bytes()); + dev.write_sector(SectorId::new(2), &dir); +} + +#[test] +fn a_file_pointing_at_cluster_zero_reads_nothing() { + let mut dev = format_fat16_disk(32); + plant_entry(&mut dev, b"BAD BIN", 0, 4096); + let mut fs = Fat16::mount(dev).unwrap(); + let h = fs.open(b"bad.bin").expect("open"); + let mut buf = [0u8; 64]; + assert_eq!(fs.read(h, &mut buf), Some(0)); +} + +#[test] +fn deleting_a_file_with_a_cyclic_chain_terminates() { + let mut dev = format_fat16_disk(32); + // cluster 2 -> 3 -> 2 -> ... + let mut fat = [0u8; 512]; + fat[4..6].copy_from_slice(&3u16.to_le_bytes()); + fat[6..8].copy_from_slice(&2u16.to_le_bytes()); + dev.write_sector(SectorId::new(1), &fat); + plant_entry(&mut dev, b"LOOP BIN", 2, 4096); + let mut fs = Fat16::mount(dev).unwrap(); + assert!(fs.delete(b"loop.bin")); +}