Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eb0ef145 authored by Elliott Hughes's avatar Elliott Hughes
Browse files

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
parent a42af360

base/mapped_file.cpp

+10 −1
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


#include "android-base/mapped_file.h"



#include <errno.h>



namespace android {

namespace base {
@@ -50,7 +52,14 @@ std::unique_ptr<MappedFile> MappedFile::FromFd(int fd, off64_t offset, size_t le 
      new MappedFile{static_cast<char*>(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<MappedFile>(new MappedFile{nullptr, 0, 0});

    }

    return nullptr;

  }

  return std::unique_ptr<MappedFile>(new MappedFile{static_cast<char*>(base), length, slop});

#endif

}

base/mapped_file_test.cpp

+10 −1
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ 
#include <string>



#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());

}

libutils/FileMap.cpp

+14 −19
Original line number Diff line number Diff line
@@ -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,21 +187,24 @@ 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));

        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);

libutils/tests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ cc_test { 


    srcs: [

        "BitSet_test.cpp",

        "FileMap_test.cpp",

        "LruCache_test.cpp",

        "Mutex_test.cpp",

        "Singleton_test.cpp",

libutils/tests/FileMap_test.cpp

0 → 100644
+34 −0
Original line number Diff line number Diff line 
/*

 * 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 <gtest/gtest.h>



#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());

}