From 431166d99519f6794f10c48694913d2fe864b841 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 27 Jan 2014 16:28:31 -0800 Subject: [PATCH] Fix 32-bit mmap/mmap64 handling of negative offsets. We don't actually need to worry about sign extension if we reject negative values ourselves. Previously it was possible to come up with negative but aligned values that we would pass to the kernel; in the case of mmap (as opposed to mmap64) we'd incorrectly turn those into large positive offsets. Change-Id: I2aa583e0f892d59bb77429aea8730b72db32dcb0 --- libc/bionic/mmap.cpp | 8 +++----- tests/Android.mk | 1 + tests/sys_mman_test.cpp | 30 ++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 tests/sys_mman_test.cpp diff --git a/libc/bionic/mmap.cpp b/libc/bionic/mmap.cpp index 84a0f76e8..28a47cc9d 100644 --- a/libc/bionic/mmap.cpp +++ b/libc/bionic/mmap.cpp @@ -38,14 +38,12 @@ extern "C" void* __mmap2(void*, size_t, int, int, int, size_t); #define MMAP2_SHIFT 12 // 2**12 == 4096 void* mmap64(void* addr, size_t size, int prot, int flags, int fd, off64_t offset) { - if (offset & ((1UL << MMAP2_SHIFT)-1)) { + if (offset < 0 || (offset & ((1UL << MMAP2_SHIFT)-1)) != 0) { errno = EINVAL; return MAP_FAILED; } - uint64_t unsigned_offset = static_cast(offset); // To avoid sign extension. - void* result = __mmap2(addr, size, prot, flags, fd, unsigned_offset >> MMAP2_SHIFT); - + void* result = __mmap2(addr, size, prot, flags, fd, offset >> MMAP2_SHIFT); if (result != MAP_FAILED && (flags & (MAP_PRIVATE | MAP_ANONYMOUS)) != 0) { ErrnoRestorer errno_restorer; madvise(result, size, MADV_MERGEABLE); @@ -55,5 +53,5 @@ 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) & 0xffffffff); + return mmap64(addr, size, prot, flags, fd, static_cast(offset)); } diff --git a/tests/Android.mk b/tests/Android.mk index 49856435e..ee22c3520 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -61,6 +61,7 @@ test_src_files = \ strings_test.cpp \ stubs_test.cpp \ sys_epoll_test.cpp \ + sys_mman_test.cpp \ sys_resource_test.cpp \ sys_select_test.cpp \ sys_sendfile_test.cpp \ diff --git a/tests/sys_mman_test.cpp b/tests/sys_mman_test.cpp new file mode 100644 index 000000000..57067d70f --- /dev/null +++ b/tests/sys_mman_test.cpp @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include + +TEST(sys_mman, mmap_negative) { + off_t off = -sysconf(_SC_PAGESIZE); // Aligned but negative. + ASSERT_EQ(MAP_FAILED, mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, -1, off)); +} + +TEST(sys_mman, mmap64_negative) { + off64_t off64 = -sysconf(_SC_PAGESIZE); // Aligned but negative. + ASSERT_EQ(MAP_FAILED, mmap64(NULL, 4096, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, -1, off64)); +}