Kernel: Don't crash the kernel if ext2 encounters disk error

This will most likely result in a corrupted filesystem, but crashing the
kernel is too much :D

kernel/include/kernel/FS/Ext2/FileSystem.h

@@ -61,17 +61,17 @@ namespace Kernel
 		BAN::ErrorOr<void> initialize_root_inode();
 
 		BAN::ErrorOr<uint32_t> create_inode(const Ext2::Inode&);
-		void delete_inode(uint32_t ino);
+		BAN::ErrorOr<void> delete_inode(uint32_t ino);
 		BAN::ErrorOr<void> resize_inode(uint32_t, size_t);
 
-		void read_block(uint32_t, BlockBufferWrapper&);
+		BAN::ErrorOr<void> read_block(uint32_t, BlockBufferWrapper&);
-		void write_block(uint32_t, const BlockBufferWrapper&);
+		BAN::ErrorOr<void> write_block(uint32_t, const BlockBufferWrapper&);
-		void sync_superblock();
+		BAN::ErrorOr<void> sync_superblock();
 
 		BlockBufferWrapper get_block_buffer();
 
 		BAN::ErrorOr<uint32_t> reserve_free_block(uint32_t primary_bgd);
-		void release_block(uint32_t block);
+		BAN::ErrorOr<void> release_block(uint32_t block);
 
 		BAN::HashMap<ino_t, BAN::RefPtr<Ext2Inode>>& inode_cache() { return m_inode_cache; }
 
@@ -82,7 +82,7 @@ namespace Kernel
 			uint32_t block;
 			uint32_t offset;
 		};
-		BlockLocation locate_inode(uint32_t);
+		BAN::ErrorOr<BlockLocation> locate_inode(uint32_t);
 		BlockLocation locate_block_group_descriptior(uint32_t);
 
 		uint32_t block_size() const { return 1024 << superblock().log_block_size; }

kernel/include/kernel/FS/Ext2/Inode.h

@@ -52,19 +52,19 @@ namespace Kernel
 		// NOTE: the inode might have more blocks than what this suggests if it has been shrinked
 		uint32_t max_used_data_block_count() const { return size() / blksize(); }
 
-		BAN::Optional<uint32_t> block_from_indirect_block(uint32_t block, uint32_t index, uint32_t depth);
+		BAN::ErrorOr<BAN::Optional<uint32_t>> block_from_indirect_block(uint32_t block, uint32_t index, uint32_t depth);
-		BAN::Optional<uint32_t> fs_block_of_data_block_index(uint32_t data_block_index);
+		BAN::ErrorOr<BAN::Optional<uint32_t>> fs_block_of_data_block_index(uint32_t data_block_index);
 
 		BAN::ErrorOr<void> link_inode_to_directory(Ext2Inode&, BAN::StringView name);
 		BAN::ErrorOr<bool> is_directory_empty();
 
-		void cleanup_indirect_block(uint32_t block, uint32_t depth);
+		BAN::ErrorOr<void> cleanup_indirect_block(uint32_t block, uint32_t depth);
 		BAN::ErrorOr<void> cleanup_default_links();
-		void cleanup_from_fs();
+		BAN::ErrorOr<void> cleanup_from_fs();
 
 		BAN::ErrorOr<uint32_t> allocate_new_block_to_indirect_block(uint32_t& block, uint32_t index, uint32_t depth);
 		BAN::ErrorOr<uint32_t> allocate_new_block(uint32_t data_block_index);
-		void sync();
+		BAN::ErrorOr<void> sync();
 
 		uint32_t block_group() const;
 

kernel/kernel/FS/Ext2/FileSystem.cpp

@@ -63,7 +63,7 @@ namespace Kernel
 
 		if (m_superblock.rev_level == Ext2::Enum::GOOD_OLD_REV)
 		{
-			memset(m_superblock.__extension_start, 0, sizeof(Ext2::Superblock) - offsetof(Ext2::Superblock, Ext2::Superblock::__extension_start));
+			memset(m_superblock.__extension_start, 0, sizeof(Ext2::Superblock) - offsetof(Ext2::Superblock, __extension_start));
 			m_superblock.first_ino = Ext2::Enum::GOOD_OLD_FIRST_INO;
 			m_superblock.inode_size = Ext2::Enum::GOOD_OLD_INODE_SIZE;
 		}
