From 25a5e59f791a78ab7d7efe30b4cd1b745f19493c Mon Sep 17 00:00:00 2001
From: Mart G <martg_@hotmail.com>
Date: Fri, 7 May 2021 14:47:38 +0200
Subject: Kernel: Place ext2 dir entries so they don't span multiple blocks

Ext2 dir entries spanning multiple blocks are not allowed.
If they do occur they are flagged as corrupt by e2fsck for example.
---
 Kernel/FileSystem/Ext2FileSystem.cpp | 52 +++++++++++++++++++++---------------
 Kernel/FileSystem/Ext2FileSystem.h   |  2 +-
 2 files changed, 31 insertions(+), 23 deletions(-)

(limited to 'Kernel/FileSystem')

diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp
index f672e4a8f9..9a1f182eb0 100644
--- a/Kernel/FileSystem/Ext2FileSystem.cpp
+++ b/Kernel/FileSystem/Ext2FileSystem.cpp
@@ -27,6 +27,7 @@ struct Ext2FSDirectoryEntry {
     String name;
     InodeIndex inode_index { 0 };
     u8 file_type { 0 };
+    u16 record_length { 0 };
 };
 
 static u8 to_ext2_file_type(mode_t mode)
@@ -1089,45 +1090,52 @@ KResult Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntr
     return KSuccess;
 }
 
-KResult Ext2FSInode::write_directory(const Vector<Ext2FSDirectoryEntry>& entries)
+KResult Ext2FSInode::write_directory(Vector<Ext2FSDirectoryEntry>& entries)
 {
     Locker locker(m_lock);
-
-    int directory_size = 0;
-    for (auto& entry : entries)
-        directory_size += EXT2_DIR_REC_LEN(entry.name.length());
-
     auto block_size = fs().block_size();
 
-    int blocks_needed = ceil_div(static_cast<size_t>(directory_size), block_size);
-    int occupied_size = blocks_needed * block_size;
-
-    dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_directory(): New directory contents to write (size {}, occupied {}):", identifier(), directory_size, occupied_size);
-
-    auto directory_data = ByteBuffer::create_uninitialized(occupied_size);
-    OutputMemoryStream stream { directory_data };
-
+    // Calculate directory size and record length of entries so that
+    // the following constraints are met:
+    // - All used blocks must be entirely filled.
+    // - Entries are aligned on a 4-byte boundary.
+    // - No entry may span multiple blocks.
+    size_t directory_size = 0;
+    size_t space_in_block = block_size;
     for (size_t i = 0; i < entries.size(); ++i) {
         auto& entry = entries[i];
+        entry.record_length = EXT2_DIR_REC_LEN(entry.name.length());
+        space_in_block -= entry.record_length;
+        if (i + 1 < entries.size()) {
+            if (EXT2_DIR_REC_LEN(entries[i + 1].name.length()) > space_in_block) {
+                entry.record_length += space_in_block;
+                space_in_block = block_size;
+            }
+        } else {
+            entry.record_length += space_in_block;
+        }
+        directory_size += entry.record_length;
+    }
 
-        int record_length = EXT2_DIR_REC_LEN(entry.name.length());
-        if (i == entries.size() - 1)
-            record_length += occupied_size - directory_size;
+    dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_directory(): New directory contents to write (size {}):", identifier(), directory_size);
 
-        dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_directory(): Writing inode: {}, name_len: {}, rec_len: {}, file_type: {}, name: {}", identifier(), entry.inode_index, u16(entry.name.length()), u16(record_length), u8(entry.file_type), entry.name);
+    auto directory_data = ByteBuffer::create_uninitialized(directory_size);
+    OutputMemoryStream stream { directory_data };
+
+    for (auto& entry : entries) {
+        dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_directory(): Writing inode: {}, name_len: {}, rec_len: {}, file_type: {}, name: {}", identifier(), entry.inode_index, u16(entry.name.length()), u16(entry.record_length), u8(entry.file_type), entry.name);
 
         stream << u32(entry.inode_index.value());
-        stream << u16(record_length);
+        stream << u16(entry.record_length);
         stream << u8(entry.name.length());
         stream << u8(entry.file_type);
         stream << entry.name.bytes();
-
-        int padding = record_length - entry.name.length() - 8;
+        int padding = entry.record_length - entry.name.length() - 8;
         for (int j = 0; j < padding; ++j)
             stream << u8(0);
     }
 
-    stream.fill_to_end(0);
+    VERIFY(stream.is_end());
 
     if (auto result = resize(stream.size()); result.is_error())
         return result;
diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h
index 6e666a4137..6027e28a12 100644
--- a/Kernel/FileSystem/Ext2FileSystem.h
+++ b/Kernel/FileSystem/Ext2FileSystem.h
@@ -58,7 +58,7 @@ private:
     virtual KResult truncate(u64) override;
     virtual KResultOr<int> get_block_address(int) override;
 
-    KResult write_directory(const Vector<Ext2FSDirectoryEntry>&);
+    KResult write_directory(Vector<Ext2FSDirectoryEntry>&);
     bool populate_lookup_cache() const;
     KResult resize(u64);
     KResult write_indirect_block(BlockBasedFS::BlockIndex, Span<BlockBasedFS::BlockIndex>);
-- 
cgit v1.2.3