RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-05-31 08:48:11 +00:00
Code Issues Activity Actions

Kernel: Ensure Ext2FSInode's lookup is populated before using it

Browse source 
This fixes #8133.

Ext2FSInode::remove_child() searches the lookup cache, so if it's not
initialized, removing the child fails. If the child was a directory,
this led to it being corrupted and having 0 children.

I also added populate_lookup_cache to add_child. I hadn't seen any
bugs there, but if the cache wasn't populated before, adding that
one entry would make it think it was populated, so that would cause
bugs later.
This commit is contained in:
Sam Atkins 2021-06-19 14:12:11 +01:00 • committed by Andreas Kling
parent f1ac0b6a5a
commit ab7023dbe5
2 changed files with 15 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

22
Kernel/FileSystem/Ext2FileSystem.cpp
View file

@ -1209,6 +1209,9 @@ KResult Ext2FSInode::add_child(Inode& child, const StringView& name, mode_t mode
if (result.is_error()) if (result.is_error())
return result; return result;
if (auto populate_result = populate_lookup_cache(); populate_result.is_error())
return populate_result;
m_lookup_cache.set(name, child.index()); m_lookup_cache.set(name, child.index());
did_add_child(child.identifier(), name); did_add_child(child.identifier(), name);
return KSuccess; return KSuccess;
@ -1220,6 +1223,9 @@ KResult Ext2FSInode::remove_child(const StringView& name)
dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::remove_child(): Removing '{}'", identifier(), name); dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::remove_child(): Removing '{}'", identifier(), name);
VERIFY(is_directory()); VERIFY(is_directory());
if (auto populate_result = populate_lookup_cache(); populate_result.is_error())
return populate_result;
auto it = m_lookup_cache.find(name); auto it = m_lookup_cache.find(name);
if (it == m_lookup_cache.end()) if (it == m_lookup_cache.end())
return ENOENT; return ENOENT;
@ -1591,11 +1597,11 @@ KResultOr<NonnullRefPtr<Inode>> Ext2FS::create_inode(Ext2FSInode& parent_inode,
return new_inode.release_nonnull(); return new_inode.release_nonnull();
} }
bool Ext2FSInode::populate_lookup_cache() const KResult Ext2FSInode::populate_lookup_cache() const
{ {
Locker locker(m_lock); Locker locker(m_lock);
if (!m_lookup_cache.is_empty()) if (!m_lookup_cache.is_empty())
return true; return KSuccess;
HashMap<String, InodeIndex> children; HashMap<String, InodeIndex> children;
KResult result = traverse_as_directory([&children](auto& entry) { KResult result = traverse_as_directory([&children](auto& entry) {
@ -1604,19 +1610,18 @@ bool Ext2FSInode::populate_lookup_cache() const
}); });
if (!result.is_success()) if (!result.is_success())
return false; return result;
if (!m_lookup_cache.is_empty()) VERIFY(m_lookup_cache.is_empty());
return false;
m_lookup_cache = move(children); m_lookup_cache = move(children);
return true; return KSuccess;
} }
RefPtr<Inode> Ext2FSInode::lookup(StringView name) RefPtr<Inode> Ext2FSInode::lookup(StringView name)
{ {
VERIFY(is_directory()); VERIFY(is_directory());
dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]:lookup(): Looking up '{}'", identifier(), name); dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]:lookup(): Looking up '{}'", identifier(), name);
if (!populate_lookup_cache()) if (populate_lookup_cache().is_error())
return {}; return {};
Locker locker(m_lock); Locker locker(m_lock);
auto it = m_lookup_cache.find(name.hash(), [&](auto& entry) { return entry.key == name; }); auto it = m_lookup_cache.find(name.hash(), [&](auto& entry) { return entry.key == name; });
@ -1702,7 +1707,8 @@ KResultOr<size_t> Ext2FSInode::directory_entry_count() const
{ {
VERIFY(is_directory()); VERIFY(is_directory());
Locker locker(m_lock); Locker locker(m_lock);
populate_lookup_cache(); if (auto result = populate_lookup_cache(); result.is_error())
return KResultOr<size_t>(result);
return m_lookup_cache.size(); return m_lookup_cache.size();
} }

2
Kernel/FileSystem/Ext2FileSystem.h
View file

@ -59,7 +59,7 @@ private:
virtual KResultOr<int> get_block_address(int) override; virtual KResultOr<int> get_block_address(int) override;
KResult write_directory(Vector<Ext2FSDirectoryEntry>&); KResult write_directory(Vector<Ext2FSDirectoryEntry>&);
bool populate_lookup_cache() const; KResult populate_lookup_cache() const;
KResult resize(u64); KResult resize(u64);
KResult write_indirect_block(BlockBasedFS::BlockIndex, Span<BlockBasedFS::BlockIndex>); KResult write_indirect_block(BlockBasedFS::BlockIndex, Span<BlockBasedFS::BlockIndex>);
KResult grow_doubly_indirect_block(BlockBasedFS::BlockIndex, size_t, Span<BlockBasedFS::BlockIndex>, Vector<BlockBasedFS::BlockIndex>&, unsigned&); KResult grow_doubly_indirect_block(BlockBasedFS::BlockIndex, size_t, Span<BlockBasedFS::BlockIndex>, Vector<BlockBasedFS::BlockIndex>&, unsigned&);