mirrors/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-04-21 20:15:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

Kernel: Synchronize removals from TmpFS inode map

Browse source 
Previously we were uncaching inodes from TmpFSInode::one_ref_left().
This was not safe, since one_ref_left() was effectively being called
on a raw pointer after decrementing the local ref count and observing
it become 1. There was a race here where someone else could trigger
the destructor by unreffing to 0 before one_ref_left() got called,
causing us to call one_ref_left() on a deleted inode.

We fix this by using the new remove_from_secondary_lists() mechanism
in ListedRefCounted and synchronizing all access to the TmpFS inode
map with the main Inode::all_instances() lock.

There's probably a nicer way to solve this.
This commit is contained in:
Andreas Kling 2022-01-11 00:51:05 +01:00
parent 3550f12543
commit 08e927f084
Notes: sideshowbarker 2024-07-17 21:13:02 +09:00
3 changed files with 18 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Kernel/FileSystem/Inode.h
View file

@ -31,7 +31,7 @@ class Inode : public ListedRefCounted<Inode, LockType::Spinlock>
public:
virtual ~Inode();
virtual void one_ref_left() { }
virtual void remove_from_secondary_lists() { }
FileSystem& fs() { return m_file_system; }
FileSystem const& fs() const { return m_file_system; }

29
Kernel/FileSystem/TmpFS.cpp
View file

@ -37,19 +37,21 @@ Inode& TmpFS::root_inode()
void TmpFS::register_inode(TmpFSInode& inode)
{
MutexLocker locker(m_lock);
VERIFY(inode.identifier().fsid() == fsid());
auto index = inode.identifier().index();
m_inodes.set(index, inode);
Inode::all_instances().with([&](auto&) {
auto index = inode.identifier().index();
m_inodes.set(index, &inode);
});
}
void TmpFS::unregister_inode(InodeIdentifier identifier)
{
MutexLocker locker(m_lock);
VERIFY(identifier.fsid() == fsid());
m_inodes.remove(identifier.index());
Inode::all_instances().with([&](auto&) {
m_inodes.remove(identifier.index());
});
}
unsigned TmpFS::next_inode_index()
@ -61,13 +63,13 @@ unsigned TmpFS::next_inode_index()
ErrorOr<NonnullRefPtr<Inode>> TmpFS::get_inode(InodeIdentifier identifier) const
{
MutexLocker locker(m_lock, Mutex::Mode::Shared);
VERIFY(identifier.fsid() == fsid());
auto it = m_inodes.find(identifier.index());
if (it == m_inodes.end())
return ENOENT;
return it->value;
return Inode::all_instances().with([&](auto&) -> ErrorOr<NonnullRefPtr<Inode>> {
VERIFY(identifier.fsid() == fsid());
auto it = m_inodes.find(identifier.index());
if (it == m_inodes.end())
return ENOENT;
return *it->value;
});
}
TmpFSInode::TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, InodeIdentifier parent)
@ -363,9 +365,8 @@ ErrorOr<void> TmpFSInode::set_mtime(time_t t)
return {};
}
void TmpFSInode::one_ref_left()
void TmpFSInode::remove_from_secondary_lists()
{
// Destroy ourselves.
fs().unregister_inode(identifier());
}

4
Kernel/FileSystem/TmpFS.h
View file

@ -33,7 +33,7 @@ private:
RefPtr<TmpFSInode> m_root_inode;
HashMap<InodeIndex, NonnullRefPtr<TmpFSInode>> m_inodes;
HashMap<InodeIndex, TmpFSInode*> m_inodes;
ErrorOr<NonnullRefPtr<Inode>> get_inode(InodeIdentifier identifier) const;
void register_inode(TmpFSInode&);
void unregister_inode(InodeIdentifier);
@ -67,7 +67,7 @@ public:
virtual ErrorOr<void> set_atime(time_t) override;
virtual ErrorOr<void> set_ctime(time_t) override;
virtual ErrorOr<void> set_mtime(time_t) override;
virtual void one_ref_left() override;
virtual void remove_from_secondary_lists() override;
private:
TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, InodeIdentifier parent);