From c3292f3fcbb3cd608cc19b7459751fa5bb64ab84 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 4 Nov 2016 10:52:13 -0700 Subject: otautil: Clean up SysUtil.cpp. Add unit testcases for sysMapFile(). Test: recovery_unit_test passes. Test: Build and use the new recovery image to sideload a package. Test: Build and use the new recovery image to install an update. Change-Id: I77d8f1ea151ab513865d992c256ba93a1fcb51a4 --- otautil/SysUtil.cpp | 339 ++++++++++++++++++++++---------------------- otautil/SysUtil.h | 47 +++--- tests/Android.mk | 13 +- tests/unit/sysutil_test.cpp | 140 ++++++++++++++++++ 4 files changed, 344 insertions(+), 195 deletions(-) create mode 100644 tests/unit/sysutil_test.cpp diff --git a/otautil/SysUtil.cpp b/otautil/SysUtil.cpp index 2936c5ca4..a8d5f4ad3 100644 --- a/otautil/SysUtil.cpp +++ b/otautil/SysUtil.cpp @@ -1,13 +1,25 @@ /* * Copyright 2006 The Android Open Source Project * - * System utilities. + * 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 "SysUtil.h" + #include #include #include #include -#include #include #include #include @@ -16,197 +28,190 @@ #include #include -#include +#include +#include +#include -#include "SysUtil.h" +#include +#include +#include +#include static bool sysMapFD(int fd, MemMapping* pMap) { - assert(pMap != NULL); + CHECK(pMap != nullptr); - struct stat sb; - if (fstat(fd, &sb) == -1) { - PLOG(ERROR) << "fstat(" << fd << ") failed"; - return false; - } + struct stat sb; + if (fstat(fd, &sb) == -1) { + PLOG(ERROR) << "fstat(" << fd << ") failed"; + return false; + } - void* memPtr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); - if (memPtr == MAP_FAILED) { - PLOG(ERROR) << "mmap(" << sb.st_size << ", R, PRIVATE, " << fd << ", 0) failed"; - return false; - } + void* memPtr = mmap(nullptr, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); + if (memPtr == MAP_FAILED) { + PLOG(ERROR) << "mmap(" << sb.st_size << ", R, PRIVATE, " << fd << ", 0) failed"; + return false; + } - pMap->addr = reinterpret_cast(memPtr); - pMap->length = sb.st_size; - pMap->range_count = 1; - pMap->ranges = reinterpret_cast(malloc(sizeof(MappedRange))); - if (pMap->ranges == NULL) { - PLOG(ERROR) << "malloc failed"; - munmap(memPtr, sb.st_size); - return false; - } - pMap->ranges[0].addr = memPtr; - pMap->ranges[0].length = sb.st_size; + pMap->addr = static_cast(memPtr); + pMap->length = sb.st_size; + pMap->ranges.push_back({ memPtr, static_cast(sb.st_size) }); - return true; + return true; } -static int sysMapBlockFile(FILE* mapf, MemMapping* pMap) -{ - char block_dev[PATH_MAX+1]; - size_t size; - unsigned int blksize; - size_t blocks; - unsigned int range_count; - unsigned int i; - - if (fgets(block_dev, sizeof(block_dev), mapf) == NULL) { - PLOG(ERROR) << "failed to read block device from header"; - return -1; +// A "block map" which looks like this (from uncrypt/uncrypt.cpp): +// +// /dev/block/platform/msm_sdcc.1/by-name/userdata # block device +// 49652 4096 # file size in bytes, block size +// 3 # count of block ranges +// 1000 1008 # block range 0 +// 2100 2102 # ... block range 1 +// 30 33 # ... block range 2 +// +// Each block range represents a half-open interval; the line "30 33" +// reprents the blocks [30, 31, 32]. +static int sysMapBlockFile(const char* filename, MemMapping* pMap) { + CHECK(pMap != nullptr); + + std::string content; + if (!android::base::ReadFileToString(filename, &content)) { + PLOG(ERROR) << "Failed to read " << filename; + return -1; + } + + std::vector lines = android::base::Split(android::base::Trim(content), "\n"); + if (lines.size() < 4) { + LOG(ERROR) << "Block map file is too short: " << lines.size(); + return -1; + } + + size_t size; + unsigned int blksize; + if (sscanf(lines[1].c_str(), "%zu %u", &size, &blksize) != 2) { + LOG(ERROR) << "Failed to parse file size and block size: " << lines[1]; + return -1; + } + + size_t range_count; + if (sscanf(lines[2].c_str(), "%zu", &range_count) != 1) { + LOG(ERROR) << "Failed to parse block map header: " << lines[2]; + return -1; + } + + size_t blocks; + if (blksize != 0) { + blocks = ((size - 1) / blksize) + 1; + } + if (size == 0 || blksize == 0 || blocks > SIZE_MAX / blksize || range_count == 0 || + lines.size() != 3 + range_count) { + LOG(ERROR) << "Invalid data in block map file: size " << size << ", blksize " << blksize + << ", range_count " << range_count << ", lines " << lines.size(); + return -1; + } + + // Reserve enough contiguous address space for the whole file. + void* reserve = mmap64(nullptr, blocks * blksize, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (reserve == MAP_FAILED) { + PLOG(ERROR) << "failed to reserve address space"; + return -1; + } + + const std::string& block_dev = lines[0]; + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(block_dev.c_str(), O_RDONLY))); + if (fd == -1) { + PLOG(ERROR) << "failed to open block device " << block_dev; + munmap(reserve, blocks * blksize); + return -1; + } + + pMap->ranges.resize(range_count); + + unsigned char* next = static_cast(reserve); + size_t remaining_size = blocks * blksize; + bool success = true; + for (size_t i = 0; i < range_count; ++i) { + const std::string& line = lines[i + 3]; + + size_t start, end; + if (sscanf(line.c_str(), "%zu %zu\n", &start, &end) != 2) { + LOG(ERROR) << "failed to parse range " << i << " in block map: " << line; + success = false; + break; } - for (i = 0; i < sizeof(block_dev); ++i) { - if (block_dev[i] == '\n') { - block_dev[i] = 0; - break; - } + size_t length = (end - start) * blksize; + if (end <= start || (end - start) > SIZE_MAX / blksize || length > remaining_size) { + LOG(ERROR) << "unexpected range in block map: " << start << " " << end; + success = false; + break; } - if (fscanf(mapf, "%zu %u\n%u\n", &size, &blksize, &range_count) != 3) { - LOG(ERROR) << "failed to parse block map header"; - return -1; - } - if (blksize != 0) { - blocks = ((size-1) / blksize) + 1; - } - if (size == 0 || blksize == 0 || blocks > SIZE_MAX / blksize || range_count == 0) { - LOG(ERROR) << "invalid data in block map file: size " << size << ", blksize " << blksize - << ", range_count " << range_count; - return -1; + void* addr = mmap64(next, length, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, + static_cast(start) * blksize); + if (addr == MAP_FAILED) { + PLOG(ERROR) << "failed to map block " << i; + success = false; + break; } + pMap->ranges[i].addr = addr; + pMap->ranges[i].length = length; - pMap->range_count = range_count; - pMap->ranges = reinterpret_cast(calloc(range_count, sizeof(MappedRange))); - if (pMap->ranges == NULL) { - PLOG(ERROR) << "calloc(" << range_count << ", " << sizeof(MappedRange) << ") failed"; - return -1; - } + next += length; + remaining_size -= length; + } + if (success && remaining_size != 0) { + LOG(ERROR) << "ranges in block map are invalid: remaining_size = " << remaining_size; + success = false; + } + if (!success) { + munmap(reserve, blocks * blksize); + return -1; + } - // Reserve enough contiguous address space for the whole file. - unsigned char* reserve = reinterpret_cast(mmap64(NULL, blocks * blksize, - PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0)); - if (reserve == MAP_FAILED) { - PLOG(ERROR) << "failed to reserve address space"; - free(pMap->ranges); - return -1; - } + pMap->addr = static_cast(reserve); + pMap->length = size; - int fd = open(block_dev, O_RDONLY); - if (fd < 0) { - PLOG(ERROR) << "failed to open block device " << block_dev; - munmap(reserve, blocks * blksize); - free(pMap->ranges); - return -1; - } + LOG(INFO) << "mmapped " << range_count << " ranges"; - unsigned char* next = reserve; - size_t remaining_size = blocks * blksize; - bool success = true; - for (i = 0; i < range_count; ++i) { - size_t start, end; - if (fscanf(mapf, "%zu %zu\n", &start, &end) != 2) { - LOG(ERROR) << "failed to parse range " << i << " in block map"; - success = false; - break; - } - size_t length = (end - start) * blksize; - if (end <= start || (end - start) > SIZE_MAX / blksize || length > remaining_size) { - LOG(ERROR) << "unexpected range in block map: " << start << " " << end; - success = false; - break; - } - - void* addr = mmap64(next, length, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd, ((off64_t)start)*blksize); - if (addr == MAP_FAILED) { - PLOG(ERROR) << "failed to map block " << i; - success = false; - break; - } - pMap->ranges[i].addr = addr; - pMap->ranges[i].length = length; - - next += length; - remaining_size -= length; - } - if (success && remaining_size != 0) { - LOG(ERROR) << "ranges in block map are invalid: remaining_size = " << remaining_size; - success = false; - } - if (!success) { - close(fd); - munmap(reserve, blocks * blksize); - free(pMap->ranges); - return -1; - } + return 0; +} - close(fd); - pMap->addr = reserve; - pMap->length = size; +int sysMapFile(const char* fn, MemMapping* pMap) { + if (fn == nullptr || pMap == nullptr) { + LOG(ERROR) << "Invalid argument(s)"; + return -1; + } - LOG(INFO) << "mmapped " << range_count << " ranges"; + *pMap = {}; - return 0; -} + if (fn[0] == '@') { + if (sysMapBlockFile(fn + 1, pMap) != 0) { + LOG(ERROR) << "Map of '" << fn << "' failed"; + return -1; + } + } else { + // This is a regular file. + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(fn, O_RDONLY))); + if (fd == -1) { + PLOG(ERROR) << "Unable to open '" << fn << "'"; + return -1; + } -int sysMapFile(const char* fn, MemMapping* pMap) -{ - memset(pMap, 0, sizeof(*pMap)); - - if (fn && fn[0] == '@') { - // A map of blocks - FILE* mapf = fopen(fn+1, "r"); - if (mapf == NULL) { - PLOG(ERROR) << "Unable to open '" << (fn+1) << "'"; - return -1; - } - - if (sysMapBlockFile(mapf, pMap) != 0) { - LOG(ERROR) << "Map of '" << fn << "' failed"; - fclose(mapf); - return -1; - } - - fclose(mapf); - } else { - // This is a regular file. - int fd = open(fn, O_RDONLY); - if (fd == -1) { - PLOG(ERROR) << "Unable to open '" << fn << "'"; - return -1; - } - - if (!sysMapFD(fd, pMap)) { - LOG(ERROR) << "Map of '" << fn << "' failed"; - close(fd); - return -1; - } - - close(fd); + if (!sysMapFD(fd, pMap)) { + LOG(ERROR) << "Map of '" << fn << "' failed"; + return -1; } - return 0; + } + return 0; } /* * Release a memory mapping. */ -void sysReleaseMap(MemMapping* pMap) -{ - int i; - for (i = 0; i < pMap->range_count; ++i) { - if (munmap(pMap->ranges[i].addr, pMap->ranges[i].length) < 0) { - PLOG(ERROR) << "munmap(" << pMap->ranges[i].addr << ", " << pMap->ranges[i].length - << ") failed"; - } +void sysReleaseMap(MemMapping* pMap) { + std::for_each(pMap->ranges.cbegin(), pMap->ranges.cend(), [](const MappedRange& range) { + if (munmap(range.addr, range.length) == -1) { + PLOG(ERROR) << "munmap(" << range.addr << ", " << range.length << ") failed"; } - free(pMap->ranges); - pMap->ranges = NULL; - pMap->range_count = 0; + }); + pMap->ranges.clear(); } diff --git a/otautil/SysUtil.h b/otautil/SysUtil.h index 7adff1e54..6a79bf31f 100644 --- a/otautil/SysUtil.h +++ b/otautil/SysUtil.h @@ -1,33 +1,40 @@ /* * Copyright 2006 The Android Open Source Project * - * System utilities. + * 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. */ -#ifndef _MINZIP_SYSUTIL -#define _MINZIP_SYSUTIL -#include +#ifndef _OTAUTIL_SYSUTIL +#define _OTAUTIL_SYSUTIL + #include -#ifdef __cplusplus -extern "C" { -#endif +#include -typedef struct MappedRange { - void* addr; - size_t length; -} MappedRange; +struct MappedRange { + void* addr; + size_t length; +}; /* * Use this to keep track of mapped segments. */ -typedef struct MemMapping { - unsigned char* addr; /* start of data */ - size_t length; /* length of data */ +struct MemMapping { + unsigned char* addr; /* start of data */ + size_t length; /* length of data */ - int range_count; - MappedRange* ranges; -} MemMapping; + std::vector ranges; +}; /* * Map a file into a private, read-only memory segment. If 'fn' @@ -45,8 +52,4 @@ int sysMapFile(const char* fn, MemMapping* pMap); */ void sysReleaseMap(MemMapping* pMap); -#ifdef __cplusplus -} -#endif - -#endif /*_MINZIP_SYSUTIL*/ +#endif // _OTAUTIL_SYSUTIL diff --git a/tests/Android.mk b/tests/Android.mk index 8f19992b6..86591f71d 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -18,7 +18,6 @@ LOCAL_PATH := $(call my-dir) # Unit tests include $(CLEAR_VARS) -LOCAL_CLANG := true LOCAL_CFLAGS := -Werror LOCAL_MODULE := recovery_unit_test LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk @@ -32,17 +31,19 @@ LOCAL_STATIC_LIBRARIES := \ libselinux \ libbase -LOCAL_SRC_FILES := unit/asn1_decoder_test.cpp -LOCAL_SRC_FILES += unit/recovery_test.cpp -LOCAL_SRC_FILES += unit/locale_test.cpp -LOCAL_SRC_FILES += unit/zip_test.cpp +LOCAL_SRC_FILES := \ + unit/asn1_decoder_test.cpp \ + unit/locale_test.cpp \ + unit/recovery_test.cpp \ + unit/sysutil_test.cpp \ + unit/zip_test.cpp + LOCAL_C_INCLUDES := bootable/recovery LOCAL_SHARED_LIBRARIES := liblog include $(BUILD_NATIVE_TEST) # Component tests include $(CLEAR_VARS) -LOCAL_CLANG := true LOCAL_CFLAGS := -Werror LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk LOCAL_MODULE := recovery_component_test diff --git a/tests/unit/sysutil_test.cpp b/tests/unit/sysutil_test.cpp new file mode 100644 index 000000000..f4699664b --- /dev/null +++ b/tests/unit/sysutil_test.cpp @@ -0,0 +1,140 @@ +/* + * Copyright 2016 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 +#include + +#include "otautil/SysUtil.h" + +TEST(SysUtilTest, InvalidArgs) { + MemMapping mapping; + + // Invalid argument. + ASSERT_EQ(-1, sysMapFile(nullptr, &mapping)); + ASSERT_EQ(-1, sysMapFile("/somefile", nullptr)); +} + +TEST(SysUtilTest, sysMapFileRegularFile) { + TemporaryFile temp_file1; + std::string content = "abc"; + ASSERT_TRUE(android::base::WriteStringToFile(content, temp_file1.path)); + + // sysMapFile() should map the file to one range. + MemMapping mapping; + ASSERT_EQ(0, sysMapFile(temp_file1.path, &mapping)); + ASSERT_NE(nullptr, mapping.addr); + ASSERT_EQ(content.size(), mapping.length); + ASSERT_EQ(1U, mapping.ranges.size()); + + sysReleaseMap(&mapping); + ASSERT_EQ(0U, mapping.ranges.size()); +} + +TEST(SysUtilTest, sysMapFileBlockMap) { + // Create a file that has 10 blocks. + TemporaryFile package; + std::string content; + constexpr size_t file_size = 4096 * 10; + content.reserve(file_size); + ASSERT_TRUE(android::base::WriteStringToFile(content, package.path)); + + TemporaryFile block_map_file; + std::string filename = std::string("@") + block_map_file.path; + MemMapping mapping; + + // One range. + std::string block_map_content = std::string(package.path) + "\n40960 4096\n1\n0 10\n"; + ASSERT_TRUE(android::base::WriteStringToFile(block_map_content, block_map_file.path)); + + ASSERT_EQ(0, sysMapFile(filename.c_str(), &mapping)); + ASSERT_EQ(file_size, mapping.length); + ASSERT_EQ(1U, mapping.ranges.size()); + + // It's okay to not have the trailing '\n'. + block_map_content = std::string(package.path) + "\n40960 4096\n1\n0 10"; + ASSERT_TRUE(android::base::WriteStringToFile(block_map_content, block_map_file.path)); + + ASSERT_EQ(0, sysMapFile(filename.c_str(), &mapping)); + ASSERT_EQ(file_size, mapping.length); + ASSERT_EQ(1U, mapping.ranges.size()); + + // Or having multiple trailing '\n's. + block_map_content = std::string(package.path) + "\n40960 4096\n1\n0 10\n\n\n"; + ASSERT_TRUE(android::base::WriteStringToFile(block_map_content, block_map_file.path)); + + ASSERT_EQ(0, sysMapFile(filename.c_str(), &mapping)); + ASSERT_EQ(file_size, mapping.length); + ASSERT_EQ(1U, mapping.ranges.size()); + + // Multiple ranges. + block_map_content = std::string(package.path) + "\n40960 4096\n3\n0 3\n3 5\n5 10\n"; + ASSERT_TRUE(android::base::WriteStringToFile(block_map_content, block_map_file.path)); + + ASSERT_EQ(0, sysMapFile(filename.c_str(), &mapping)); + ASSERT_EQ(file_size, mapping.length); + ASSERT_EQ(3U, mapping.ranges.size()); + + sysReleaseMap(&mapping); + ASSERT_EQ(0U, mapping.ranges.size()); +} + +TEST(SysUtilTest, sysMapFileBlockMapInvalidBlockMap) { + MemMapping mapping; + TemporaryFile temp_file; + std::string filename = std::string("@") + temp_file.path; + + // Block map file is too short. + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n4096 4096\n0\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + // Block map file has unexpected number of lines. + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n4096 4096\n1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n4096 4096\n2\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + // Invalid size/blksize/range_count. + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\nabc 4096\n1\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n4096 4096\n\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + // size/blksize/range_count don't match. + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n0 4096\n1\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n4096 0\n1\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + ASSERT_TRUE(android::base::WriteStringToFile("/somefile\n4096 4096\n0\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + // Invalid block dev path. + ASSERT_TRUE(android::base::WriteStringToFile("/doesntexist\n4096 4096\n1\n0 1\n", temp_file.path)); + ASSERT_EQ(-1, sysMapFile(filename.c_str(), &mapping)); + + sysReleaseMap(&mapping); + ASSERT_EQ(0U, mapping.ranges.size()); +} -- cgit v1.2.3