3 files changed, 192 insertions, 71 deletions

diff --git a/otautil/include/otautil/sysutil.h b/otautil/include/otautil/sysutil.h
index 2eeb7c302..692a99e9d 100644
--- a/otautil/include/otautil/sysutil.h
+++ b/otautil/include/otautil/sysutil.h
@@ -22,6 +22,57 @@
 #include <string>
 #include <vector>
 
+#include "rangeset.h"
+
+// This class holds the content of a block map file.
+class BlockMapData {
+ public:
+  // 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 BlockMapData ParseBlockMapFile(const std::string& block_map_path);
+
+  explicit operator bool() const {
+    return !path_.empty();
+  }
+
+  std::string path() const {
+    return path_;
+  }
+  uint64_t file_size() const {
+    return file_size_;
+  }
+  uint32_t block_size() const {
+    return block_size_;
+  }
+  RangeSet block_ranges() const {
+    return block_ranges_;
+  }
+
+ private:
+  BlockMapData() = default;
+
+  BlockMapData(const std::string& path, uint64_t file_size, uint32_t block_size,
+               RangeSet block_ranges)
+      : path_(path),
+        file_size_(file_size),
+        block_size_(block_size),
+        block_ranges_(std::move(block_ranges)) {}
+
+  std::string path_;
+  uint64_t file_size_ = 0;
+  uint32_t block_size_ = 0;
+  RangeSet block_ranges_;
+};
+
 /*
  * Use this to keep track of mapped segments.
  */
diff --git a/otautil/sysutil.cpp b/otautil/sysutil.cpp
index d8969a0bb..8366fa0ac 100644
--- a/otautil/sysutil.cpp
+++ b/otautil/sysutil.cpp
@@ -18,12 +18,13 @@
 
 #include <errno.h>  // TEMP_FAILURE_RETRY
 #include <fcntl.h>
-#include <stdint.h>  // SIZE_MAX
+#include <inttypes.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 
 #include <algorithm>
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -34,6 +35,68 @@
 #include <android-base/unique_fd.h>
 #include <cutils/android_reboot.h>
 
