fix(ext4): review fixes — bounds checks, delete safety, blocks_count, multi-extent dirs

crates/servers/storage-server/src/fs/ext4/mod.rs

@@ -249,34 +249,7 @@ impl<B: BlockDevice> Ext4<B> {
 
     fn find_in_dir(&mut self, dir_inode: u32, name: &[u8]) -> Option<u32> {
         let inode = self.read_inode(dir_inode)?;
-        if !inode.is_dir() { return None; }
+        dir::find_entry(&mut self.device, &self.sb, self.gdt_block, &inode, name)
-
-        let extent = self.read_extents(&inode)?;
-        let mut block_buf = [0u8; 4096];
-
-        for blk in 0..extent.length as u64 {
-            read_block(
-                &mut self.device,
-                extent.physical_block + blk,
-                self.sb.sectors_per_block(),
-                &mut block_buf,
-            );
-            let mut pos = 0;
-            while pos + 8 <= self.sb.block_size {
-                let entry_inode = read_u32(&block_buf, pos);
-                let rec_len = read_u16(&block_buf, pos + 4) as usize;
-                let name_len = block_buf[pos + 6] as usize;
-
-                if rec_len == 0 { break; }
-                if entry_inode != 0 && name_len == name.len() {
-                    if &block_buf[pos + 8..pos + 8 + name_len] == name {
-                        return Some(entry_inode);
-                    }
-                }
-                pos += rec_len;
-            }
-        }
-        None
     }
 
     fn resolve_path(&mut self, path: &[u8]) -> Option<u32> {
@@ -338,6 +311,19 @@ impl<B: BlockDevice> Ext4<B> {
         true
     }
 
+    fn count_extent_blocks(block_data: &[u8; 60]) -> u32 {
+        let magic = read_u16(block_data, 0);
+        if magic != EXTENT_MAGIC { return 0; }
+        let entries = read_u16(block_data, 2);
+        let mut total = 0u32;
+        for i in 0..entries as usize {
+            let off = 12 + i * 12;
+            if off + 12 > block_data.len() { break; }
+            total += read_u16(block_data, off + 4) as u32;
+        }
+        total
+    }
+
     fn free_all_blocks(&mut self, inode: &Inode) {
         if !inode.uses_extents() { return; }
         let hdr = &inode.block_data;
@@ -373,6 +359,7 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
     }
 
     fn read(&mut self, handle: FileHandle, buf: &mut [u8]) -> Option<usize> {
+        if handle.raw() as usize >= MAX_OPEN_FILES { return None; }
         let file = *self.open_files[handle.raw() as usize].as_ref()?;
         if file.offset >= file.size { return Some(0); }
 
@@ -405,6 +392,7 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
     }
 
     fn write(&mut self, handle: FileHandle, buf: &[u8]) -> Option<usize> {
+        if handle.raw() as usize >= MAX_OPEN_FILES { return None; }
         let file = *self.open_files[handle.raw() as usize].as_ref()?;
         let bs = self.sb.block_size;
         let mut written = 0;
@@ -424,7 +412,10 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
             let phys_block = match self.map_logical_block(&tmp_inode, logical_block) {
                 Some(p) => p,
                 None => {
-                    let new_block = self.alloc_block()?;
+                    let new_block = match self.alloc_block() {
+                        Some(b) => b,
+                        None => break,
+                    };
                     if !Self::append_extent(&mut block_data, logical_block, new_block as u64, 1) {
                         self.free_block(new_block);
                         break;
@@ -460,6 +451,7 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
             };
             inode.size = size as u64;
             inode.block_data = block_data;
+            inode.blocks_count = Self::count_extent_blocks(&block_data) * (bs as u32 / 512);
             self.write_inode(inode_num, &inode);
         }
 
@@ -467,6 +459,7 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
     }
 
     fn close(&mut self, handle: FileHandle) {
+        if handle.raw() as usize >= MAX_OPEN_FILES { return; }
         if let Some(file) = self.open_files[handle.raw() as usize].take() {
             if let Some(mut inode) = self.read_inode(file.inode_num) {
                 inode.size = file.size as u64;
@@ -484,45 +477,15 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
         };
 
         let inode = self.read_inode(dir_inode_num)?;
-        if !inode.is_dir() { return None; }
+        let count = dir::list_entries(&mut self.device, &self.sb, &inode, entries)?;
-
+        for i in 0..count {
-        let extent = self.read_extents(&inode)?;
+            if !entries[i].is_dir {
-        let mut block_buf = [0u8; 4096];
+                let name = entries[i].name;
-        let mut count = 0;
+                if let Some(child_ino) = dir::find_entry(&mut self.device, &self.sb, self.gdt_block, &inode, name.as_bytes()) {
-
+                    if let Some(child) = self.read_inode(child_ino) {
-        for blk in 0..extent.length as u64 {
+                        entries[i].size = child.size as usize;
-            read_block(
-                &mut self.device,
-                extent.physical_block + blk,
-                self.sb.sectors_per_block(),
-                &mut block_buf,
-            );
-            let mut pos = 0;
-            while pos + 8 <= self.sb.block_size && count < entries.len() {
-                let entry_inode = read_u32(&block_buf, pos);
-                let rec_len = read_u16(&block_buf, pos + 4) as usize;
-                let name_len = block_buf[pos + 6] as usize;
-                let file_type = block_buf[pos + 7];
-
-                if rec_len == 0 { break; }
-                if entry_inode != 0 && name_len > 0 {
-                    let name = &block_buf[pos + 8..pos + 8 + name_len];
-                    if name != b"." && name != b".." {
-                        let child_size = if file_type == 1 {
-                            self.read_inode(entry_inode)
-                                .map(|i| i.size as usize)
-                                .unwrap_or(0)
-                        } else { 0 };
-
-                        entries[count] = DirEntry {
-                            name: FileName::from_bytes(name),
-                            size: child_size,
-                            is_dir: file_type == 2,
-                        };
-                        count += 1;
                     }
                 }
-                pos += rec_len;
             }
         }
         Some(count)
@@ -568,21 +531,41 @@ impl<B: BlockDevice> FileSystem for Ext4<B> {
             None => return false,
         };
 
-        let child_inode_num = match dir::remove_entry(
+        let child_inode_num = match dir::find_entry(&mut self.device, &self.sb, self.gdt_block, &parent_inode, filename) {
-            &mut self.device, &self.sb, self.gdt_block, &parent_inode, filename,
-        ) {
             Some(n) => n,
             None => return false,
         };
 
+        if let Some(child_inode) = self.read_inode(child_inode_num) {
+            if child_inode.is_dir() {
+                return false;
+            }
+        }
+
+        let parent_inode = match self.read_inode(parent_inode_num) {
+            Some(i) => i,
+            None => return false,
+        };
+        if dir::remove_entry(&mut self.device, &self.sb, self.gdt_block, &parent_inode, filename).is_none() {
+            return false;
+        }
+
         if let Some(child_inode) = self.read_inode(child_inode_num) {
             self.free_all_blocks(&child_inode);
         }
         self.free_inode(child_inode_num);
+        for slot in self.open_files.iter_mut() {
+            if let Some(f) = slot {
+                if f.inode_num == child_inode_num {
+                    *slot = None;
+                }
+            }
+        }
         true
     }
 
     fn file_size(&mut self, handle: FileHandle) -> Option<usize> {
+        if handle.raw() as usize >= MAX_OPEN_FILES { return None; }
         self.open_files[handle.raw() as usize].as_ref().map(|f| f.size)
     }
 

crates/servers/storage-server/src/tests/ext4.rs

@@ -562,3 +562,36 @@ fn open_directory_as_file_returns_none() {
     assert!(fs.mkdir(b"adir"));
     assert!(fs.open(b"adir").is_none());
 }
+
+#[test]
+fn invalid_handle_does_not_panic() {
+    let mut fs = Ext4::mount(format_ext4_disk()).unwrap();
+    let bad = crate::filesystem::FileHandle::new(99);
+    assert!(fs.read(bad, &mut [0u8; 16]).is_none());
+    assert!(fs.write(bad, b"x").is_none());
+    assert!(fs.file_size(bad).is_none());
+    fs.close(bad);
+}
+
+#[test]
+fn delete_open_file_invalidates_handle() {
+    let mut fs = Ext4::mount(format_ext4_disk()).unwrap();
+    let h = fs.create(b"victim").unwrap();
+    fs.write(h, b"data").unwrap();
+    fs.close(h);
+    let h = fs.open(b"victim").unwrap();
+    assert!(fs.delete(b"victim"));
+    assert!(fs.read(h, &mut [0u8; 16]).is_none());
+}
+
+#[test]
+fn delete_nonempty_dir_returns_false() {
+    let mut fs = Ext4::mount(format_ext4_disk()).unwrap();
+    assert!(fs.mkdir(b"parent"));
+    let h = fs.create(b"parent/child").unwrap();
+    fs.close(h);
+    assert!(!fs.delete(b"parent"));
+    let mut entries = [DirEntry::empty(); 8];
+    let count = fs.list_dir(b"parent", &mut entries).unwrap();
+    assert_eq!(count, 1);
+}