From 483ce1d4961e500f45b9f9831f6a88c5959071ea Mon Sep 17 00:00:00 2001
From: David Anderson <dvander@google.com>
Date: Wed, 13 Mar 2019 17:55:41 -0700
Subject: [PATCH] libfiemap_writer: Calculate FIBMAP blocks correctly.

FIBMAP blocks are returned in FIGETBSZ units, which means the number of
blocks also needs to be determined by FIGETBSZ. Using the stat blocksize
is incorrect. On VFAT, FIGETBSZ returns 512 whereas st_blksize is 32768.

Bug: 126230649
Test: fiemap_writer_test gtest
Change-Id: Id0a667936ff9c0b60b1e8f72920cf62ceece1657
---
 fs_mgr/libfiemap_writer/fiemap_writer.cpp     | 23 +++++---
 .../libfiemap_writer/fiemap_writer_test.cpp   | 57 ++++++++++++++++++-
 2 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
index 45b997ffe..85589cce6 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
@@ -507,7 +507,17 @@ static bool ReadFibmap(int file_fd, const std::string& file_path,
         return false;
     }
 
-    uint64_t num_blocks = (s.st_size + s.st_blksize - 1) / s.st_blksize;
+    unsigned int blksize;
+    if (ioctl(file_fd, FIGETBSZ, &blksize) < 0) {
+        PLOG(ERROR) << "Failed to get FIGETBSZ for " << file_path;
+        return false;
+    }
+    if (!blksize) {
+        LOG(ERROR) << "Invalid filesystem block size: " << blksize;
+        return false;
+    }
+
+    uint64_t num_blocks = (s.st_size + blksize - 1) / blksize;
     if (num_blocks > std::numeric_limits<uint32_t>::max()) {
         LOG(ERROR) << "Too many blocks for FIBMAP (" << num_blocks << ")";
         return false;
@@ -525,13 +535,12 @@ static bool ReadFibmap(int file_fd, const std::string& file_path,
         }
 
         if (!extents->empty() && block == last_block + 1) {
-            extents->back().fe_length += s.st_blksize;
+            extents->back().fe_length += blksize;
         } else {
-            extents->push_back(
-                    fiemap_extent{.fe_logical = block_number,
-                                  .fe_physical = static_cast<uint64_t>(block) * s.st_blksize,
-                                  .fe_length = static_cast<uint64_t>(s.st_blksize),
-                                  .fe_flags = 0});
+            extents->push_back(fiemap_extent{.fe_logical = block_number,
+                                             .fe_physical = static_cast<uint64_t>(block) * blksize,
+                                             .fe_length = static_cast<uint64_t>(blksize),
+                                             .fe_flags = 0});
         }
         last_block = block;
     }
diff --git a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
index 4a1a5ab6c..ca5168968 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
@@ -196,6 +196,61 @@ TEST_F(FiemapWriterTest, MaxBlockSize) {
     ASSERT_GT(DetermineMaximumFileSize(testfile), 0);
 }
 
+TEST_F(FiemapWriterTest, FibmapBlockAddressing) {
+    FiemapUniquePtr fptr = FiemapWriter::Open(testfile, gBlockSize);
+    ASSERT_NE(fptr, nullptr);
+
+    switch (fptr->fs_type()) {
+        case F2FS_SUPER_MAGIC:
+        case EXT4_SUPER_MAGIC:
+            // Skip the test for FIEMAP supported filesystems. This is really
+            // because f2fs/ext4 have caches that seem to defeat reading back
+            // directly from the block device, and writing directly is too
+            // dangerous.
+            std::cout << "Skipping test, filesystem does not use FIBMAP\n";
+            return;
+    }
+
+    bool uses_dm;
+    std::string bdev_path;
+    ASSERT_TRUE(FiemapWriter::GetBlockDeviceForFile(testfile, &bdev_path, &uses_dm));
+
+    if (uses_dm) {
+        // We could use a device-mapper wrapper here to bypass encryption, but
+        // really this test is for FIBMAP correctness on VFAT (where encryption
+        // is never used), so we don't bother.
+        std::cout << "Skipping test, block device is metadata encrypted\n";
+        return;
+    }
+
+    std::string data(fptr->size(), '\0');
+    for (size_t i = 0; i < data.size(); i++) {
+        data[i] = 'A' + static_cast<char>(data.size() % 26);
+    }
+
+    {
+        unique_fd fd(open(testfile.c_str(), O_WRONLY | O_CLOEXEC));
+        ASSERT_GE(fd, 0);
+        ASSERT_TRUE(android::base::WriteFully(fd, data.data(), data.size()));
+        ASSERT_EQ(fsync(fd), 0);
+    }
+
+    ASSERT_FALSE(fptr->extents().empty());
+    const auto& first_extent = fptr->extents()[0];
+
+    unique_fd bdev(open(fptr->bdev_path().c_str(), O_RDONLY | O_CLOEXEC));
+    ASSERT_GE(bdev, 0);
+
+    off_t where = first_extent.fe_physical;
+    ASSERT_EQ(lseek(bdev, where, SEEK_SET), where);
+
+    // Note: this will fail on encrypted folders.
+    std::string actual(data.size(), '\0');
+    ASSERT_GE(first_extent.fe_length, data.size());
+    ASSERT_TRUE(android::base::ReadFully(bdev, actual.data(), actual.size()));
+    EXPECT_EQ(memcmp(actual.data(), data.data(), data.size()), 0);
+}
+
 TEST_F(SplitFiemapTest, Create) {
     auto ptr = SplitFiemap::Create(testfile, 1024 * 768, 1024 * 32);
     ASSERT_NE(ptr, nullptr);
@@ -446,7 +501,7 @@ int main(int argc, char** argv) {
     if (argc <= 1) {
         cerr << "Usage: <test_dir> [file_size]\n";
         cerr << "\n";
-        cerr << "Note: test_dir must be a writable directory.\n";
+        cerr << "Note: test_dir must be a writable, unencrypted directory.\n";
         exit(EXIT_FAILURE);
     }
     ::android::base::InitLogging(argv, ::android::base::StderrLogger);