+BlockMapData BlockMapData::ParseBlockMapFile(const std::string& block_map_path) {
+  std::string content;
+  if (!android::base::ReadFileToString(block_map_path, &content)) {
+    LOG(ERROR) << "Failed to read " << block_map_path;
+    return {};
+  }
+
+  std::vector<std::string> lines = android::base::Split(android::base::Trim(content), "\n");
+  if (lines.size() < 4) {
+    LOG(ERROR) << "Block map file is too short: " << lines.size();
+    return {};
+  }
+
+  const std::string& block_dev = lines[0];
+
+  uint64_t file_size;
+  uint32_t blksize;
+  if (sscanf(lines[1].c_str(), "%" SCNu64 "%" SCNu32, &file_size, &blksize) != 2) {
+    LOG(ERROR) << "Failed to parse file size and block size: " << lines[1];
+    return {};
+  }
+
+  if (file_size == 0 || blksize == 0) {
+    LOG(ERROR) << "Invalid size in block map file: size " << file_size << ", blksize " << blksize;
+    return {};
+  }
+
+  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 {};
+  }
+
+  uint64_t blocks = ((file_size - 1) / blksize) + 1;
+  if (blocks > std::numeric_limits<uint32_t>::max() || range_count == 0 ||
+      lines.size() != 3 + range_count) {
+    LOG(ERROR) << "Invalid data in block map file: size " << file_size << ", blksize " << blksize
+               << ", range_count " << range_count << ", lines " << lines.size();
+    return {};
+  }
+
+  RangeSet ranges;
+  uint64_t remaining_blocks = blocks;
+  for (size_t i = 0; i < range_count; ++i) {
+    const std::string& line = lines[i + 3];
+    uint64_t start, end;
+    if (sscanf(line.c_str(), "%" SCNu64 "%" SCNu64, &start, &end) != 2) {
+      LOG(ERROR) << "failed to parse range " << i << ": " << line;
+      return {};
+    }
+    uint64_t range_blocks = end - start;
+    if (end <= start || range_blocks > remaining_blocks) {
+      LOG(ERROR) << "Invalid range: " << start << " " << end;
+      return {};
+    }
+    ranges.PushBack({ start, end });
+    remaining_blocks -= range_blocks;
+  }
+
+  return BlockMapData(block_dev, file_size, blksize, std::move(ranges));
+}
+
 bool MemMapping::MapFD(int fd) {
   struct stat sb;
   if (fstat(fd, &sb) == -1) {
@@ -55,115 +118,61 @@ bool MemMapping::MapFD(int fd) {
   return true;
 }
 
-// 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].
 bool MemMapping::MapBlockFile(const std::string& filename) {
-  std::string content;
-  if (!android::base::ReadFileToString(filename, &content)) {
-    PLOG(ERROR) << "Failed to read " << filename;
-    return false;
-  }
-
-  std::vector<std::string> lines = android::base::Split(android::base::Trim(content), "\n");
-  if (lines.size() < 4) {
-    LOG(ERROR) << "Block map file is too short: " << lines.size();
-    return false;
-  }
-
-  size_t size;
-  size_t blksize;
-  if (sscanf(lines[1].c_str(), "%zu %zu", &size, &blksize) != 2) {
-    LOG(ERROR) << "Failed to parse file size and block size: " << lines[1];
-    return false;
-  }
-
-  size_t range_count;
-  if (sscanf(lines[2].c_str(), "%zu", &range_count) != 1) {
-    LOG(ERROR) << "Failed to parse block map header: " << lines[2];
+  auto block_map_data = BlockMapData::ParseBlockMapFile(filename);
+  if (!block_map_data) {
     return false;
   }
 
-  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();
+  if (block_map_data.file_size() > std::numeric_limits<size_t>::max()) {
+    LOG(ERROR) << "File size is too large for mmap " << block_map_data.file_size();
     return false;
   }
 
   // Reserve enough contiguous address space for the whole file.
+  uint32_t blksize = block_map_data.block_size();
+  uint64_t blocks = ((block_map_data.file_size() - 1) / blksize) + 1;
   void* reserve = mmap(nullptr, blocks * blksize, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0);
   if (reserve == MAP_FAILED) {
     PLOG(ERROR) << "failed to reserve address space";
     return false;
   }
 
-  const std::string& block_dev = lines[0];
-  android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(block_dev.c_str(), O_RDONLY)));
+  android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(block_map_data.path().c_str(), O_RDONLY)));
   if (fd == -1) {
-    PLOG(ERROR) << "failed to open block device " << block_dev;
+    PLOG(ERROR) << "failed to open block device " << block_map_data.path();
     munmap(reserve, blocks * blksize);
     return false;
   }
 
   ranges_.clear();
 
-  unsigned char* next = static_cast<unsigned char*>(reserve);
+  auto next = static_cast<unsigned char*>(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 << ": " << line;
-      success = false;
-      break;
-    }
+  for (const auto& [start, end] : block_map_data.block_ranges()) {
     size_t range_size = (end - start) * blksize;
-    if (end <= start || (end - start) > SIZE_MAX / blksize || range_size > remaining_size) {
-      LOG(ERROR) << "Invalid range: " << start << " " << end;
-      success = false;
-      break;
-    }
-
     void* range_start = mmap(next, range_size, PROT_READ, MAP_PRIVATE | MAP_FIXED, fd,
                              static_cast<off_t>(start) * blksize);
     if (range_start == MAP_FAILED) {
-      PLOG(ERROR) << "failed to map range " << i << ": " << line;
-      success = false;
-      break;
+      PLOG(ERROR) << "failed to map range " << start << ": " << end;
+      munmap(reserve, blocks * blksize);
+      return false;
     }
     ranges_.emplace_back(MappedRange{ range_start, range_size });
 
     next += range_size;
     remaining_size -= range_size;
   }