@@ -140,7 +140,7 @@ namespace Kernel
 
 			for (uint32_t bg : m_superblock_backups)
 			{
-				read_block(superblock().first_data_block + superblock().blocks_per_group * bg, block_buffer);
+				TRY(read_block(superblock().first_data_block + superblock().blocks_per_group * bg, block_buffer));
 				Ext2::Superblock& superblock_backup = *(Ext2::Superblock*)block_buffer.data();
 				if (superblock_backup.magic != Ext2::Enum::SUPER_MAGIC)
 					derrorln("superblock backup at block {} is invalid ({4H})", bg, superblock_backup.magic);
@@ -184,7 +184,7 @@ namespace Kernel
 				current_group = ino_group;
 
 				bgd_location = locate_block_group_descriptior(current_group);
-				read_block(bgd_location.block, bgd_buffer);
+				TRY(read_block(bgd_location.block, bgd_buffer));
 
 				bgd = (Ext2::BlockGroupDescriptor*)(bgd_buffer.data() + bgd_location.offset);
 				if (bgd->free_inodes_count == 0)
@@ -193,7 +193,7 @@ namespace Kernel
 					continue;
 				}
 
-				read_block(bgd->inode_bitmap, inode_bitmap);
+				TRY(read_block(bgd->inode_bitmap, inode_bitmap));
 			}
 
 			const uint32_t ino_bitmap_byte = ino_index / 8;
@@ -202,16 +202,15 @@ namespace Kernel
 				continue;
 
 			inode_bitmap[ino_bitmap_byte] |= 1 << ino_bitmap_bit;
-			write_block(bgd->inode_bitmap, inode_bitmap);
+			TRY(write_block(bgd->inode_bitmap, inode_bitmap));
 
 			bgd->free_inodes_count--;
 			if (Inode::Mode(ext2_inode.mode).ifdir())
 				bgd->used_dirs_count++;
-			write_block(bgd_location.block, bgd_buffer);
+			TRY(write_block(bgd_location.block, bgd_buffer));
 
 			const uint32_t inode_table_offset = ino_index * superblock().inode_size;
-			const BlockLocation inode_location
+			const BlockLocation inode_location {
-			{
 				.block  = inode_table_offset / block_size + bgd->inode_table,
 				.offset = inode_table_offset % block_size
 			};
@@ -219,14 +218,14 @@ namespace Kernel
 			// NOTE: we don't need inode bitmap anymore, so we can reuse it
 			auto& inode_buffer = inode_bitmap;
 
-			read_block(inode_location.block, inode_buffer);
+			TRY(read_block(inode_location.block, inode_buffer));
 			memcpy(inode_buffer.data() + inode_location.offset, &ext2_inode, sizeof(Ext2::Inode));
 			if (superblock().inode_size > sizeof(Ext2::Inode))
 				memset(inode_buffer.data() + inode_location.offset + sizeof(Ext2::Inode), 0, superblock().inode_size - sizeof(Ext2::Inode));
-			write_block(inode_location.block, inode_buffer);
+			TRY(write_block(inode_location.block, inode_buffer));
 
 			m_superblock.free_inodes_count--;
-			sync_superblock();
+			TRY(sync_superblock());
 
 			return ino;
 		}
@@ -235,7 +234,7 @@ namespace Kernel
 		return BAN::Error::from_error_code(ErrorCode::Ext2_Corrupted);
 	}
 
-	void Ext2FS::delete_inode(uint32_t ino)
+	BAN::ErrorOr<void> Ext2FS::delete_inode(uint32_t ino)
 	{
 		LockGuard _(m_mutex);
 
@@ -250,20 +249,20 @@ namespace Kernel
 		const uint32_t inode_index = (ino - 1) % superblock().inodes_per_group;
 
 		auto bgd_location = locate_block_group_descriptior(inode_group);
-		read_block(bgd_location.block, bgd_buffer);
+		TRY(read_block(bgd_location.block, bgd_buffer));
 		auto& bgd = bgd_buffer.span().slice(bgd_location.offset).as<Ext2::BlockGroupDescriptor>();
 
 		// update inode bitmap
-		read_block(bgd.inode_bitmap, bitmap_buffer);
+		TRY(read_block(bgd.inode_bitmap, bitmap_buffer));
 		const uint32_t byte = inode_index / 8;
 		const uint32_t bit  = inode_index % 8;
 		ASSERT(bitmap_buffer[byte] & (1 << bit));
 		bitmap_buffer[byte] &= ~(1 << bit);
-		write_block(bgd.inode_bitmap, bitmap_buffer);
+		TRY(write_block(bgd.inode_bitmap, bitmap_buffer));
 
 		// memset inode to zero or fsck will complain
-		auto inode_location = locate_inode(ino);
+		auto inode_location = TRY(locate_inode(ino));
-		read_block(inode_location.block, inode_buffer);
+		TRY(read_block(inode_location.block, inode_buffer));
 		auto& inode = inode_buffer.span().slice(inode_location.offset).as<Ext2::Inode>();
 #if EXT2_VERIFY_NO_BLOCKS
 		static const char zero_buffer[sizeof(inode.block)] {};
@@ -271,24 +270,26 @@ namespace Kernel
 #endif
 		bool is_directory = Inode::Mode(inode.mode).ifdir();
 		memset(&inode, 0x00, m_superblock.inode_size);
-		write_block(inode_location.block, inode_buffer);
+		TRY(write_block(inode_location.block, inode_buffer));
 
 		// update bgd counts
 		bgd.free_inodes_count++;
 		if (is_directory)
 			bgd.used_dirs_count--;
-		write_block(bgd_location.block, bgd_buffer);
+		TRY(write_block(bgd_location.block, bgd_buffer));
 
 		// update superblock inode count
 		m_superblock.free_inodes_count++;
-		sync_superblock();
+		TRY(sync_superblock());
 
 		// remove inode from cache
 		if (m_inode_cache.contains(ino))
 			m_inode_cache.remove(ino);
+
+		return {};
 	}
 
