diff --git a/fs_mgr/libfstab/boot_config.cpp b/fs_mgr/libfstab/boot_config.cpp index fee40158b..53c843e93 100644 --- a/fs_mgr/libfstab/boot_config.cpp +++ b/fs_mgr/libfstab/boot_config.cpp @@ -34,7 +34,7 @@ const std::string& GetAndroidDtDir() { // Set once and saves time for subsequent calls to this function static const std::string kAndroidDtDir = [] { std::string android_dt_dir; - if ((fs_mgr_get_boot_config_from_bootconfig_source("android_dt_dir", &android_dt_dir) || + if ((GetBootconfig("androidboot.android_dt_dir", &android_dt_dir) || fs_mgr_get_boot_config_from_kernel_cmdline("android_dt_dir", &android_dt_dir)) && !android_dt_dir.empty()) { // Ensure the returned path ends with a / @@ -51,6 +51,65 @@ const std::string& GetAndroidDtDir() { return kAndroidDtDir; } +void ImportBootconfigFromString(const std::string& bootconfig, + const std::function& fn) { + for (std::string_view line : android::base::Split(bootconfig, "\n")) { + const auto equal_pos = line.find('='); + std::string key = android::base::Trim(line.substr(0, equal_pos)); + if (key.empty()) { + continue; + } + std::string value; + if (equal_pos != line.npos) { + value = android::base::Trim(line.substr(equal_pos + 1)); + // If the value is a comma-delimited list, the kernel would insert a space between the + // list elements when read from /proc/bootconfig. + // BoardConfig.mk: + // BOARD_BOOTCONFIG := key=value1,value2,value3 + // /proc/bootconfig: + // key = "value1", "value2", "value3" + if (key == "androidboot.boot_device" || key == "androidboot.boot_devices") { + // boot_device[s] is a special case where a list element can contain comma and the + // caller expects a space-delimited list, so don't remove space here. + value.erase(std::remove(value.begin(), value.end(), '"'), value.end()); + } else { + // In order to not break the expectations of existing code, we modify the value to + // keep the format consistent with the kernel cmdline by removing quote and space. + std::string_view sv(value); + android::base::ConsumePrefix(&sv, "\""); + android::base::ConsumeSuffix(&sv, "\""); + value = android::base::StringReplace(sv, R"(", ")", ",", true); + } + } + // "key" and "key =" means empty value. + fn(std::move(key), std::move(value)); + } +} + +bool GetBootconfigFromString(const std::string& bootconfig, const std::string& key, + std::string* out) { + bool found = false; + ImportBootconfigFromString(bootconfig, [&](std::string config_key, std::string value) { + if (!found && config_key == key) { + *out = std::move(value); + found = true; + } + }); + return found; +} + +void ImportBootconfig(const std::function& fn) { + std::string bootconfig; + android::base::ReadFileToString("/proc/bootconfig", &bootconfig); + ImportBootconfigFromString(bootconfig, fn); +} + +bool GetBootconfig(const std::string& key, std::string* out) { + std::string bootconfig; + android::base::ReadFileToString("/proc/bootconfig", &bootconfig); + return GetBootconfigFromString(bootconfig, key, out); +} + } // namespace fs_mgr } // namespace android @@ -88,44 +147,6 @@ std::vector> fs_mgr_parse_cmdline(const std: return result; } -std::vector> fs_mgr_parse_proc_bootconfig( - const std::string& cmdline) { - static constexpr char quote = '"'; - - std::vector> result; - for (auto& line : android::base::Split(cmdline, "\n")) { - line.erase(std::remove(line.begin(), line.end(), quote), line.end()); - auto equal_sign = line.find('='); - if (equal_sign == line.npos) { - if (!line.empty()) { - // no difference between and = - result.emplace_back(std::move(line), ""); - } - } else { - result.emplace_back(android::base::Trim(line.substr(0, equal_sign)), - android::base::Trim(line.substr(equal_sign + 1))); - } - } - - return result; -} - -bool fs_mgr_get_boot_config_from_bootconfig(const std::string& bootconfig, - const std::string& android_key, std::string* out_val) { - FSTAB_CHECK(out_val != nullptr); - - const std::string bootconfig_key("androidboot." + android_key); - for (const auto& [key, value] : fs_mgr_parse_proc_bootconfig(bootconfig)) { - if (key == bootconfig_key) { - *out_val = value; - return true; - } - } - - *out_val = ""; - return false; -} - bool fs_mgr_get_boot_config_from_kernel(const std::string& cmdline, const std::string& android_key, std::string* out_val) { FSTAB_CHECK(out_val != nullptr); @@ -142,17 +163,6 @@ bool fs_mgr_get_boot_config_from_kernel(const std::string& cmdline, const std::s return false; } -// Tries to get the given boot config value from bootconfig. -// Returns true if successfully found, false otherwise. -bool fs_mgr_get_boot_config_from_bootconfig_source(const std::string& key, std::string* out_val) { - std::string bootconfig; - if (!android::base::ReadFileToString("/proc/bootconfig", &bootconfig)) return false; - if (!bootconfig.empty() && bootconfig.back() == '\n') { - bootconfig.pop_back(); - } - return fs_mgr_get_boot_config_from_bootconfig(bootconfig, key, out_val); -} - // Tries to get the given boot config value from kernel cmdline. // Returns true if successfully found, false otherwise. bool fs_mgr_get_boot_config_from_kernel_cmdline(const std::string& key, std::string* out_val) { @@ -188,7 +198,8 @@ bool fs_mgr_get_boot_config(const std::string& key, std::string* out_val) { } // next, check if we have the property in bootconfig - if (fs_mgr_get_boot_config_from_bootconfig_source(key, out_val)) { + const std::string config_key = "androidboot." + key; + if (android::fs_mgr::GetBootconfig(config_key, out_val)) { return true; } diff --git a/fs_mgr/libfstab/fstab.cpp b/fs_mgr/libfstab/fstab.cpp index 86ba03148..3cbd8c819 100644 --- a/fs_mgr/libfstab/fstab.cpp +++ b/fs_mgr/libfstab/fstab.cpp @@ -864,21 +864,19 @@ const FstabEntry* GetEntryForMountPoint(const Fstab* fstab, const std::string& p std::set GetBootDevices() { // First check bootconfig, then kernel commandline, then the device tree - std::string dt_file_name = GetAndroidDtDir() + "boot_devices"; std::string value; - if (fs_mgr_get_boot_config_from_bootconfig_source("boot_devices", &value) || - fs_mgr_get_boot_config_from_bootconfig_source("boot_device", &value)) { + if (GetBootconfig("androidboot.boot_devices", &value) || + GetBootconfig("androidboot.boot_device", &value)) { std::set boot_devices; - // remove quotes and split by spaces - auto boot_device_strings = base::Split(base::StringReplace(value, "\"", "", true), " "); - for (std::string_view device : boot_device_strings) { - // trim the trailing comma, keep the rest. + // split by spaces and trim the trailing comma. + for (std::string_view device : android::base::Split(value, " ")) { base::ConsumeSuffix(&device, ","); boot_devices.emplace(device); } return boot_devices; } + std::string dt_file_name = GetAndroidDtDir() + "boot_devices"; if (fs_mgr_get_boot_config_from_kernel_cmdline("boot_devices", &value) || ReadDtFile(dt_file_name, &value)) { auto boot_devices = Split(value, ","); diff --git a/fs_mgr/libfstab/fstab_priv.h b/fs_mgr/libfstab/fstab_priv.h index 5d226f879..cc5682567 100644 --- a/fs_mgr/libfstab/fstab_priv.h +++ b/fs_mgr/libfstab/fstab_priv.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include #include @@ -29,11 +30,6 @@ bool fs_mgr_get_boot_config_from_kernel(const std::string& cmdline, const std::s std::string* out_val); bool fs_mgr_get_boot_config_from_kernel_cmdline(const std::string& key, std::string* out_val); bool fs_mgr_get_boot_config(const std::string& key, std::string* out_val); -std::vector> fs_mgr_parse_proc_bootconfig( - const std::string& bootconfig); -bool fs_mgr_get_boot_config_from_bootconfig(const std::string& bootconfig, const std::string& key, - std::string* out_val); -bool fs_mgr_get_boot_config_from_bootconfig_source(const std::string& key, std::string* out_val); bool fs_mgr_update_for_slotselect(android::fs_mgr::Fstab* fstab); bool is_dt_compatible(); @@ -46,5 +42,11 @@ bool ParseFstabFromString(const std::string& fstab_str, bool proc_mounts, Fstab* bool SkipMountWithConfig(const std::string& skip_config, Fstab* fstab, bool verbose); std::string GetFstabPath(); +void ImportBootconfigFromString(const std::string& bootconfig, + const std::function& fn); + +bool GetBootconfigFromString(const std::string& bootconfig, const std::string& key, + std::string* out); + } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/libfstab/include/fstab/fstab.h b/fs_mgr/libfstab/include/fstab/fstab.h index 0a45fe87a..79a07eef2 100644 --- a/fs_mgr/libfstab/include/fstab/fstab.h +++ b/fs_mgr/libfstab/include/fstab/fstab.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -128,5 +129,13 @@ std::string GetVerityDeviceName(const FstabEntry& entry); // If the platform does not configure a custom DT path, returns the standard one (based in procfs). const std::string& GetAndroidDtDir(); +// Import the kernel bootconfig by calling the callback |fn| with each key-value pair. +void ImportBootconfig(const std::function& fn); + +// Get the kernel bootconfig value for |key|. +// Returns true if |key| is found in bootconfig. +// Otherwise returns false and |*out| is not modified. +bool GetBootconfig(const std::string& key, std::string* out); + } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/tests/Android.bp b/fs_mgr/tests/Android.bp index b7f792fe6..2aeba0acd 100644 --- a/fs_mgr/tests/Android.bp +++ b/fs_mgr/tests/Android.bp @@ -38,6 +38,8 @@ cc_test { ], static_libs: [ "libfs_mgr", + "libgmock", + "libgtest", ], srcs: [ "file_wait_test.cpp", diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp index c51df2a7e..fbed37160 100644 --- a/fs_mgr/tests/fs_mgr_test.cpp +++ b/fs_mgr/tests/fs_mgr_test.cpp @@ -29,11 +29,13 @@ #include #include #include +#include #include #include "../fs_mgr_priv.h" using namespace android::fs_mgr; +using namespace testing; namespace { @@ -119,37 +121,42 @@ const std::vector> result_space = { {"terminator", "truncated"}, }; -const std::string bootconfig = - "androidboot.bootdevice = \"1d84000.ufshc\"\n" - "androidboot.boot_devices = \"dev1\", \"dev2,withcomma\", \"dev3\"\n" - "androidboot.baseband = \"sdy\"\n" - "androidboot.keymaster = \"1\"\n" - "androidboot.serialno = \"BLAHBLAHBLAH\"\n" - "androidboot.slot_suffix = \"_a\"\n" - "androidboot.hardware.platform = \"sdw813\"\n" - "androidboot.hardware = \"foo\"\n" - "androidboot.revision = \"EVT1.0\"\n" - "androidboot.bootloader = \"burp-0.1-7521\"\n" - "androidboot.hardware.sku = \"mary\"\n" - "androidboot.hardware.radio.subtype = \"0\"\n" - "androidboot.dtbo_idx = \"2\"\n" - "androidboot.mode = \"normal\"\n" - "androidboot.hardware.ddr = \"1GB,combuchi,LPDDR4X\"\n" - "androidboot.ddr_info = \"combuchiandroidboot.ddr_size=2GB\"\n" - "androidboot.hardware.ufs = \"2GB,combushi\"\n" - "androidboot.boottime = \"0BLE:58,1BLL:22,1BLE:571,2BLL:105,ODT:0,AVB:123\"\n" - "androidboot.ramdump = \"disabled\"\n" - "androidboot.vbmeta.device = \"PARTUUID=aa08f1a4-c7c9-402e-9a66-9707cafa9ceb\"\n" - "androidboot.vbmeta.avb_version = \"1.1\"\n" - "androidboot.vbmeta.device_state = \"unlocked\"\n" - "androidboot.vbmeta.hash_alg = \"sha256\"\n" - "androidboot.vbmeta.size = \"5248\"\n" - "androidboot.vbmeta.digest = \"" - "ac13147e959861c20f2a6da97d25fe79e60e902c022a371c5c039d31e7c68860\"\n" - "androidboot.vbmeta.invalidate_on_error = \"yes\"\n" - "androidboot.veritymode = \"enforcing\"\n" - "androidboot.verifiedbootstate = \"orange\"\n" - "androidboot.space = \"sha256 5248 androidboot.nospace = nope\"\n"; +const std::string bootconfig = R"( +androidboot.bootdevice = "1d84000.ufshc" +androidboot.boot_devices = "dev1", "dev2,withcomma", "dev3" +androidboot.baseband = "sdy" +androidboot.keymaster = "1" +androidboot.serialno = "BLAHBLAHBLAH" +androidboot.slot_suffix = "_a" +androidboot.hardware.platform = "sdw813" +androidboot.hardware = "foo" +androidboot.revision = "EVT1.0" +androidboot.bootloader = "burp-0.1-7521" +androidboot.hardware.sku = "mary" +androidboot.hardware.radio.subtype = "0" +androidboot.dtbo_idx = "2" +androidboot.mode = "normal" +androidboot.hardware.ddr = "1GB,combuchi,LPDDR4X" +androidboot.ddr_info = "combuchiandroidboot.ddr_size=2GB" +androidboot.hardware.ufs = "2GB,combushi" +androidboot.boottime = "0BLE:58,1BLL:22,1BLE:571,2BLL:105,ODT:0,AVB:123" +androidboot.ramdump = "disabled" +androidboot.vbmeta.device = "PARTUUID=aa08f1a4-c7c9-402e-9a66-9707cafa9ceb" +androidboot.vbmeta.avb_version = "1.1" +androidboot.vbmeta.device_state = "unlocked" +androidboot.vbmeta.hash_alg = "sha256" +androidboot.vbmeta.size = "5248" +androidboot.vbmeta.digest = "ac13147e959861c20f2a6da97d25fe79e60e902c022a371c5c039d31e7c68860" +androidboot.vbmeta.invalidate_on_error = "yes" +androidboot.veritymode = "enforcing" +androidboot.verifiedbootstate = "orange" +androidboot.space = "sha256 5248 androidboot.nospace = nope" +just.key +key.empty.value = +dessert.value = "ice, cream" +dessert.list = "ice", "cream" +ambiguous.list = ", ", ", " +)"; const std::vector> bootconfig_result_space = { {"androidboot.bootdevice", "1d84000.ufshc"}, @@ -182,6 +189,11 @@ const std::vector> bootconfig_result_space = {"androidboot.veritymode", "enforcing"}, {"androidboot.verifiedbootstate", "orange"}, {"androidboot.space", "sha256 5248 androidboot.nospace = nope"}, + {"just.key", ""}, + {"key.empty.value", ""}, + {"dessert.value", "ice, cream"}, + {"dessert.list", "ice,cream"}, + {"ambiguous.list", ", ,, "}, }; bool CompareFlags(FstabEntry::FsMgrFlags& lhs, FstabEntry::FsMgrFlags& rhs) { @@ -231,25 +243,35 @@ TEST(fs_mgr, fs_mgr_get_boot_config_from_kernel_cmdline) { EXPECT_TRUE(content.empty()) << content; } -TEST(fs_mgr, fs_mgr_parse_bootconfig) { - EXPECT_EQ(bootconfig_result_space, fs_mgr_parse_proc_bootconfig(bootconfig)); +TEST(fs_mgr, ImportBootconfig) { + std::vector> result; + ImportBootconfigFromString(bootconfig, [&](std::string key, std::string value) { + result.emplace_back(key, value); + }); + EXPECT_THAT(result, ContainerEq(bootconfig_result_space)); } -TEST(fs_mgr, fs_mgr_get_boot_config_from_bootconfig) { +TEST(fs_mgr, GetBootconfig) { std::string content; - for (const auto& entry : bootconfig_result_space) { - static constexpr char androidboot[] = "androidboot."; - if (!android::base::StartsWith(entry.first, androidboot)) continue; - auto key = entry.first.substr(strlen(androidboot)); - EXPECT_TRUE(fs_mgr_get_boot_config_from_bootconfig(bootconfig, key, &content)) - << " for " << key; - EXPECT_EQ(entry.second, content); + for (const auto& [key, value] : bootconfig_result_space) { + EXPECT_TRUE(GetBootconfigFromString(bootconfig, key, &content)) << " for " << key; + EXPECT_EQ(content, value); } - EXPECT_FALSE(fs_mgr_get_boot_config_from_bootconfig(bootconfig, "vbmeta.avb_versio", &content)); - EXPECT_TRUE(content.empty()) << content; - EXPECT_FALSE(fs_mgr_get_boot_config_from_bootconfig(bootconfig, "nospace", &content)); - EXPECT_TRUE(content.empty()) << content; + const std::string kUnmodifiedToken = ""; + content = kUnmodifiedToken; + EXPECT_FALSE(android::fs_mgr::GetBootconfigFromString(bootconfig, "", &content)); + EXPECT_EQ(content, kUnmodifiedToken) << "output parameter shouldn't be overridden"; + + content = kUnmodifiedToken; + EXPECT_FALSE(android::fs_mgr::GetBootconfigFromString( + bootconfig, "androidboot.vbmeta.avb_versio", &content)); + EXPECT_EQ(content, kUnmodifiedToken) << "output parameter shouldn't be overridden"; + + content = kUnmodifiedToken; + EXPECT_FALSE( + android::fs_mgr::GetBootconfigFromString(bootconfig, "androidboot.nospace", &content)); + EXPECT_EQ(content, kUnmodifiedToken) << "output parameter shouldn't be overridden"; } TEST(fs_mgr, fs_mgr_read_fstab_file_proc_mounts) { diff --git a/init/devices.cpp b/init/devices.cpp index 7c234924b..6b8474eaa 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -204,7 +205,7 @@ std::string DeviceHandler::GetPartitionNameForDevice(const std::string& query_de } }; ImportKernelCmdline(parser); - ImportBootconfig(parser); + android::fs_mgr::ImportBootconfig(parser); return partition_map; }(); diff --git a/init/property_service.cpp b/init/property_service.cpp index 0e82022a0..35577874f 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -1351,7 +1351,7 @@ static void ProcessKernelCmdline() { static void ProcessBootconfig() { - ImportBootconfig([&](const std::string& key, const std::string& value) { + android::fs_mgr::ImportBootconfig([&](const std::string& key, const std::string& value) { if (StartsWith(key, ANDROIDBOOT_PREFIX)) { InitPropertySet("ro.boot." + key.substr(ANDROIDBOOT_PREFIX.size()), value); } diff --git a/init/reboot_utils.cpp b/init/reboot_utils.cpp index e6b868e79..547b1869f 100644 --- a/init/reboot_utils.cpp +++ b/init/reboot_utils.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "capabilities.h" @@ -48,13 +49,9 @@ void SetFatalRebootTarget(const std::optional& reboot_target) { const std::string kInitFatalPanicParamString = "androidboot.init_fatal_panic"; if (cmdline.find(kInitFatalPanicParamString) == std::string::npos) { - init_fatal_panic = false; - ImportBootconfig( - [kInitFatalPanicParamString](const std::string& key, const std::string& value) { - if (key == kInitFatalPanicParamString && value == "true") { - init_fatal_panic = true; - } - }); + std::string value; + init_fatal_panic = (android::fs_mgr::GetBootconfig(kInitFatalPanicParamString, &value) && + value == "true"); } else { const std::string kInitFatalPanicString = kInitFatalPanicParamString + "=true"; init_fatal_panic = cmdline.find(kInitFatalPanicString) != std::string::npos; @@ -68,11 +65,7 @@ void SetFatalRebootTarget(const std::optional& reboot_target) { const std::string kRebootTargetString = "androidboot.init_fatal_reboot_target"; auto start_pos = cmdline.find(kRebootTargetString); if (start_pos == std::string::npos) { - ImportBootconfig([kRebootTargetString](const std::string& key, const std::string& value) { - if (key == kRebootTargetString) { - init_fatal_reboot_target = value; - } - }); + android::fs_mgr::GetBootconfig(kRebootTargetString, &init_fatal_reboot_target); // We already default to bootloader if no setting is provided. } else { const std::string kRebootTargetStringPattern = kRebootTargetString + "="; diff --git a/init/selinux.cpp b/init/selinux.cpp index 51093d898..6fb028b16 100644 --- a/init/selinux.cpp +++ b/init/selinux.cpp @@ -110,11 +110,11 @@ EnforcingStatus StatusFromProperty() { }); if (status == SELINUX_ENFORCING) { - ImportBootconfig([&](const std::string& key, const std::string& value) { - if (key == "androidboot.selinux" && value == "permissive") { - status = SELINUX_PERMISSIVE; - } - }); + std::string value; + if (android::fs_mgr::GetBootconfig("androidboot.selinux", &value) && + value == "permissive") { + status = SELINUX_PERMISSIVE; + } } return status; diff --git a/init/util.cpp b/init/util.cpp index 61d59a480..b34d45d70 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -254,21 +254,6 @@ void ImportKernelCmdline(const std::function& fn) { - std::string bootconfig; - android::base::ReadFileToString("/proc/bootconfig", &bootconfig); - - for (const auto& entry : android::base::Split(bootconfig, "\n")) { - std::vector pieces = android::base::Split(entry, "="); - if (pieces.size() == 2) { - // get rid of the extra space between a list of values and remove the quotes. - std::string value = android::base::StringReplace(pieces[1], "\", \"", ",", true); - value.erase(std::remove(value.begin(), value.end(), '"'), value.end()); - fn(android::base::Trim(pieces[0]), android::base::Trim(value)); - } - } -} - bool make_dir(const std::string& path, mode_t mode) { std::string secontext; if (SelabelLookupFileContext(path, mode, &secontext) && !secontext.empty()) { diff --git a/init/util.h b/init/util.h index 1c00a3efc..0b0ef632b 100644 --- a/init/util.h +++ b/init/util.h @@ -55,7 +55,6 @@ Result DecodeUid(const std::string& name); bool mkdir_recursive(const std::string& pathname, mode_t mode); int wait_for_file(const char *filename, std::chrono::nanoseconds timeout); void ImportKernelCmdline(const std::function&); -void ImportBootconfig(const std::function&); bool make_dir(const std::string& path, mode_t mode); bool is_dir(const char* pathname); Result ExpandProps(const std::string& src);