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