ext2: fix inode table offset calculation

Signed-off-by: Andy-Python-Programmer <andypythonappdeveloper@gmail.com>

Author: Andy-Python-Programmer

src/aero_kernel/src/drivers/block/ide/mod.rs

@@ -52,7 +52,7 @@ impl IdeDrive {
 }
 
 impl BlockDeviceInterface for IdeDrive {
-    fn read(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
+    fn read_block(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
         let count = dest.len().ceil_div(512);
         let request = Arc::new(DmaRequest::new(sector, count));
 
@@ -66,7 +66,11 @@ impl BlockDeviceInterface for IdeDrive {
         res
     }
 
-    fn write(&self, _sector: usize, _buf: &[u8]) -> Option<usize> {
+    fn block_size(&self) -> usize {
+        todo!()
+    }
+
+    fn write_block(&self, _sector: usize, _buf: &[u8]) -> Option<usize> {
         unimplemented!()
     }
 }

src/aero_kernel/src/drivers/block/nvme/mod.rs

@@ -468,12 +468,16 @@ impl<'a> Controller<'a> {
 }
 
 impl<'a> BlockDeviceInterface for Controller<'a> {
-    fn read(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
+    fn read_block(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
         self.namespaces.lock()[0].read(sector, dest);
         Some(dest.len())
     }
 
-    fn write(&self, _sector: usize, _buf: &[u8]) -> Option<usize> {
+    fn block_size(&self) -> usize {
+        self.namespaces.lock()[0].block_size
+    }
+
+    fn write_block(&self, _sector: usize, _buf: &[u8]) -> Option<usize> {
         unimplemented!()
     }
 }

src/aero_kernel/src/fs/block/gpt.rs

@@ -127,7 +127,7 @@ impl Gpt {
 
         controller
             .device()
-            .read(1, header.as_bytes_mut())
+            .read_block(1, header.as_bytes_mut())
             .expect("gpt: failed to read first sector");
 
         // SAFETY: The buffer is initialized above.
@@ -144,7 +144,7 @@ impl Gpt {
 
         controller
             .device()
-            .read(
+            .read_block(
                 header.starting_lba as _,
                 MaybeUninit::slice_as_bytes_mut(&mut entry_list),
             )

src/aero_kernel/src/fs/block/mod.rs

@@ -19,6 +19,7 @@
 
 mod gpt;
 
+use alloc::boxed::Box;
 use gpt::Gpt;
 
 use core::mem::MaybeUninit;
@@ -31,14 +32,34 @@ use crate::fs::devfs::install_device;
 use crate::fs::{Path, Result, MOUNT_MANAGER};
 
 use crate::fs::ext2::Ext2;
+use crate::mem::paging::{align_down, align_up};
 use crate::utils::sync::Mutex;
 
 use super::devfs::{alloc_device_marker, Device};
 use super::inode::INodeInterface;
 
 pub trait BlockDeviceInterface: Send + Sync {
-    fn read(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize>;
-    fn write(&self, sector: usize, buf: &[u8]) -> Option<usize>;
+    fn block_size(&self) -> usize;
+
+    fn read_block(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize>;
+    fn write_block(&self, sector: usize, buf: &[u8]) -> Option<usize>;
+
+    fn read(&self, offset: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
+        let aligned_offset = align_down(offset as u64, self.block_size() as u64) as usize;
+        let sector = aligned_offset / self.block_size();
+
+        let aligned_size = align_up(dest.len() as u64, self.block_size() as u64) as usize;
+        let mut buffer = Box::<[u8]>::new_uninit_slice(aligned_size);
+
+        self.read_block(sector, MaybeUninit::slice_as_bytes_mut(&mut buffer))?;
+        // SAFETY: We have initialized the buffer above.
+        let buffer = unsafe { buffer.assume_init() };
+
+        let offset = offset - aligned_offset;
+        MaybeUninit::write_slice(dest, &buffer[offset..(offset + dest.len())]);
+
+        Some(dest.len())
+    }
 }
 
 static BLOCK_DEVS: Mutex<BTreeMap<usize, Arc<BlockDevice>>> = Mutex::new(BTreeMap::new());
@@ -113,20 +134,24 @@ impl PartitionBlockDevice {
 }
 
 impl BlockDeviceInterface for PartitionBlockDevice {
-    fn read(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
+    fn read_block(&self, sector: usize, dest: &mut [MaybeUninit<u8>]) -> Option<usize> {
         if sector >= self.size {
             return None;
         }
 
-        self.device.read(self.offset + sector, dest)
+        self.device.read_block(self.offset + sector, dest)
+    }
+
+    fn block_size(&self) -> usize {
+        self.device.block_size()
     }
 
-    fn write(&self, sector: usize, buf: &[u8]) -> Option<usize> {
+    fn write_block(&self, sector: usize, buf: &[u8]) -> Option<usize> {
         if sector >= self.size {
             return None;
         }
 
-        self.device.write(self.offset + sector, buf)
+        self.device.write_block(self.offset + sector, buf)
     }
 }
 

src/aero_kernel/src/fs/ext2/mod.rs

@@ -22,12 +22,13 @@ use core::mem::MaybeUninit;
 use alloc::boxed::Box;
 use alloc::sync::{Arc, Weak};
 
+use crate::fs::cache::CachedINode;
 use crate::utils::CeilDiv;
 
 use super::block::BlockDevice;
 
 use super::cache;
-use super::cache::{CachedINode, DirCacheItem, INodeCacheItem};
+use super::cache::{DirCacheItem, INodeCacheItem};
 
 use super::inode::{DirEntry, INodeInterface, Metadata};
 use super::FileSystem;
@@ -154,7 +155,7 @@ impl From<FileType> for super::inode::FileType {
     }
 }
 
-#[repr(C)]
+#[repr(C, packed)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct DiskINode {
     type_and_perm: u16,
@@ -213,24 +214,32 @@ impl INode {
             Some(inode)
         } else {
             let fs = ext2.upgrade()?;
+            let superblock = &fs.superblock;
+
+            // There is one inode table per block group and can be located by
+            // the `inode_table` offset in the group descriptor. Also there are
+            // `inodes_per_group` inodes per table.
+            let ino_per_group = superblock.inodes_per_group as usize;
 
-            let inode_block_group = (id - 1) / fs.superblock.inodes_per_group as usize;
-            let inode_table_idx = (id - 1) % fs.superblock.inodes_per_group as usize;
+            let ino_block_group = (id - 1) / ino_per_group;
+            let ino_table_index = (id - 1) % ino_per_group;
 
-            let group_descriptor = &fs.bgdt[inode_block_group];
-            let inode_size = core::mem::size_of::<DiskINode>(); // TODO: the inode size can be different
+            let group_descriptor = &fs.bgdt[ino_block_group];
 
-            let table_offset = group_descriptor.inode_table as usize * fs.superblock.block_size();
-            let inode_offset = table_offset + (inode_size * inode_table_idx);
+            let table_offset = group_descriptor.inode_table as usize * superblock.block_size();
 
             let mut inode = Box::<DiskINode>::new_uninit();
-            fs.block
-                .device()
-                .read(inode_offset / 512, inode.as_bytes_mut());
+
+            fs.block.device().read(
+                table_offset + (ino_table_index * core::mem::size_of::<DiskINode>()),
+                inode.as_bytes_mut(),
+            )?;
 
             // SAFETY: We have initialized the inode above.
             let inode = unsafe { inode.assume_init() };
 
+            log::debug!("ino_table_index={ino_table_index:?} inode={inode:?}");
+
             Some(icache.make_item_cached(CachedINode::new(Arc::new(Self {
                 inode,
                 id,
@@ -268,7 +277,7 @@ impl Ext2 {
 
     pub fn new(block: Arc<BlockDevice>) -> Option<Arc<Self>> {
         let mut superblock = Box::<SuperBlock>::new_uninit();
-        block.device().read(2, superblock.as_bytes_mut())?;
+        block.device().read_block(2, superblock.as_bytes_mut())?;
 
         // SAFETY: We have initialized the superblock above.
         let superblock = unsafe { superblock.assume_init() };
@@ -277,10 +286,15 @@ impl Ext2 {
             return None;
         }
 
+        assert_eq!(
+            superblock.inode_size as usize,
+            core::mem::size_of::<DiskINode>()
+        );
+
         let bgdt_len = superblock.bgdt_len();
         let mut bgdt = Box::<[GroupDescriptor]>::new_uninit_slice(bgdt_len);
 
-        block.device().read(
+        block.device().read_block(
             superblock.bgdt_sector(),
             MaybeUninit::slice_as_bytes_mut(&mut bgdt),
         )?;