-	void Ext2FS::read_block(uint32_t block, BlockBufferWrapper& buffer)
+	BAN::ErrorOr<void> Ext2FS::read_block(uint32_t block, BlockBufferWrapper& buffer)
 	{
 		LockGuard _(m_mutex);
 
@@ -298,10 +299,12 @@ namespace Kernel
 
 		ASSERT(block >= superblock().first_data_block + 1);
 		ASSERT(buffer.size() >= block_size);
-		MUST(m_block_device->read_blocks(block * sectors_per_block, sectors_per_block, buffer.span()));
+		TRY(m_block_device->read_blocks(block * sectors_per_block, sectors_per_block, buffer.span()));
+
+		return {};
 	}
 
-	void Ext2FS::write_block(uint32_t block, const BlockBufferWrapper& buffer)
+	BAN::ErrorOr<void> Ext2FS::write_block(uint32_t block, const BlockBufferWrapper& buffer)
 	{
 		LockGuard _(m_mutex);
 
@@ -311,10 +314,12 @@ namespace Kernel
 
 		ASSERT(block >= superblock().first_data_block + 1);
 		ASSERT(buffer.size() >= block_size);
-		MUST(m_block_device->write_blocks(block * sectors_per_block, sectors_per_block, buffer.span()));
+		TRY(m_block_device->write_blocks(block * sectors_per_block, sectors_per_block, buffer.span()));
+
+		return {};
 	}
 
-	void Ext2FS::sync_superblock()
+	BAN::ErrorOr<void> Ext2FS::sync_superblock()
 	{
 		LockGuard _(m_mutex);
 
@@ -331,12 +336,14 @@ namespace Kernel
 
 		auto superblock_buffer = get_block_buffer();
 
-		MUST(m_block_device->read_blocks(lba, sector_count, superblock_buffer.span()));
+		TRY(m_block_device->read_blocks(lba, sector_count, superblock_buffer.span()));
 		if (memcmp(superblock_buffer.data(), &m_superblock, superblock_bytes))
 		{
 			memcpy(superblock_buffer.data(), &m_superblock, superblock_bytes);
-			MUST(m_block_device->write_blocks(lba, sector_count, superblock_buffer.span()));
+			TRY(m_block_device->write_blocks(lba, sector_count, superblock_buffer.span()));
 		}
+
+		return {};
 	}
 
 	Ext2FS::BlockBufferWrapper Ext2FS::get_block_buffer()
@@ -356,16 +363,16 @@ namespace Kernel
 		auto block_bitmap = m_buffer_manager.get_buffer();
 
 		auto check_block_group =
-			[&](uint32_t block_group) -> uint32_t
+			[&](uint32_t block_group) -> BAN::ErrorOr<uint32_t>
 			{
 				auto bgd_location = locate_block_group_descriptior(block_group);
-				read_block(bgd_location.block, bgd_buffer);
+				TRY(read_block(bgd_location.block, bgd_buffer));
 
 				auto& bgd = *(Ext2::BlockGroupDescriptor*)(bgd_buffer.data() + bgd_location.offset);
 				if (bgd.free_blocks_count == 0)
 					return 0;
 
-				read_block(bgd.block_bitmap, block_bitmap);
+				TRY(read_block(bgd.block_bitmap, block_bitmap));
 				for (uint32_t block_offset = 0; block_offset < m_superblock.blocks_per_group; block_offset++)
 				{
 					const uint32_t fs_block_index = m_superblock.first_data_block + m_superblock.blocks_per_group * block_group + block_offset;
@@ -378,13 +385,13 @@ namespace Kernel
 						continue;
 
 					block_bitmap[byte] |= 1 << bit;
-					write_block(bgd.block_bitmap, block_bitmap);
+					TRY(write_block(bgd.block_bitmap, block_bitmap));
 
 					bgd.free_blocks_count--;
-					write_block(bgd_location.block, bgd_buffer);
+					TRY(write_block(bgd_location.block, bgd_buffer));
 
 					m_superblock.free_blocks_count--;
-					sync_superblock();
+					TRY(sync_superblock());
 
 					return fs_block_index;
 				}
@@ -393,20 +400,20 @@ namespace Kernel
 				return 0;
 			};
 
