Merge "libfiemap_writer: Calculate FIBMAP blocks correctly."
diff --git a/fs_mgr/libfiemap_writer/fiemap_writer.cpp b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
index 45b997f..85589cc 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer.cpp
@@ -507,7 +507,17 @@
         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 @@
         }
 
         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 4a1a5ab..ca51689 100644
--- a/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
+++ b/fs_mgr/libfiemap_writer/fiemap_writer_test.cpp
@@ -196,6 +196,61 @@
     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 @@
     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);