From 77f9272aaf7d882f37567069df13ade5adee9bbf Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Wed, 22 Dec 2021 12:23:06 +0100 Subject: [PATCH] Kernel+UE: Add MAP_FIXED_NOREPLACE mmap() flag This feature was introduced in version 4.17 of the Linux kernel, and while it's not specified by POSIX, I think it will be a nice addition to our system. MAP_FIXED_NOREPLACE provides a less error-prone alternative to MAP_FIXED: while regular fixed mappings would cause any intersecting ranges to be unmapped, MAP_FIXED_NOREPLACE returns EEXIST instead. This ensures that we don't corrupt our process's address space if something is already at the requested address. Note that the more portable way to do this is to use regular MAP_ANONYMOUS, and check afterwards whether the returned address matches what we wanted. This, however, has a large performance impact on programs like Wine which try to reserve large portions of the address space at once, as the non-matching addresses have to be unmapped separately. --- Base/usr/share/man/man2/pledge.md | 2 +- Kernel/API/POSIX/sys/mman.h | 1 + Kernel/Syscalls/mmap.cpp | 18 +++++++++--------- .../UserspaceEmulator/Emulator_syscalls.cpp | 5 +++-- .../DevTools/UserspaceEmulator/MmapRegion.cpp | 4 ++-- Userland/Utilities/strace.cpp | 3 ++- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/Base/usr/share/man/man2/pledge.md b/Base/usr/share/man/man2/pledge.md index a6572c7127..27efd6956c 100644 --- a/Base/usr/share/man/man2/pledge.md +++ b/Base/usr/share/man/man2/pledge.md @@ -53,7 +53,7 @@ If the process later attempts to use any system functionality it has previously * `recvfd`: Receive file descriptors over a local socket * `ptrace`: The [`ptrace`(2)](ptrace.md) syscall (\*) * `prot_exec`: [`mmap`(2)](mmap.md) and [`mprotect`(2)](mprotect.md) with `PROT_EXEC` -* `map_fixed`: [`mmap`(2)](mmap.md) with `MAP_FIXED` (\*) +* `map_fixed`: [`mmap`(2)](mmap.md) with `MAP_FIXED` or `MAP_FIXED_NOREPLACE` (\*) Promises marked with an asterisk (\*) are SerenityOS specific extensions not supported by the original OpenBSD `pledge()`. diff --git a/Kernel/API/POSIX/sys/mman.h b/Kernel/API/POSIX/sys/mman.h index 187da47020..e2e27c4f75 100644 --- a/Kernel/API/POSIX/sys/mman.h +++ b/Kernel/API/POSIX/sys/mman.h @@ -23,6 +23,7 @@ extern "C" { #define MAP_NORESERVE 0x80 #define MAP_RANDOMIZED 0x100 #define MAP_PURGEABLE 0x200 +#define MAP_FIXED_NOREPLACE 0x400 #define PROT_READ 0x1 #define PROT_WRITE 0x2 diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index b4e901cd75..a1bcad7ee3 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -135,7 +135,7 @@ ErrorOr Process::sys$mmap(Userspace use REQUIRE_PROMISE(prot_exec); } - if (prot & MAP_FIXED) { + if (prot & MAP_FIXED || prot & MAP_FIXED_NOREPLACE) { REQUIRE_PROMISE(map_fixed); } @@ -167,6 +167,7 @@ ErrorOr Process::sys$mmap(Userspace use bool map_fixed = flags & MAP_FIXED; bool map_noreserve = flags & MAP_NORESERVE; bool map_randomized = flags & MAP_RANDOMIZED; + bool map_fixed_noreplace = flags & MAP_FIXED_NOREPLACE; if (map_shared && map_private) return EINVAL; @@ -174,7 +175,7 @@ ErrorOr Process::sys$mmap(Userspace use if (!map_shared && !map_private) return EINVAL; - if (map_fixed && map_randomized) + if ((map_fixed || map_fixed_noreplace) && map_randomized) return EINVAL; if (!validate_mmap_prot(prot, map_stack, map_anonymous)) @@ -186,19 +187,18 @@ ErrorOr Process::sys$mmap(Userspace use Memory::Region* region = nullptr; auto range = TRY([&]() -> ErrorOr { - if (map_randomized) { + if (map_randomized) return address_space().page_directory().range_allocator().try_allocate_randomized(Memory::page_round_up(size), alignment); - } + + // If MAP_FIXED is specified, existing mappings that intersect the requested range are removed. + if (map_fixed) + TRY(address_space().unmap_mmap_range(VirtualAddress(addr), size)); auto range = address_space().try_allocate_range(VirtualAddress(addr), size, alignment); if (range.is_error()) { - if (addr && !map_fixed) { + if (addr && !(map_fixed || map_fixed_noreplace)) { // If there's an address but MAP_FIXED wasn't specified, the address is just a hint. range = address_space().try_allocate_range({}, size, alignment); - } else if (map_fixed) { - // If MAP_FIXED is specified, existing mappings that intersect the requested range are removed. - TRY(address_space().unmap_mmap_range(VirtualAddress(addr), size)); - range = address_space().try_allocate_range(VirtualAddress(addr), size, alignment); } } return range; diff --git a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp index 7a46c74e78..07ca7dade8 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp +++ b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp @@ -873,10 +873,11 @@ u32 Emulator::virt$mmap(u32 params_addr) Optional result; if (params.flags & MAP_RANDOMIZED) { result = m_range_allocator.allocate_randomized(requested_size, params.alignment); - } else if (params.flags & MAP_FIXED) { + } else if (params.flags & MAP_FIXED || params.flags & MAP_FIXED_NOREPLACE) { if (params.addr) { // If MAP_FIXED is specified, existing mappings that intersect the requested range are removed. - virt$munmap(params.addr, requested_size); + if (params.flags & MAP_FIXED) + virt$munmap(params.addr, requested_size); 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. diff --git a/Userland/DevTools/UserspaceEmulator/MmapRegion.cpp b/Userland/DevTools/UserspaceEmulator/MmapRegion.cpp index 9ddd3cfda3..4351e0763f 100644 --- a/Userland/DevTools/UserspaceEmulator/MmapRegion.cpp +++ b/Userland/DevTools/UserspaceEmulator/MmapRegion.cpp @@ -36,8 +36,8 @@ NonnullOwnPtr MmapRegion::create_anonymous(u32 base, u32 size, u32 p NonnullOwnPtr MmapRegion::create_file_backed(u32 base, u32 size, u32 prot, int flags, int fd, off_t offset, String name) { - // Since we put the memory to an arbitrary location, do not pass MAP_FIXED to the Kernel. - auto real_flags = flags & ~MAP_FIXED; + // Since we put the memory to an arbitrary location, do not pass MAP_FIXED and MAP_FIXED_NOREPLACE to the Kernel. + auto real_flags = flags & ~(MAP_FIXED | MAP_FIXED_NOREPLACE); auto* data = (u8*)mmap_with_name(nullptr, size, prot, real_flags, fd, offset, name.is_empty() ? nullptr : String::formatted("(UE) {}", name).characters()); VERIFY(data != MAP_FAILED); auto* shadow_data = (u8*)mmap_initialized(size, 1, "MmapRegion ShadowData"); diff --git a/Userland/Utilities/strace.cpp b/Userland/Utilities/strace.cpp index cc16d2dc0e..a945e084ac 100644 --- a/Userland/Utilities/strace.cpp +++ b/Userland/Utilities/strace.cpp @@ -634,7 +634,8 @@ static void format_recvmsg(FormattedSyscallBuilder& builder, int socket, struct struct MmapFlags : BitflagBase { static constexpr auto options = { BITFLAG(MAP_SHARED), BITFLAG(MAP_PRIVATE), BITFLAG(MAP_FIXED), BITFLAG(MAP_ANONYMOUS), - BITFLAG(MAP_RANDOMIZED), BITFLAG(MAP_STACK), BITFLAG(MAP_NORESERVE), BITFLAG(MAP_PURGEABLE) + BITFLAG(MAP_RANDOMIZED), BITFLAG(MAP_STACK), BITFLAG(MAP_NORESERVE), BITFLAG(MAP_PURGEABLE), + BITFLAG(MAP_FIXED_NOREPLACE) }; static constexpr StringView default_ = "MAP_FILE"; };