-		if (auto ret = check_block_group(primary_bgd))
+		if (auto ret = TRY(check_block_group(primary_bgd)))
 			return ret;
 
 		uint32_t number_of_block_groups = BAN::Math::div_round_up(m_superblock.blocks_count, m_superblock.blocks_per_group);
 		for (uint32_t block_group = 0; block_group < number_of_block_groups; block_group++)
 			if (block_group != primary_bgd)
-				if (auto ret = check_block_group(block_group))
+				if (auto ret = TRY(check_block_group(block_group)))
 					return ret;
 
 		derrorln("Corrupted file system. Superblock indicates free blocks but none were found.");
 		return BAN::Error::from_error_code(ErrorCode::Ext2_Corrupted);
 	}
 
-	void Ext2FS::release_block(uint32_t block)
+	BAN::ErrorOr<void> Ext2FS::release_block(uint32_t block)
 	{
 		LockGuard _(m_mutex);
 
@@ -420,26 +427,28 @@ namespace Kernel
 		auto bitmap_buffer = get_block_buffer();
 
 		auto bgd_location = locate_block_group_descriptior(block_group);
-		read_block(bgd_location.block, bgd_buffer);
+		TRY(read_block(bgd_location.block, bgd_buffer));
 
 		auto& bgd = bgd_buffer.span().slice(bgd_location.offset).as<Ext2::BlockGroupDescriptor>();
-		read_block(bgd.block_bitmap, bitmap_buffer);
+		TRY(read_block(bgd.block_bitmap, bitmap_buffer));
 
 		const uint32_t byte = block_offset / 8;
 		const uint32_t bit  = block_offset % 8;
 		ASSERT(bitmap_buffer[byte] & (1 << bit));
 
 		bitmap_buffer[byte] &= ~(1 << bit);
-		write_block(bgd.block_bitmap, bitmap_buffer);
+		TRY(write_block(bgd.block_bitmap, bitmap_buffer));
 
 		bgd.free_blocks_count++;
-		write_block(bgd_location.block, bgd_buffer);
+		TRY(write_block(bgd_location.block, bgd_buffer));
 
 		m_superblock.free_blocks_count++;
-		sync_superblock();
+		TRY(sync_superblock());
+
+		return {};
 	}
 
