diff options
author | Ben Wiederhake <BenWiederhake.GitHub@gmx.de> | 2021-03-10 17:59:05 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-03-11 08:54:54 +0100 |
commit | 292871c4bc63cc7443d71083df603ee55e17945e (patch) | |
tree | 3380ee2136b4f255bc511140391dca2b22023ba9 /Userland | |
parent | 22d9bd0c456a18bbf04b0e2da7ae4128449eb19d (diff) | |
download | serenity-292871c4bc63cc7443d71083df603ee55e17945e.zip |
UserspaceEmulator: Handle MAP_FIXED with invalid addresses better
Old behavior: Crash due to VERIFY, unless we're completely and entirely out of
memory (m_available_ranges being empty), in which case it would return -ENOMEM.
New behavior: Return ENOMEM (and don't crash). In the case of nullptr,
also emit a helpful diagnostic.
Note that MAP_FIXED with nullptr is technically okay, but tends to be a sign
that something went wrong.
Also, this should improve mmap performance marginally, as it pulls the check out
of a loop that does not modify any parts of the check.
UE is now self-hosting! Fixes #5709.
However, this still needs some love: "ue UserspaceEmulator true" spits out tons
of error messages, probably false-positives, and takes about 229 seconds to run.
Then again, true-in-ue-in-ue-in-Qemu is three levels of emulation, so no wonder
it takes a long time! :D
Diffstat (limited to 'Userland')
-rw-r--r-- | Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp | 9 | ||||
-rw-r--r-- | Userland/DevTools/UserspaceEmulator/RangeAllocator.cpp | 5 |
2 files changed, 12 insertions, 2 deletions
diff --git a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp index dda6fced3d..553ba31a75 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp +++ b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp @@ -737,7 +737,14 @@ u32 Emulator::virt$mmap(u32 params_addr) if (params.flags & MAP_RANDOMIZED) { result = m_range_allocator.allocate_randomized(requested_size, params.alignment); } else if (params.flags & MAP_FIXED) { - result = m_range_allocator.allocate_specific(VirtualAddress { params.addr }, requested_size); + if (params.addr) + result = m_range_allocator.allocate_specific(VirtualAddress { params.addr }, requested_size); + else { + // mmap(nullptr, …, MAP_FIXED) is technically okay, but tends to be a bug. + // Therefore, refuse to be helpful. + reportln("\n=={}== \033[31;1mTried to mmap at nullptr with MAP_FIXED.\033[0m, 0x{:x} bytes.", params.size); + dump_backtrace(); + } } else { result = m_range_allocator.allocate_anywhere(requested_size, params.alignment); } diff --git a/Userland/DevTools/UserspaceEmulator/RangeAllocator.cpp b/Userland/DevTools/UserspaceEmulator/RangeAllocator.cpp index 7dfc8c6e49..60ba47e233 100644 --- a/Userland/DevTools/UserspaceEmulator/RangeAllocator.cpp +++ b/Userland/DevTools/UserspaceEmulator/RangeAllocator.cpp @@ -149,9 +149,12 @@ Optional<Range> RangeAllocator::allocate_specific(VirtualAddress base, size_t si VERIFY((size % PAGE_SIZE) == 0); Range allocated_range(base, size); + if (!m_total_range.contains(allocated_range)) { + dbgln("Unallocatable mmap request?! {:p}+{:p}", base.get(), size); + return {}; + } for (size_t i = 0; i < m_available_ranges.size(); ++i) { auto& available_range = m_available_ranges[i]; - VERIFY(m_total_range.contains(allocated_range)); if (!available_range.contains(base, size)) continue; if (available_range == allocated_range) { |