Skip to content

Commit f071a3e

Browse files
committed
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. Test: treehugger Merged-in: Ia375c29d18e31e646b795e643534f0be07d382b9 Change-Id: Ia375c29d18e31e646b795e643534f0be07d382b9
1 parent 0af0c44 commit f071a3e

File tree

5 files changed

+44
-60
lines changed

5 files changed

+44
-60
lines changed

libc/Android.bp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,6 @@ cc_library_static {
879879
"bionic/mkfifo.cpp",
880880
"bionic/mknod.cpp",
881881
"bionic/mntent.cpp",
882-
"bionic/mremap.cpp",
883882
"bionic/netdb.cpp",
884883
"bionic/net_if.cpp",
885884
"bionic/netinet_ether.cpp",

libc/SYSCALLS.TXT

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ ssize_t copy_file_range(int, off64_t*, int, off64_t*, size_t, unsigned int)
118118
pid_t __getpid:getpid() all
119119
int memfd_create(const char*, unsigned) all
120120
int munmap(void*, size_t) all
121-
void* __mremap:mremap(void*, size_t, size_t, int, void*) all
122121
int msync(const void*, size_t, int) all
123122
int mprotect(const void*, size_t, int) all
124123
int madvise(void*, size_t, int) all
@@ -190,6 +189,10 @@ int ftruncate|ftruncate64(int, off_t) lp64
190189
void* __mmap2:mmap2(void*, size_t, int, int, int, long) lp32
191190
void* mmap|mmap64(void*, size_t, int, int, int, off_t) lp64
192191

192+
# mremap is in C++ for 32-bit so we can add the PTRDIFF_MAX check.
193+
void* __mremap:mremap(void*, size_t, size_t, int, void*) lp32
194+
void* mremap(void*, size_t, size_t, int, void*) lp64
195+
193196
# posix_fadvise64 is awkward: arm has shuffled arguments,
194197
# the POSIX functions don't set errno, and no architecture has posix_fadvise.
195198
int __arm_fadvise64_64:arm_fadvise64_64(int, int, off64_t, off64_t) arm

libc/bionic/legacy_32_bit_support.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
#include <errno.h>
3232
#include <fcntl.h>
33+
#include <stdarg.h>
3334
#include <stdint.h>
3435
#include <sys/mman.h>
3536
#include <sys/resource.h>
@@ -135,3 +136,28 @@ void* mmap64(void* addr, size_t size, int prot, int flags, int fd, off64_t offse
135136
void* mmap(void* addr, size_t size, int prot, int flags, int fd, off_t offset) {
136137
return mmap64(addr, size, prot, flags, fd, static_cast<off64_t>(offset));
137138
}
139+
140+
// The only difference here is that the libc API uses varargs for the
141+
// optional `new_address` argument that's only used by MREMAP_FIXED.
142+
extern "C" void* __mremap(void*, size_t, size_t, int, void*);
143+
144+
void* mremap(void* old_address, size_t old_size, size_t new_size, int flags, ...) {
145+
// Prevent allocations large enough for `end - start` to overflow,
146+
// to avoid security bugs.
147+
size_t rounded = __BIONIC_ALIGN(new_size, page_size());
148+
if (rounded < new_size || rounded > PTRDIFF_MAX) {
149+
errno = ENOMEM;
150+
return MAP_FAILED;
151+
}
152+
153+
// The optional argument is only valid if the MREMAP_FIXED flag is set,
154+
// so we assume it's not present otherwise.
155+
void* new_address = nullptr;
156+
if ((flags & MREMAP_FIXED) != 0) {
157+
va_list ap;
158+
va_start(ap, flags);
159+
new_address = va_arg(ap, void*);
160+
va_end(ap);
161+
}
162+
return __mremap(old_address, old_size, new_size, flags, new_address);
163+
}

libc/bionic/mremap.cpp

Lines changed: 0 additions & 58 deletions
This file was deleted.

tests/sys_mman_test.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,20 @@ TEST(sys_mman, mremap_PTRDIFF_MAX) {
243243
ASSERT_EQ(0, munmap(map, kPageSize));
244244
}
245245

246+
TEST(sys_mman, mremap_MREMAP_FIXED) {
247+
// We're not trying to test the kernel here; that's external/ltp's job.
248+
// We just want to check that optional argument (mremap() is varargs)
249+
// gets passed through in an MREMAP_FIXED call.
250+
void* vma1 = mmap(NULL, getpagesize(), PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
251+
ASSERT_NE(MAP_FAILED, vma1);
252+
253+
void* vma2 = mmap(NULL, getpagesize(), PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
254+
ASSERT_NE(MAP_FAILED, vma2);
255+
256+
void* vma3 = mremap(vma1, getpagesize(), getpagesize(), MREMAP_FIXED | MREMAP_MAYMOVE, vma2);
257+
ASSERT_EQ(vma2, vma3);
258+
}
259+
246260
TEST(sys_mman, mmap_bug_27265969) {
247261
char* base = reinterpret_cast<char*>(
248262
mmap(nullptr, kPageSize * 2, PROT_EXEC | PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0));

0 commit comments

Comments
 (0)