-	Ext2FS::BlockLocation Ext2FS::locate_inode(uint32_t ino)
+	BAN::ErrorOr<Ext2FS::BlockLocation> Ext2FS::locate_inode(uint32_t ino)
 	{
 		LockGuard _(m_mutex);
 
@@ -454,7 +463,7 @@ namespace Kernel
 
 		auto bgd_location = locate_block_group_descriptior(inode_group);
 
-		read_block(bgd_location.block, bgd_buffer);
+		TRY(read_block(bgd_location.block, bgd_buffer));
 
 		auto& bgd = *(Ext2::BlockGroupDescriptor*)(bgd_buffer.data() + bgd_location.offset);
 

kernel/kernel/FS/Ext2/Inode.cpp

@@ -27,10 +27,10 @@ namespace Kernel
 		if (fs.inode_cache().contains(inode_ino))
 			return fs.inode_cache()[inode_ino];
 
-		auto inode_location = fs.locate_inode(inode_ino);
+		auto inode_location = TRY(fs.locate_inode(inode_ino));
 
 		auto block_buffer = fs.get_block_buffer();
-		fs.read_block(inode_location.block, block_buffer);
+		TRY(fs.read_block(inode_location.block, block_buffer));
 
 		auto& inode = block_buffer.span().slice(inode_location.offset).as<Ext2::Inode>();
 
@@ -41,18 +41,20 @@ namespace Kernel
 
 	Ext2Inode::~Ext2Inode()
 	{
-		if (m_inode.links_count == 0)
+		if (m_inode.links_count > 0)
-			cleanup_from_fs();
+			return;
+		if (auto ret = cleanup_from_fs(); ret.is_error())
+			dwarnln("Could not cleanup inode from FS: {}", ret.error());
 	}
 
-	BAN::Optional<uint32_t> Ext2Inode::block_from_indirect_block(uint32_t block, uint32_t index, uint32_t depth)
+	BAN::ErrorOr<BAN::Optional<uint32_t>> Ext2Inode::block_from_indirect_block(uint32_t block, uint32_t index, uint32_t depth)
 	{
 		if (block == 0)
-			return {};
+			return BAN::Optional<uint32_t>();
 		ASSERT(depth >= 1);
 
 		auto block_buffer = m_fs.get_block_buffer();
-		m_fs.read_block(block, block_buffer);
+		TRY(m_fs.read_block(block, block_buffer));
 
 		const uint32_t indices_per_block = blksize() / sizeof(uint32_t);
 
@@ -60,22 +62,22 @@ namespace Kernel
 
 		const uint32_t next_block = block_buffer.span().as_span<uint32_t>()[(index / divisor) % indices_per_block];
 		if (next_block == 0)
-			return {};
+			return BAN::Optional<uint32_t>();
 		if (depth == 1)
-			return next_block;
+			return BAN::Optional<uint32_t>(next_block);
 
 		return block_from_indirect_block(next_block, index, depth - 1);
 	}
 
-	BAN::Optional<uint32_t> Ext2Inode::fs_block_of_data_block_index(uint32_t data_block_index)
+	BAN::ErrorOr<BAN::Optional<uint32_t>> Ext2Inode::fs_block_of_data_block_index(uint32_t data_block_index)
 	{
 		const uint32_t indices_per_block = blksize() / sizeof(uint32_t);
 
 		if (data_block_index < 12)
 		{
 			if (m_inode.block[data_block_index] == 0)
-				return {};
+				return BAN::Optional<uint32_t>();
-			return m_inode.block[data_block_index];
+			return BAN::Optional<uint32_t>(m_inode.block[data_block_index]);
 		}
 		data_block_index -= 12;
 
@@ -129,9 +131,9 @@ namespace Kernel
 
 		for (uint32_t data_block_index = first_block; data_block_index < last_block; data_block_index++)
 		{
-			auto block_index = fs_block_of_data_block_index(data_block_index);
+			auto block_index = TRY(fs_block_of_data_block_index(data_block_index));
 			if (block_index.has_value())
-				m_fs.read_block(block_index.value(), block_buffer);
+				TRY(m_fs.read_block(block_index.value(), block_buffer));
 			else
 				memset(block_buffer.data(), 0x00, block_buffer.size());
 
@@ -168,9 +170,9 @@ namespace Kernel
 		// Write partial block
 		if (offset % block_size)
 		{
-			auto block_index = fs_block_of_data_block_index(offset / block_size);
+			auto block_index = TRY(fs_block_of_data_block_index(offset / block_size));
 			if (block_index.has_value())
-				m_fs.read_block(block_index.value(), block_buffer);
+				TRY(m_fs.read_block(block_index.value(), block_buffer));
 			else
 			{
 				block_index = TRY(allocate_new_block(offset / block_size));;
@@ -181,7 +183,7 @@ namespace Kernel
 			uint32_t to_copy = BAN::Math::min<uint32_t>(block_size - block_offset, to_write);
 
 			memcpy(block_buffer.data() + block_offset, buffer.data(), to_copy);
-			m_fs.write_block(block_index.value(), block_buffer);
+			TRY(m_fs.write_block(block_index.value(), block_buffer));
 
 			written += to_copy;
 			offset += to_copy;
@@ -190,12 +192,12 @@ namespace Kernel
 
 		while (to_write >= block_size)
 		{
-			auto block_index = fs_block_of_data_block_index(offset / block_size);
+			auto block_index = TRY(fs_block_of_data_block_index(offset / block_size));
 			if (!block_index.has_value())
 				block_index = TRY(allocate_new_block(offset / block_size));
 
 			memcpy(block_buffer.data(), buffer.data() + written, block_buffer.size());
-			m_fs.write_block(block_index.value(), block_buffer);
+			TRY(m_fs.write_block(block_index.value(), block_buffer));
 
 			written += block_size;
 			offset += block_size;
@@ -204,9 +206,9 @@ namespace Kernel
 
 		if (to_write > 0)
 		{
-			auto block_index = fs_block_of_data_block_index(offset / block_size);
+			auto block_index = TRY(fs_block_of_data_block_index(offset / block_size));
 			if (block_index.has_value())
-				m_fs.read_block(block_index.value(), block_buffer);
+				TRY(m_fs.read_block(block_index.value(), block_buffer));
 			else
 			{
 				block_index = TRY(allocate_new_block(offset / block_size));
@@ -214,7 +216,7 @@ namespace Kernel
 			}
 
 			memcpy(block_buffer.data(), buffer.data() + written, to_write);
-			m_fs.write_block(block_index.value(), block_buffer);
+			TRY(m_fs.write_block(block_index.value(), block_buffer));
 		}
 
 		return buffer.size();
@@ -227,8 +229,14 @@ namespace Kernel
 
 		// TODO: we should remove unused blocks on shrink
 
+		const auto old_size = m_inode.size;
+
 		m_inode.size = new_size;
-		sync();
+		if (auto ret = sync(); ret.is_error())
+		{
+			m_inode.size = old_size;
+			return ret.release_error();
+		}
 
 		return {};
 	}
@@ -238,23 +246,31 @@ namespace Kernel
 		ASSERT((mode & Inode::Mode::TYPE_MASK) == 0);
 		if (m_inode.mode == mode)
 			return {};
+
+		const auto old_mode = m_inode.mode;
+
 		m_inode.mode = (m_inode.mode & Inode::Mode::TYPE_MASK) | mode;
-		sync();
+		if (auto ret = sync(); ret.is_error())
+		{
+			m_inode.mode = old_mode;
+			return ret.release_error();
+		}
+
 		return {};
 	}
 
-	void Ext2Inode::cleanup_indirect_block(uint32_t block, uint32_t depth)
+	BAN::ErrorOr<void> Ext2Inode::cleanup_indirect_block(uint32_t block, uint32_t depth)
 	{
 		ASSERT(block);
 
 		if (depth == 0)
 		{
-			m_fs.release_block(block);
+			TRY(m_fs.release_block(block));
-			return;
+			return {};
 		}
 
 		auto block_buffer = m_fs.get_block_buffer();
-		m_fs.read_block(block, block_buffer);
+		TRY(m_fs.read_block(block, block_buffer));
 
 		const uint32_t ids_per_block = blksize() / sizeof(uint32_t);
 		for (uint32_t i = 0; i < ids_per_block; i++)
@@ -262,13 +278,14 @@ namespace Kernel
 			const uint32_t next_block = block_buffer.span().as_span<uint32_t>()[i];
 			if (next_block == 0)
 				continue;
-			cleanup_indirect_block(next_block, depth - 1);
+			TRY(cleanup_indirect_block(next_block, depth - 1));
 		}
 
-		m_fs.release_block(block);
+		TRY(m_fs.release_block(block));
+		return {};
 	}
 
-	void Ext2Inode::cleanup_from_fs()
+	BAN::ErrorOr<void> Ext2Inode::cleanup_from_fs()
 	{
 		ASSERT(m_inode.links_count == 0);
 
@@ -278,15 +295,15 @@ namespace Kernel
 		// cleanup direct blocks
 		for (uint32_t i = 0; i < 12; i++)
 			if (m_inode.block[i])
-				m_fs.release_block(m_inode.block[i]);
+				TRY(m_fs.release_block(m_inode.block[i]));
 
 		// cleanup indirect blocks
 		if (m_inode.block[12])
-			cleanup_indirect_block(m_inode.block[12], 1);
+			TRY(cleanup_indirect_block(m_inode.block[12], 1));
 		if (m_inode.block[13])
-			cleanup_indirect_block(m_inode.block[13], 2);
+			TRY(cleanup_indirect_block(m_inode.block[13], 2));
 		if (m_inode.block[14])
-			cleanup_indirect_block(m_inode.block[14], 3);
+			TRY(cleanup_indirect_block(m_inode.block[14], 3));
 
 done:
 		// mark blocks as deleted
@@ -294,9 +311,11 @@ done:
 
 		// FIXME: this is only required since fs does not get
 		//        deleting inode from its cache
-		sync();
+		TRY(sync());
 
-		m_fs.delete_inode(ino());
+		TRY(m_fs.delete_inode(ino()));
+
+		return {};
 	}
 
 	BAN::ErrorOr<size_t> Ext2Inode::list_next_inodes_impl(off_t offset, struct dirent* list, size_t list_size)
@@ -308,11 +327,11 @@ done:
 			return 0;
 
 		// FIXME: can we actually assume directories have all their blocks allocated
-		const uint32_t block_index = fs_block_of_data_block_index(offset).value();
+		const uint32_t block_index = TRY(fs_block_of_data_block_index(offset)).value();
 
 		auto block_buffer = m_fs.get_block_buffer();
 
-		m_fs.read_block(block_index, block_buffer);
+		TRY(m_fs.read_block(block_index, block_buffer));
 
 		// First determine if we have big enough list
 		size_t entry_count = 0;
@@ -413,7 +432,7 @@ done:
 		auto inode_or_error = Ext2Inode::create(m_fs, new_ino);
 		if (inode_or_error.is_error())
 		{
-			m_fs.delete_inode(new_ino);
+			TRY(m_fs.delete_inode(new_ino));
 			return inode_or_error.release_error();
 		}
 
@@ -434,19 +453,21 @@ done:
 		auto inode_or_error = Ext2Inode::create(m_fs, new_ino);
 		if (inode_or_error.is_error())
 		{
-			m_fs.delete_inode(new_ino);
+			TRY(m_fs.delete_inode(new_ino));
 			return inode_or_error.release_error();
 		}
 
 		auto inode = inode_or_error.release_value();
-		BAN::ScopeGuard cleanup([&] { inode->cleanup_from_fs(); });
 
-		TRY(inode->link_inode_to_directory(*inode, "."_sv));
+		// link . and ..
-		TRY(inode->link_inode_to_directory(*this, ".."_sv));
+		if (auto ret = inode->link_inode_to_directory(*inode, "."_sv); ret.is_error())
+			return ({ TRY(inode->cleanup_from_fs()); ret.release_error(); });
+		if (auto ret = inode->link_inode_to_directory(*this, ".."_sv); ret.is_error())
+			return ({ TRY(inode->cleanup_from_fs()); ret.release_error(); });
 
-		TRY(link_inode_to_directory(*inode, name));
+		// link to parent
-
+		if (auto ret = link_inode_to_directory(*inode, name); ret.is_error())
-		cleanup.disable();
+			return ({ TRY(inode->cleanup_from_fs()); ret.release_error(); });
 
 		return {};
 	}
@@ -475,7 +496,8 @@ done:
 
 		auto block_buffer = m_fs.get_block_buffer();
 
-		auto write_inode = [&](uint32_t entry_offset, uint32_t entry_rec_len)
+		auto write_inode =
+			[&](uint32_t entry_offset, uint32_t entry_rec_len) -> BAN::ErrorOr<void>
 			{
 				auto typed_mode = inode.mode();
 				uint8_t file_type = (m_fs.superblock().rev_level == Ext2::Enum::GOOD_OLD_REV) ? 0
@@ -496,7 +518,9 @@ done:
 				memcpy(new_entry.name, name.data(), name.size());
 
 				inode.m_inode.links_count++;
-			inode.sync();
+				TRY(inode.sync());
+
+				return {};
 			};
 
 		uint32_t block_index = 0;
@@ -512,8 +536,8 @@ done:
 			goto needs_new_block;
 
 		// Try to insert inode to last data block
-		block_index = fs_block_of_data_block_index(data_block_count - 1).value();
+		block_index = TRY(fs_block_of_data_block_index(data_block_count - 1)).value();
-		m_fs.read_block(block_index, block_buffer);
+		TRY(m_fs.read_block(block_index, block_buffer));
 
 		while (entry_offset < block_size)
 		{
@@ -525,8 +549,8 @@ done:
 
 			if (entry.inode == 0 && needed_entry_len <= entry.rec_len)
 			{
-				write_inode(entry_offset, entry.rec_len);
+				TRY(write_inode(entry_offset, entry.rec_len));
-				m_fs.write_block(block_index, block_buffer);
+				TRY(m_fs.write_block(block_index, block_buffer));
 				return {};
 			}
 			else if (needed_entry_len <= entry.rec_len - entry_min_rec_len)
@@ -534,8 +558,8 @@ done:
 				uint32_t new_rec_len = entry.rec_len - entry_min_rec_len;
 				entry.rec_len = entry_min_rec_len;
 
-				write_inode(entry_offset + entry.rec_len, new_rec_len);
+				TRY(write_inode(entry_offset + entry.rec_len, new_rec_len));
-				m_fs.write_block(block_index, block_buffer);
+				TRY(m_fs.write_block(block_index, block_buffer));
 				return {};
 			}
 
@@ -547,8 +571,8 @@ needs_new_block:
 		m_inode.size += blksize();
 
 		memset(block_buffer.data(), 0x00, block_buffer.size());
-		write_inode(0, block_size);
+		TRY(write_inode(0, block_size));
-		m_fs.write_block(block_index, block_buffer);
+		TRY(m_fs.write_block(block_index, block_buffer));
 
 		return {};
 	}
@@ -563,8 +587,8 @@ needs_new_block:
 		for (uint32_t i = 0; i < max_used_data_block_count(); i++)
 		{
 			// FIXME: can we actually assume directories have all their blocks allocated
-			const uint32_t block_index = fs_block_of_data_block_index(i).value();
+			const uint32_t block_index = TRY(fs_block_of_data_block_index(i)).value();
-			m_fs.read_block(block_index, block_buffer);
+			TRY(m_fs.read_block(block_index, block_buffer));
 
 			blksize_t offset = 0;
 			while (offset < blksize())
@@ -599,8 +623,8 @@ needs_new_block:
 		for (uint32_t i = 0; i < max_used_data_block_count(); i++)
 		{
 			// FIXME: can we actually assume directories have all their blocks allocated
-			const uint32_t block_index = fs_block_of_data_block_index(i).value();
+			const uint32_t block_index = TRY(fs_block_of_data_block_index(i)).value();
-			m_fs.read_block(block_index, block_buffer);
+			TRY(m_fs.read_block(block_index, block_buffer));
 
 			bool modified = false;
 
@@ -616,13 +640,13 @@ needs_new_block:
 					if (entry_name == "."_sv)
 					{
 						m_inode.links_count--;
-						sync();
+						TRY(sync());
 					}
 					else if (entry_name == ".."_sv)
 					{
 						auto parent = TRY(Ext2Inode::create(m_fs, entry.inode));
 						parent->m_inode.links_count--;
-						parent->sync();
+						TRY(parent->sync());
 					}
 					else
 						ASSERT_NOT_REACHED();
@@ -635,7 +659,7 @@ needs_new_block:
 			}
 
 			if (modified)
-				m_fs.write_block(block_index, block_buffer);
+				TRY(m_fs.write_block(block_index, block_buffer));
 		}
 
 		return {};
@@ -655,8 +679,8 @@ needs_new_block:
 		for (uint32_t i = 0; i < max_used_data_block_count(); i++)
 		{
 			// FIXME: can we actually assume directories have all their blocks allocated
-			const uint32_t block_index = fs_block_of_data_block_index(i).value();
+			const uint32_t block_index = TRY(fs_block_of_data_block_index(i)).value();
-			m_fs.read_block(block_index, block_buffer);
+			TRY(m_fs.read_block(block_index, block_buffer));
 
 			blksize_t offset = 0;
 			while (offset < blksize())
@@ -677,7 +701,7 @@ needs_new_block:
 					else
 						inode->m_inode.links_count--;
 
-					sync();
+					TRY(sync());
 
 					// NOTE: If this was the last link to inode we must
 					//       remove it from inode cache to trigger cleanup
@@ -690,7 +714,7 @@ needs_new_block:
 
 					// FIXME: This should expand the last inode if exists
 					entry.inode = 0;
-					m_fs.write_block(block_index, block_buffer);
+					TRY(m_fs.write_block(block_index, block_buffer));
 				}
 				offset += entry.rec_len;
 			}
