From 82b033271d7c9556aae5ab540c030102347951e2 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 7 Jun 2024 17:16:34 -0400 Subject: [PATCH] Move the ILP32 mremap() hackery into legacy_32_bit_support.cpp. Similar to mmap(), this confuses me every time I look at it. Move it out of the way, and make it clearer that this is just junk that can be deleted when we remove 32-bit support. Also improve coverage by adding a test for the varargs special case. Ignore-AOSP-First: work around the lack of an ABI 35 dump in AOSP Test: treehugger Change-Id: Ia375c29d18e31e646b795e643534f0be07d382b9 --- libc/Android.bp | 1 - libc/SYSCALLS.TXT | 5 ++- libc/bionic/legacy_32_bit_support.cpp | 26 ++++++++++++ libc/bionic/mremap.cpp | 58 --------------------------- tests/sys_mman_test.cpp | 14 +++++++ 5 files changed, 44 insertions(+), 60 deletions(-) delete mode 100644 libc/bionic/mremap.cpp diff --git a/libc/Android.bp b/libc/Android.bp index d8467a88a..943d41fba 100644 --- a/libc/Android.bp +++ b/libc/Android.bp @@ -879,7 +879,6 @@ cc_library_static { "bionic/mkfifo.cpp", "bionic/mknod.cpp", "bionic/mntent.cpp", - "bionic/mremap.cpp", "bionic/netdb.cpp", "bionic/net_if.cpp", "bionic/netinet_ether.cpp", diff --git a/libc/SYSCALLS.TXT b/libc/SYSCALLS.TXT index ce14e49c3..a8c70e515 100644 --- a/libc/SYSCALLS.TXT +++ b/libc/SYSCALLS.TXT @@ -118,7 +118,6 @@ ssize_t copy_file_range(int, off64_t*, int, off64_t*, size_t, unsigned int) pid_t __getpid:getpid() all int memfd_create(const char*, unsigned) all int munmap(void*, size_t) all -void* __mremap:mremap(void*, size_t, size_t, int, void*) all int msync(const void*, size_t, int) all int mprotect(const void*, size_t, int) all int madvise(void*, size_t, int) all @@ -190,6 +189,10 @@ int ftruncate|ftruncate64(int, off_t) lp64 void* __mmap2:mmap2(void*, size_t, int, int, int, long) lp32 void* mmap|mmap64(void*, size_t, int, int, int, off_t) lp64 +# mremap is in C++ for 32-bit so we can add the PTRDIFF_MAX check. +void* __mremap:mremap(void*, size_t, size_t, int, void*) lp32 +void* mremap(void*, size_t, size_t, int, void*) lp64 + # posix_fadvise64 is awkward: arm has shuffled arguments, # the POSIX functions don't set errno, and no architecture has posix_fadvise. int __arm_fadvise64_64:arm_fadvise64_64(int, int, off64_t, off64_t) arm diff --git a/libc/bionic/legacy_32_bit_support.cpp b/libc/bionic/legacy_32_bit_support.cpp index 314fe9bcb..4e19ebf02 100644 --- a/libc/bionic/legacy_32_bit_support.cpp +++ b/libc/bionic/legacy_32_bit_support.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -135,3 +136,28 @@ void* mmap64(void* addr, size_t size, int prot, int flags, int fd, off64_t offse void* mmap(void* addr, size_t size, int prot, int flags, int fd, off_t offset) { return mmap64(addr, size, prot, flags, fd, static_cast(offset)); } + +// The only difference here is that the libc API uses varargs for the +// optional `new_address` argument that's only used by MREMAP_FIXED. +extern "C" void* __mremap(void*, size_t, size_t, int, void*); + +void* mremap(void* old_address, size_t old_size, size_t new_size, int flags, ...) { + // Prevent allocations large enough for `end - start` to overflow, + // to avoid security bugs. + size_t rounded = __BIONIC_ALIGN(new_size, page_size()); + if (rounded < new_size || rounded > PTRDIFF_MAX) { + errno = ENOMEM; + return MAP_FAILED; + } + + // The optional argument is only valid if the MREMAP_FIXED flag is set, + // so we assume it's not present otherwise. + void* new_address = nullptr; + if ((flags & MREMAP_FIXED) != 0) { + va_list ap; + va_start(ap, flags); + new_address = va_arg(ap, void*); + va_end(ap); + } + return __mremap(old_address, old_size, new_size, flags, new_address); +} diff --git a/libc/bionic/mremap.cpp b/libc/bionic/mremap.cpp deleted file mode 100644 index 88ec829de..000000000 --- a/libc/bionic/mremap.cpp +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT - * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include -#include -#include -#include -#include - -#include "platform/bionic/macros.h" -#include "platform/bionic/page.h" - -extern "C" void* __mremap(void*, size_t, size_t, int, void*); - -void* mremap(void* old_address, size_t old_size, size_t new_size, int flags, ...) { - // prevent allocations large enough for `end - start` to overflow - size_t rounded = __BIONIC_ALIGN(new_size, page_size()); - if (rounded < new_size || rounded > PTRDIFF_MAX) { - errno = ENOMEM; - return MAP_FAILED; - } - - void* new_address = nullptr; - // The optional argument is only valid if the MREMAP_FIXED flag is set, - // so we assume it's not present otherwise. - if ((flags & MREMAP_FIXED) != 0) { - va_list ap; - va_start(ap, flags); - new_address = va_arg(ap, void*); - va_end(ap); - } - return __mremap(old_address, old_size, new_size, flags, new_address); -} diff --git a/tests/sys_mman_test.cpp b/tests/sys_mman_test.cpp index df13e07d3..40c85f2ca 100644 --- a/tests/sys_mman_test.cpp +++ b/tests/sys_mman_test.cpp @@ -243,6 +243,20 @@ TEST(sys_mman, mremap_PTRDIFF_MAX) { ASSERT_EQ(0, munmap(map, kPageSize)); } +TEST(sys_mman, mremap_MREMAP_FIXED) { + // We're not trying to test the kernel here; that's external/ltp's job. + // We just want to check that optional argument (mremap() is varargs) + // gets passed through in an MREMAP_FIXED call. + void* vma1 = mmap(NULL, getpagesize(), PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + ASSERT_NE(MAP_FAILED, vma1); + + void* vma2 = mmap(NULL, getpagesize(), PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + ASSERT_NE(MAP_FAILED, vma2); + + void* vma3 = mremap(vma1, getpagesize(), getpagesize(), MREMAP_FIXED | MREMAP_MAYMOVE, vma2); + ASSERT_EQ(vma2, vma3); +} + TEST(sys_mman, mmap_bug_27265969) { char* base = reinterpret_cast( mmap(nullptr, kPageSize * 2, PROT_EXEC | PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0));