From ea4369d141ce0799a0f0df16e0245924d03bb817 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Mon, 22 Mar 2021 13:45:09 +0800 Subject: [PATCH 1/2] fs_mgr: Strengthen ReadFstabFromFile() around gsi_public_metadata_file ReadFstabFromFile() calls access() to check the existence of DSU metadata files to determine if device is in DSU running state. This is error prone because a failed access() can mean non-exsitent file as well as the caller lacking the permission to path resolute the pathname. Strengthen ReadFstabFromFile() to check the errno after a failed access() or open(), if the errno is not ENOENT, then return with error, as this may be indicating the caller doesn't have sufficient access rights to call ReadFstabFromFile(). After this change, processes would need these policies to call ReadFstabFromFile(): allow scontext { metadata_file gsi_metadata_file_type }:dir search; And these policies to call ReadFstabFromFile() within a DSU system: allow scontext gsi_public_metadata_file:file r_file_perms; Bug: 181110285 Test: Presubmit Change-Id: I1a6a796cb9b7b49af3aa5e7a5e8d99cde25e5857 --- fs_mgr/fs_mgr_fstab.cpp | 50 ++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 796a39abf..7a456bb43 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -693,22 +693,32 @@ bool ReadFstabFromFile(const std::string& path, Fstab* fstab) { LERROR << __FUNCTION__ << "(): failed to load fstab from : '" << path << "'"; return false; } - if (!is_proc_mounts && !access(android::gsi::kGsiBootedIndicatorFile, F_OK)) { - // This is expected to fail if host is android Q, since Q doesn't - // support DSU slotting. The DSU "active" indicator file would be - // non-existent or empty if DSU is enabled within the guest system. - // In that case, just use the default slot name "dsu". - std::string dsu_slot; - if (!android::gsi::GetActiveDsu(&dsu_slot)) { - PWARNING << __FUNCTION__ << "(): failed to get active dsu slot"; + if (!is_proc_mounts) { + if (!access(android::gsi::kGsiBootedIndicatorFile, F_OK)) { + // This is expected to fail if host is android Q, since Q doesn't + // support DSU slotting. The DSU "active" indicator file would be + // non-existent or empty if DSU is enabled within the guest system. + // In that case, just use the default slot name "dsu". + std::string dsu_slot; + if (!android::gsi::GetActiveDsu(&dsu_slot) && errno != ENOENT) { + PERROR << __FUNCTION__ << "(): failed to get active DSU slot"; + return false; + } + if (dsu_slot.empty()) { + dsu_slot = "dsu"; + LWARNING << __FUNCTION__ << "(): assuming default DSU slot: " << dsu_slot; + } + // This file is non-existent on Q vendor. + std::string lp_names; + if (!ReadFileToString(gsi::kGsiLpNamesFile, &lp_names) && errno != ENOENT) { + PERROR << __FUNCTION__ << "(): failed to read DSU LP names"; + return false; + } + TransformFstabForDsu(fstab, dsu_slot, Split(lp_names, ",")); + } else if (errno != ENOENT) { + PERROR << __FUNCTION__ << "(): failed to access() DSU booted indicator"; + return false; } - if (dsu_slot.empty()) { - dsu_slot = "dsu"; - } - - std::string lp_names; - ReadFileToString(gsi::kGsiLpNamesFile, &lp_names); - TransformFstabForDsu(fstab, dsu_slot, Split(lp_names, ",")); } SkipMountingPartitions(fstab, false /* verbose */); @@ -802,16 +812,14 @@ bool ReadDefaultFstab(Fstab* fstab) { } Fstab default_fstab; - if (!default_fstab_path.empty()) { - ReadFstabFromFile(default_fstab_path, &default_fstab); + if (!default_fstab_path.empty() && ReadFstabFromFile(default_fstab_path, &default_fstab)) { + for (auto&& entry : default_fstab) { + fstab->emplace_back(std::move(entry)); + } } else { LINFO << __FUNCTION__ << "(): failed to find device default fstab"; } - for (auto&& entry : default_fstab) { - fstab->emplace_back(std::move(entry)); - } - return !fstab->empty(); } From 1a3c050a35a0130808794305375cf8850abbc860 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Tue, 30 Mar 2021 20:28:21 +0800 Subject: [PATCH 2/2] fs_mgr: Refactor ReadDefaultFstab() and ReadFstabFromFile() * Eliminate redundant std::move() by transforming ReadFstabFromDt(&dt_fstab, false); *fstab = std::move(dt_fstab); to fstab->clear(); ReadFstabFromDt(fstab, false); * Don't modify output parameter if ReadFstabFromFile() failed. Bug: 181110285 Test: Presubmit Change-Id: I4e4d9852cc618a66d79e423780bf97773dca2a58 --- fs_mgr/fs_mgr_fstab.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 7a456bb43..950fc9676 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -680,7 +680,7 @@ void EnableMandatoryFlags(Fstab* fstab) { } } -bool ReadFstabFromFile(const std::string& path, Fstab* fstab) { +bool ReadFstabFromFile(const std::string& path, Fstab* fstab_out) { auto fstab_file = std::unique_ptr{fopen(path.c_str(), "re"), fclose}; if (!fstab_file) { PERROR << __FUNCTION__ << "(): cannot open file: '" << path << "'"; @@ -689,7 +689,8 @@ bool ReadFstabFromFile(const std::string& path, Fstab* fstab) { bool is_proc_mounts = path == "/proc/mounts"; - if (!ReadFstabFile(fstab_file.get(), is_proc_mounts, fstab)) { + Fstab fstab; + if (!ReadFstabFile(fstab_file.get(), is_proc_mounts, &fstab)) { LERROR << __FUNCTION__ << "(): failed to load fstab from : '" << path << "'"; return false; } @@ -714,16 +715,17 @@ bool ReadFstabFromFile(const std::string& path, Fstab* fstab) { PERROR << __FUNCTION__ << "(): failed to read DSU LP names"; return false; } - TransformFstabForDsu(fstab, dsu_slot, Split(lp_names, ",")); + TransformFstabForDsu(&fstab, dsu_slot, Split(lp_names, ",")); } else if (errno != ENOENT) { PERROR << __FUNCTION__ << "(): failed to access() DSU booted indicator"; return false; } } - SkipMountingPartitions(fstab, false /* verbose */); - EnableMandatoryFlags(fstab); + SkipMountingPartitions(&fstab, false /* verbose */); + EnableMandatoryFlags(&fstab); + *fstab_out = std::move(fstab); return true; } @@ -798,10 +800,8 @@ bool SkipMountingPartitions(Fstab* fstab, bool verbose) { // Loads the fstab file and combines with fstab entries passed in from device tree. bool ReadDefaultFstab(Fstab* fstab) { - Fstab dt_fstab; - ReadFstabFromDt(&dt_fstab, false /* verbose */); - - *fstab = std::move(dt_fstab); + fstab->clear(); + ReadFstabFromDt(fstab, false /* verbose */); std::string default_fstab_path; // Use different fstab paths for normal boot and recovery boot, respectively