From 7aa37f1f21c83479b047706a8867d8ba225b1691 Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Mon, 6 Apr 2020 23:28:40 +0100 Subject: [PATCH] get_mounted_entry_for_userdata: Realpath block devices from fstabs Use realpath as a canonical representation of block devices. This makes it easier to reason about block devices. This also fixes a bug, in which fs_mgr_get_mounted_entry_for_userdata didn't properly work on devices that don't support metadata encryption. Test: atest CtsFsMgrTestCases Test: atest CtsUserspaceRebootHostSideTestCases Bug: 153363818 Change-Id: I139c2be46336a632bbaee86667019c075d7de814 --- fs_mgr/fs_mgr.cpp | 42 ++++++++++++++++++++---------------- fs_mgr/include/fs_mgr.h | 5 ++--- fs_mgr/tests/fs_mgr_test.cpp | 4 +++- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 5475caebf..1486e87e7 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -97,7 +97,6 @@ using android::base::Basename; using android::base::GetBoolProperty; using android::base::GetUintProperty; -using android::base::Readlink; using android::base::Realpath; using android::base::SetProperty; using android::base::StartsWith; @@ -1552,8 +1551,8 @@ static std::chrono::milliseconds GetMillisProperty(const std::string& name, return std::chrono::milliseconds(std::move(value)); } -static bool fs_mgr_unmount_all_data_mounts(const std::string& block_device) { - LINFO << __FUNCTION__ << "(): about to umount everything on top of " << block_device; +static bool fs_mgr_unmount_all_data_mounts(const std::string& data_block_device) { + LINFO << __FUNCTION__ << "(): about to umount everything on top of " << data_block_device; Timer t; auto timeout = GetMillisProperty("init.userspace_reboot.userdata_remount.timeoutmillis", 5s); while (true) { @@ -1565,7 +1564,13 @@ static bool fs_mgr_unmount_all_data_mounts(const std::string& block_device) { } // Now proceed with other bind mounts on top of /data. for (const auto& entry : proc_mounts) { - if (entry.blk_device == block_device) { + std::string block_device; + if (StartsWith(entry.blk_device, "/dev/block") && + !Realpath(entry.blk_device, &block_device)) { + PWARNING << __FUNCTION__ << "(): failed to realpath " << entry.blk_device; + block_device = entry.blk_device; + } + if (data_block_device == block_device) { if (umount2(entry.mount_point.c_str(), 0) != 0) { PERROR << __FUNCTION__ << "(): Failed to umount " << entry.mount_point; umount_done = false; @@ -1577,7 +1582,8 @@ static bool fs_mgr_unmount_all_data_mounts(const std::string& block_device) { return true; } if (t.duration() > timeout) { - LERROR << __FUNCTION__ << "(): Timed out unmounting all mounts on " << block_device; + LERROR << __FUNCTION__ << "(): Timed out unmounting all mounts on " + << data_block_device; Fstab remaining_mounts; if (!ReadFstabFromFile("/proc/mounts", &remaining_mounts)) { LERROR << __FUNCTION__ << "(): Can't read /proc/mounts"; @@ -1616,14 +1622,11 @@ static bool UnwindDmDeviceStack(const std::string& block_device, return true; } -FstabEntry* fs_mgr_get_mounted_entry_for_userdata(Fstab* fstab, const FstabEntry& mounted_entry) { - if (mounted_entry.mount_point != "/data") { - LERROR << mounted_entry.mount_point << " is not /data"; - return nullptr; - } +FstabEntry* fs_mgr_get_mounted_entry_for_userdata(Fstab* fstab, + const std::string& data_block_device) { std::vector dm_stack; - if (!UnwindDmDeviceStack(mounted_entry.blk_device, &dm_stack)) { - LERROR << "Failed to unwind dm-device stack for " << mounted_entry.blk_device; + if (!UnwindDmDeviceStack(data_block_device, &dm_stack)) { + LERROR << "Failed to unwind dm-device stack for " << data_block_device; return nullptr; } for (auto& entry : *fstab) { @@ -1637,15 +1640,15 @@ FstabEntry* fs_mgr_get_mounted_entry_for_userdata(Fstab* fstab, const FstabEntry continue; } block_device = entry.blk_device; - } else if (!Readlink(entry.blk_device, &block_device)) { - PWARNING << "Failed to read link " << entry.blk_device; + } else if (!Realpath(entry.blk_device, &block_device)) { + PWARNING << "Failed to realpath " << entry.blk_device; block_device = entry.blk_device; } if (std::find(dm_stack.begin(), dm_stack.end(), block_device) != dm_stack.end()) { return &entry; } } - LERROR << "Didn't find entry that was used to mount /data onto " << mounted_entry.blk_device; + LERROR << "Didn't find entry that was used to mount /data onto " << data_block_device; return nullptr; } @@ -1656,14 +1659,17 @@ int fs_mgr_remount_userdata_into_checkpointing(Fstab* fstab) { LERROR << "Can't read /proc/mounts"; return -1; } - std::string block_device; auto mounted_entry = GetEntryForMountPoint(&proc_mounts, "/data"); if (mounted_entry == nullptr) { LERROR << "/data is not mounted"; return -1; } - block_device = mounted_entry->blk_device; - auto fstab_entry = fs_mgr_get_mounted_entry_for_userdata(fstab, *mounted_entry); + std::string block_device; + if (!Realpath(mounted_entry->blk_device, &block_device)) { + PERROR << "Failed to realpath " << mounted_entry->blk_device; + return -1; + } + auto fstab_entry = fs_mgr_get_mounted_entry_for_userdata(fstab, block_device); if (fstab_entry == nullptr) { LERROR << "Can't find /data in fstab"; return -1; diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 3d556c9e5..86090c19e 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -107,10 +107,9 @@ enum FsMgrUmountStatus : int { // it destroys verity devices from device mapper after the device is unmounted. int fs_mgr_umount_all(android::fs_mgr::Fstab* fstab); -// Finds a entry in |fstab| that was used to mount a /data |mounted_entry| from -// /proc/mounts. +// Finds a entry in |fstab| that was used to mount a /data on |data_block_device|. android::fs_mgr::FstabEntry* fs_mgr_get_mounted_entry_for_userdata( - android::fs_mgr::Fstab* fstab, const android::fs_mgr::FstabEntry& mounted_entry); + android::fs_mgr::Fstab* fstab, const std::string& data_block_device); int fs_mgr_remount_userdata_into_checkpointing(android::fs_mgr::Fstab* fstab); // Finds the dm_bow device on which this block device is stacked, or returns diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp index 3fec60814..27c8aae5b 100644 --- a/fs_mgr/tests/fs_mgr_test.cpp +++ b/fs_mgr/tests/fs_mgr_test.cpp @@ -1020,6 +1020,8 @@ TEST(fs_mgr, UserdataMountedFromDefaultFstab) { ASSERT_TRUE(ReadFstabFromFile("/proc/mounts", &proc_mounts)) << "Failed to read /proc/mounts"; auto mounted_entry = GetEntryForMountPoint(&proc_mounts, "/data"); ASSERT_NE(mounted_entry, nullptr) << "/data is not mounted"; - ASSERT_NE(nullptr, fs_mgr_get_mounted_entry_for_userdata(&fstab, *mounted_entry)) + std::string block_device; + ASSERT_TRUE(android::base::Realpath(mounted_entry->blk_device, &block_device)); + ASSERT_NE(nullptr, fs_mgr_get_mounted_entry_for_userdata(&fstab, block_device)) << "/data wasn't mounted from default fstab"; }