-  if (success && remaining_size != 0) {
+  if (remaining_size != 0) {
     LOG(ERROR) << "Invalid ranges: remaining_size " << remaining_size;
-    success = false;
-  }
-  if (!success) {
     munmap(reserve, blocks * blksize);
     return false;
   }
 
   addr = static_cast<unsigned char*>(reserve);
-  length = size;
+  length = block_map_data.file_size();
 
-  LOG(INFO) << "mmapped " << range_count << " ranges";
+  LOG(INFO) << "mmapped " << block_map_data.block_ranges().size() << " ranges";
 
   return true;
 }
diff --git a/tests/unit/sysutil_test.cpp b/tests/unit/sysutil_test.cpp
index 77625dbe9..3466e8eec 100644
--- a/tests/unit/sysutil_test.cpp
+++ b/tests/unit/sysutil_test.cpp
@@ -17,8 +17,10 @@
 #include <string>
 
 #include <android-base/file.h>
+#include <android-base/strings.h>
 #include <gtest/gtest.h>
 
+#include "otautil/rangeset.h"
 #include "otautil/sysutil.h"
 
 TEST(SysUtilTest, InvalidArgs) {
@@ -28,6 +30,65 @@ TEST(SysUtilTest, InvalidArgs) {
   ASSERT_FALSE(mapping.MapFile(""));
 }
 
+TEST(SysUtilTest, ParseBlockMapFile_smoke) {
+  std::vector<std::string> content = {
+    "/dev/abc", "49652 4096", "3", "1000 1008", "2100 2102", "30 33",
+  };
+
+  TemporaryFile temp_file;
+  ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path));
+
+  auto block_map_data = BlockMapData::ParseBlockMapFile(temp_file.path);
+  ASSERT_EQ("/dev/abc", block_map_data.path());
+  ASSERT_EQ(49652, block_map_data.file_size());
+  ASSERT_EQ(4096, block_map_data.block_size());
+  ASSERT_EQ(RangeSet(std::vector<Range>{
+                { 1000, 1008 },
+                { 2100, 2102 },
+                { 30, 33 },
+            }),
+            block_map_data.block_ranges());
+}
+
+TEST(SysUtilTest, ParseBlockMapFile_invalid_line_count) {
+  std::vector<std::string> content = {
+    "/dev/abc", "49652 4096", "2", "1000 1008", "2100 2102", "30 33",
+  };
+
+  TemporaryFile temp_file;
+  ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path));
+
+  auto block_map_data1 = BlockMapData::ParseBlockMapFile(temp_file.path);
+  ASSERT_FALSE(block_map_data1);
+}
+
+TEST(SysUtilTest, ParseBlockMapFile_invalid_size) {
+  std::vector<std::string> content = {
+    "/dev/abc",
+    "42949672950 4294967295",
+    "1",
+    "0 9",
+  };
+
+  TemporaryFile temp_file;
+  ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path));
+
+  auto block_map_data = BlockMapData::ParseBlockMapFile(temp_file.path);
+  ASSERT_EQ("/dev/abc", block_map_data.path());
+  ASSERT_EQ(42949672950, block_map_data.file_size());
+  ASSERT_EQ(4294967295, block_map_data.block_size());
+
+  content[1] = "42949672950 4294967296";
+  ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path));
+  auto large_block_size = BlockMapData::ParseBlockMapFile(temp_file.path);
+  ASSERT_FALSE(large_block_size);
+
+  content[1] = "4294967296 1";
+  ASSERT_TRUE(android::base::WriteStringToFile(android::base::Join(content, '\n'), temp_file.path));
+  auto too_many_blocks = BlockMapData::ParseBlockMapFile(temp_file.path);
+  ASSERT_FALSE(too_many_blocks);
+}
+
 TEST(SysUtilTest, MapFileRegularFile) {
   TemporaryFile temp_file1;
   std::string content = "abc";