mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-04-21 20:15:17 +00:00
Fix VMO leak in Process::exec().
Gotta make sure things get cleaned up before we yield-teleport in exec(). Also VMOs and regions are now viewable through /proc/mm and /proc/regions.
This commit is contained in:
parent
cd1e7419f0
commit
7b96218389
Notes:
sideshowbarker
2024-07-19 18:31:30 +09:00
Author: https://github.com/awesomekling Commit: https://github.com/SerenityOS/serenity/commit/7b96218389c
4 changed files with 51 additions and 8 deletions
|
@ -619,6 +619,8 @@ Region::Region(LinearAddress a, size_t s, String&& n, bool r, bool w, bool cow)
|
|||
, is_writable(w)
|
||||
, cow_map(Bitmap::create(m_vmo->page_count(), cow))
|
||||
{
|
||||
m_vmo->set_name(name);
|
||||
MM.register_region(*this);
|
||||
}
|
||||
|
||||
Region::Region(LinearAddress a, size_t s, RetainPtr<VirtualFileSystem::Node>&& vnode, String&& n, bool r, bool w)
|
||||
|
@ -630,6 +632,7 @@ Region::Region(LinearAddress a, size_t s, RetainPtr<VirtualFileSystem::Node>&& v
|
|||
, is_writable(w)
|
||||
, cow_map(Bitmap::create(m_vmo->page_count()))
|
||||
{
|
||||
MM.register_region(*this);
|
||||
}
|
||||
|
||||
Region::Region(LinearAddress a, size_t s, RetainPtr<VMObject>&& vmo, size_t offset_in_vmo, String&& n, bool r, bool w, bool cow)
|
||||
|
@ -642,10 +645,12 @@ Region::Region(LinearAddress a, size_t s, RetainPtr<VMObject>&& vmo, size_t offs
|
|||
, is_writable(w)
|
||||
, cow_map(Bitmap::create(m_vmo->page_count(), cow))
|
||||
{
|
||||
MM.register_region(*this);
|
||||
}
|
||||
|
||||
Region::~Region()
|
||||
{
|
||||
MM.unregister_region(*this);
|
||||
}
|
||||
|
||||
void PhysicalPage::return_to_freelist()
|
||||
|
@ -738,10 +743,24 @@ int Region::commit(Process& process)
|
|||
|
||||
void MemoryManager::register_vmo(VMObject& vmo)
|
||||
{
|
||||
InterruptDisabler disabler;
|
||||
m_vmos.set(&vmo);
|
||||
}
|
||||
|
||||
void MemoryManager::unregister_vmo(VMObject& vmo)
|
||||
{
|
||||
InterruptDisabler disabler;
|
||||
m_vmos.remove(&vmo);
|
||||
}
|
||||
|
||||
void MemoryManager::register_region(Region& region)
|
||||
{
|
||||
InterruptDisabler disabler;
|
||||
m_regions.set(®ion);
|
||||
}
|
||||
|
||||
void MemoryManager::unregister_region(Region& region)
|
||||
{
|
||||
InterruptDisabler disabler;
|
||||
m_regions.remove(®ion);
|
||||
}
|
||||
|
|
|
@ -146,6 +146,7 @@ class MemoryManager {
|
|||
friend class Region;
|
||||
friend class VMObject;
|
||||
friend ByteBuffer procfs$mm();
|
||||
friend ByteBuffer procfs$regions();
|
||||
public:
|
||||
static MemoryManager& the() PURE;
|
||||
|
||||
|
@ -181,6 +182,8 @@ private:
|
|||
|
||||
void register_vmo(VMObject&);
|
||||
void unregister_vmo(VMObject&);
|
||||
void register_region(Region&);
|
||||
void unregister_region(Region&);
|
||||
|
||||
LinearAddress allocate_linear_address_range(size_t);
|
||||
void map_region_at_address(PageDirectory*, Region&, LinearAddress, bool user_accessible);
|
||||
|
@ -296,6 +299,7 @@ private:
|
|||
Vector<RetainPtr<PhysicalPage>> m_free_physical_pages;
|
||||
|
||||
HashTable<VMObject*> m_vmos;
|
||||
HashTable<Region*> m_regions;
|
||||
};
|
||||
|
||||
struct KernelPagingScope {
|
||||
|
|
|
@ -171,7 +171,12 @@ ByteBuffer procfs$mm()
|
|||
auto buffer = ByteBuffer::createUninitialized(1024 + 80 * MM.m_vmos.size());
|
||||
char* ptr = (char*)buffer.pointer();
|
||||
for (auto* vmo : MM.m_vmos) {
|
||||
ptr += ksprintf(ptr, "VMO: %p %s (p:%u, r:%u)\n", vmo, vmo->name().characters(), vmo->page_count(), vmo->retainCount());
|
||||
ptr += ksprintf(ptr, "VMO: %p %s(%u): p:%4u %s\n",
|
||||
vmo,
|
||||
vmo->is_anonymous() ? "anon" : "file",
|
||||
vmo->retainCount(),
|
||||
vmo->page_count(),
|
||||
vmo->name().characters());
|
||||
}
|
||||
ptr += ksprintf(ptr, "VMO count: %u\n", MM.m_vmos.size());
|
||||
ptr += ksprintf(ptr, "Free physical pages: %u\n", MM.m_free_physical_pages.size());
|
||||
|
@ -179,6 +184,22 @@ ByteBuffer procfs$mm()
|
|||
return buffer;
|
||||
}
|
||||
|
||||
ByteBuffer procfs$regions()
|
||||
{
|
||||
// FIXME: Implement
|
||||
InterruptDisabler disabler;
|
||||
auto buffer = ByteBuffer::createUninitialized(1024 + 80 * MM.m_regions.size());
|
||||
char* ptr = (char*)buffer.pointer();
|
||||
for (auto* region : MM.m_regions) {
|
||||
ptr += ksprintf(ptr, "Region: %p VMO=%p %s\n",
|
||||
region,
|
||||
®ion->vmo(),
|
||||
region->name.characters());
|
||||
}
|
||||
ptr += ksprintf(ptr, "Region count: %u\n", MM.m_regions.size());
|
||||
buffer.trim(ptr - (char*)buffer.pointer());
|
||||
return buffer;
|
||||
}
|
||||
|
||||
ByteBuffer procfs$mounts()
|
||||
{
|
||||
|
@ -302,6 +323,7 @@ bool ProcFileSystem::initialize()
|
|||
{
|
||||
SyntheticFileSystem::initialize();
|
||||
addFile(createGeneratedFile("mm", procfs$mm));
|
||||
addFile(createGeneratedFile("regions", procfs$regions));
|
||||
addFile(createGeneratedFile("mounts", procfs$mounts));
|
||||
addFile(createGeneratedFile("kmalloc", procfs$kmalloc));
|
||||
addFile(createGeneratedFile("summary", procfs$summary));
|
||||
|
|
|
@ -307,15 +307,11 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
|
|||
return -ENOTIMPL;
|
||||
}
|
||||
|
||||
{ // Put everything into a scope so things get cleaned up before we yield-teleport into the new executable.
|
||||
auto vmo = VMObject::create_file_backed(descriptor->vnode(), descriptor->metadata().size);
|
||||
vmo->set_name(descriptor->absolute_path());
|
||||
auto* region = allocate_region_with_vmo(LinearAddress(), descriptor->metadata().size, vmo.copyRef(), 0, "helper", true, false);
|
||||
|
||||
#if 0
|
||||
auto elfData = descriptor->readEntireFile();
|
||||
if (!elfData)
|
||||
return -EIO; // FIXME: Get a more detailed error from VFS.
|
||||
#endif
|
||||
|
||||
dword entry_eip = 0;
|
||||
PageDirectory* old_page_directory = m_page_directory;
|
||||
PageDirectory* new_page_directory = reinterpret_cast<PageDirectory*>(kmalloc_page_aligned(sizeof(PageDirectory)));
|
||||
|
@ -324,6 +320,7 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
|
|||
m_page_directory = new_page_directory;
|
||||
MM.enter_process_paging_scope(*this);
|
||||
|
||||
// FIXME: Should we consider doing on-demand paging here? Is it actually useful?
|
||||
bool success = region->page_in(*new_page_directory);
|
||||
|
||||
ASSERT(success);
|
||||
|
@ -400,10 +397,11 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
|
|||
#endif
|
||||
|
||||
set_state(Skip1SchedulerPass);
|
||||
} // Ready to yield-teleport!
|
||||
|
||||
if (current == this) {
|
||||
bool success = Scheduler::yield();
|
||||
ASSERT(success);
|
||||
ASSERT_NOT_REACHED();
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
|
Loading…
Add table
Reference in a new issue