@@ -714,14 +738,14 @@ needs_new_block:
 
 			auto block_buffer = m_fs.get_block_buffer();
 			memset(block_buffer.data(), 0x00, block_buffer.size());
-			m_fs.write_block(block, block_buffer);
+			TRY(m_fs.write_block(block, block_buffer));
 		}
 
 		if (depth == 0)
 			return block;
 
 		auto block_buffer = m_fs.get_block_buffer();
-		m_fs.read_block(block, block_buffer);
+		TRY(m_fs.read_block(block, block_buffer));
 
 		uint32_t divisor = 1;
 		for (uint32_t i = 1; i < depth; i++)
@@ -730,7 +754,9 @@ needs_new_block:
 		uint32_t& new_block = block_buffer.span().as_span<uint32_t>()[(index / divisor) % indices_per_fs_block];
 
 		uint32_t allocated_block = TRY(allocate_new_block_to_indirect_block(new_block, index, depth - 1));
-		m_fs.write_block(block, block_buffer);
+		TRY(m_fs.write_block(block, block_buffer));
+
+		TRY(sync());
 
 		return allocated_block;
 	}
@@ -740,13 +766,12 @@ needs_new_block:
 		const uint32_t inode_blocks_per_fs_block = blksize() / 512;
 		const uint32_t indices_per_fs_block = blksize() / sizeof(uint32_t);
 
