From eb0ef145fd38922617e6ab3d94eaacde93e04c95 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 6 Feb 2019 14:28:32 -0800 Subject: [PATCH] MappedFile and FileMap should support zero-length mappings. Bug: http://b/119818070 "app crashes when reading asset of zero length" Test: ran tests Change-Id: Idd2ad6f6e72c8e445aff78a460fac96dea41c950 --- base/mapped_file.cpp | 11 ++++++++++- base/mapped_file_test.cpp | 11 ++++++++++- libutils/FileMap.cpp | 33 ++++++++++++++------------------ libutils/tests/Android.bp | 1 + libutils/tests/FileMap_test.cpp | 34 +++++++++++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 21 deletions(-) create mode 100644 libutils/tests/FileMap_test.cpp diff --git a/base/mapped_file.cpp b/base/mapped_file.cpp index f7901afa4..faa845d14 100644 --- a/base/mapped_file.cpp +++ b/base/mapped_file.cpp @@ -16,6 +16,8 @@ #include "android-base/mapped_file.h" +#include + namespace android { namespace base { @@ -50,7 +52,14 @@ std::unique_ptr MappedFile::FromFd(int fd, off64_t offset, size_t le new MappedFile{static_cast(base), length, slop, handle}); #else void* base = mmap(nullptr, file_length, prot, MAP_SHARED, fd, file_offset); - if (base == MAP_FAILED) return nullptr; + if (base == MAP_FAILED) { + // http://b/119818070 "app crashes when reading asset of zero length". + // mmap fails with EINVAL for a zero length region. + if (errno == EINVAL && length == 0) { + return std::unique_ptr(new MappedFile{nullptr, 0, 0}); + } + return nullptr; + } return std::unique_ptr(new MappedFile{static_cast(base), length, slop}); #endif } diff --git a/base/mapped_file_test.cpp b/base/mapped_file_test.cpp index 7e89723c6..cfde73c74 100644 --- a/base/mapped_file_test.cpp +++ b/base/mapped_file_test.cpp @@ -25,7 +25,6 @@ #include #include "android-base/file.h" -#include "android-base/unique_fd.h" TEST(mapped_file, smoke) { TemporaryFile tf; @@ -37,3 +36,13 @@ TEST(mapped_file, smoke) { ASSERT_EQ('l', m->data()[0]); ASSERT_EQ('o', m->data()[1]); } + +TEST(mapped_file, zero_length_mapping) { + // http://b/119818070 "app crashes when reading asset of zero length". + // mmap fails with EINVAL for a zero length region. + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + + auto m = android::base::MappedFile::FromFd(tf.fd, 4096, 0, PROT_READ); + ASSERT_EQ(0u, m->size()); +} diff --git a/libutils/FileMap.cpp b/libutils/FileMap.cpp index 5feb2aa76..1202c156d 100644 --- a/libutils/FileMap.cpp +++ b/libutils/FileMap.cpp @@ -174,12 +174,6 @@ bool FileMap::create(const char* origFileName, int fd, off64_t offset, size_t le return false; } #else // !defined(__MINGW32__) - int prot, flags, adjust; - off64_t adjOffset; - size_t adjLength; - - void* ptr; - assert(fd >= 0); assert(offset >= 0); assert(length > 0); @@ -193,20 +187,23 @@ bool FileMap::create(const char* origFileName, int fd, off64_t offset, size_t le } } - adjust = offset % mPageSize; - adjOffset = offset - adjust; - adjLength = length + adjust; + int adjust = offset % mPageSize; + off64_t adjOffset = offset - adjust; + size_t adjLength = length + adjust; - flags = MAP_SHARED; - prot = PROT_READ; - if (!readOnly) - prot |= PROT_WRITE; + int flags = MAP_SHARED; + int prot = PROT_READ; + if (!readOnly) prot |= PROT_WRITE; - ptr = mmap(nullptr, adjLength, prot, flags, fd, adjOffset); + void* ptr = mmap(nullptr, adjLength, prot, flags, fd, adjOffset); if (ptr == MAP_FAILED) { - ALOGE("mmap(%lld,%zu) failed: %s\n", - (long long)adjOffset, adjLength, strerror(errno)); - return false; + if (errno == EINVAL && length == 0) { + ptr = nullptr; + adjust = 0; + } else { + ALOGE("mmap(%lld,%zu) failed: %s\n", (long long)adjOffset, adjLength, strerror(errno)); + return false; + } } mBasePtr = ptr; #endif // !defined(__MINGW32__) @@ -217,8 +214,6 @@ bool FileMap::create(const char* origFileName, int fd, off64_t offset, size_t le mDataPtr = (char*) mBasePtr + adjust; mDataLength = length; - assert(mBasePtr != NULL); - ALOGV("MAP: base %p/%zu data %p/%zu\n", mBasePtr, mBaseLength, mDataPtr, mDataLength); diff --git a/libutils/tests/Android.bp b/libutils/tests/Android.bp index 1390552fa..62f5acb4e 100644 --- a/libutils/tests/Android.bp +++ b/libutils/tests/Android.bp @@ -22,6 +22,7 @@ cc_test { srcs: [ "BitSet_test.cpp", + "FileMap_test.cpp", "LruCache_test.cpp", "Mutex_test.cpp", "Singleton_test.cpp", diff --git a/libutils/tests/FileMap_test.cpp b/libutils/tests/FileMap_test.cpp new file mode 100644 index 000000000..576d89bbe --- /dev/null +++ b/libutils/tests/FileMap_test.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2019 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 "utils/FileMap.h" + +#include + +#include "android-base/file.h" + +TEST(FileMap, zero_length_mapping) { + // http://b/119818070 "app crashes when reading asset of zero length". + // mmap fails with EINVAL for a zero length region. + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + + android::FileMap m; + ASSERT_TRUE(m.create("test", tf.fd, 4096, 0, true)); + ASSERT_STREQ("test", m.getFileName()); + ASSERT_EQ(0u, m.getDataLength()); + ASSERT_EQ(4096, m.getDataOffset()); +}