-		BAN::ScopeGuard syncer([&] { sync(); });
-
 		if (data_block_index < 12)
 		{
 			ASSERT(m_inode.block[data_block_index] == 0);
 			m_inode.block[data_block_index] = TRY(m_fs.reserve_free_block(block_group()));
 			m_inode.blocks += inode_blocks_per_fs_block;
+			TRY(sync());
 			return m_inode.block[data_block_index];
 		}
 		data_block_index -= 12;
@@ -765,17 +790,19 @@ needs_new_block:
 		ASSERT_NOT_REACHED();
 	}
 
-	void Ext2Inode::sync()
+	BAN::ErrorOr<void> Ext2Inode::sync()
 	{
-		auto inode_location = m_fs.locate_inode(ino());
+		auto inode_location = TRY(m_fs.locate_inode(ino()));
 		auto block_buffer = m_fs.get_block_buffer();
 
-		m_fs.read_block(inode_location.block, block_buffer);
+		TRY(m_fs.read_block(inode_location.block, block_buffer));
 		if (memcmp(block_buffer.data() + inode_location.offset, &m_inode, sizeof(Ext2::Inode)))
 		{
 			memcpy(block_buffer.data() + inode_location.offset, &m_inode, sizeof(Ext2::Inode));
-			m_fs.write_block(inode_location.block, block_buffer);
+			TRY(m_fs.write_block(inode_location.block, block_buffer));
 		}
+
+		return {};
 	}
 
 	BAN::ErrorOr<BAN::RefPtr<Inode>> Ext2Inode::find_inode_impl(BAN::StringView file_name)
@@ -787,8 +814,8 @@ needs_new_block:
 		for (uint32_t i = 0; i < max_used_data_block_count(); i++)
 		{
 			// FIXME: can we actually assume directories have all their blocks allocated
-			const uint32_t block_index = fs_block_of_data_block_index(i).value();
+			const uint32_t block_index = TRY(fs_block_of_data_block_index(i)).value();
-			m_fs.read_block(block_index, block_buffer);
+			TRY(m_fs.read_block(block_index, block_buffer));
 
 			BAN::ConstByteSpan entry_span = block_buffer.span();
 			while (entry_span.size() >= sizeof(Ext2::LinkedDirectoryEntry))