From c7230a1eb1d1c2786e351d7e580d36eb399dfc66 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 20 Oct 2023 20:42:56 +0900 Subject: [PATCH] Don't overwrite persistent property files This fixes a bug introduced by I81b6bd984aad8f7ddec93ce74f4543e4f71be508 In the original CL, setting a sysprop `next_boot.` and then rebooting the device could remove all the persistent properties stored in /data/property/persistent_properties. It happened because the function `WritePersistentProperty` is called with a properties set which is initialized as an empty set and then added with the properties which had the next_boot prefix. As a result... Before the boot: * persist.a = 1 * next_boot.b = 2 * next_boot.persist.c = 3 After the reboot: * b = 2 * persist.c = 3 persist.a gets lost. This change fixes the issue by populating properties set from the memory and then save it to the file. Bug: 306062513 Bug: 300111812 Test: do the following. $ adb root $ adb shell setprop persist.a 1 $ adb shell setprop next_boot.b 1 $ adb shell setprop next_boot.persist.c 1 $ adb reboot $ adb shell getprop persist.a 1 // was (none) before this change $ adb shell getprop b 1 $ adb shell getprop persist.c 1 $ adb reboot $ adb shell getprop persist.a 1 // was (none) before this change $ adb shell getprop b // (none) because b isn't persisted. WAI. $ adb shell getprop persist.c 1 Change-Id: I85d3777f9b32523b010e49b8ca53f4319dd2ce05 --- init/persistent_properties.cpp | 36 +++++++++++++++++----------------- init/persistent_properties.h | 1 + init/property_service.cpp | 15 +++----------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/init/persistent_properties.cpp b/init/persistent_properties.cpp index 8efb72c7e..e89244f13 100644 --- a/init/persistent_properties.cpp +++ b/init/persistent_properties.cpp @@ -115,24 +115,6 @@ void RemoveLegacyPersistentPropertyFiles() { } } -PersistentProperties LoadPersistentPropertiesFromMemory() { - PersistentProperties persistent_properties; - __system_property_foreach( - [](const prop_info* pi, void* cookie) { - __system_property_read_callback( - pi, - [](void* cookie, const char* name, const char* value, unsigned serial) { - if (StartsWith(name, "persist.")) { - auto properties = reinterpret_cast(cookie); - AddPersistentProperty(name, value, properties); - } - }, - cookie); - }, - &persistent_properties); - return persistent_properties; -} - Result ReadPersistentPropertyFile() { const std::string temp_filename = persistent_property_filename + ".tmp"; if (access(temp_filename.c_str(), F_OK) == 0) { @@ -221,6 +203,24 @@ Result WritePersistentPropertyFile(const PersistentProperties& persistent_ return {}; } +PersistentProperties LoadPersistentPropertiesFromMemory() { + PersistentProperties persistent_properties; + __system_property_foreach( + [](const prop_info* pi, void* cookie) { + __system_property_read_callback( + pi, + [](void* cookie, const char* name, const char* value, unsigned serial) { + if (StartsWith(name, "persist.")) { + auto properties = reinterpret_cast(cookie); + AddPersistentProperty(name, value, properties); + } + }, + cookie); + }, + &persistent_properties); + return persistent_properties; +} + // Persistent properties are not written often, so we rather not keep any data in memory and read // then rewrite the persistent property file for each update. void WritePersistentProperty(const std::string& name, const std::string& value) { diff --git a/init/persistent_properties.h b/init/persistent_properties.h index a6f80e64f..7e9d4387a 100644 --- a/init/persistent_properties.h +++ b/init/persistent_properties.h @@ -29,6 +29,7 @@ void AddPersistentProperty(const std::string& name, const std::string& value, PersistentProperties* persistent_properties); PersistentProperties LoadPersistentProperties(); void WritePersistentProperty(const std::string& name, const std::string& value); +PersistentProperties LoadPersistentPropertiesFromMemory(); // Exposed only for testing Result LoadPersistentPropertyFile(); diff --git a/init/property_service.cpp b/init/property_service.cpp index 38cbea378..b08a904a7 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -1400,8 +1400,6 @@ static void HandleInitSocket() { // Apply staged and persistent properties bool has_staged_prop = false; auto const staged_prefix = std::string_view("next_boot."); - auto const staged_persist_prefix = std::string_view("next_boot.persist."); - auto persist_props_map = std::unordered_map(); auto persistent_properties = LoadPersistentProperties(); for (const auto& property_record : persistent_properties.properties()) { @@ -1412,23 +1410,16 @@ static void HandleInitSocket() { has_staged_prop = true; auto actual_prop_name = prop_name.substr(staged_prefix.size()); InitPropertySet(actual_prop_name, prop_value); - if (StartsWith(prop_name, staged_persist_prefix)) { - persist_props_map[actual_prop_name] = prop_value; - } - } else if (!persist_props_map.count(prop_name)) { + } else { InitPropertySet(prop_name, prop_value); } } // Update persist prop file if there are staged props if (has_staged_prop) { - PersistentProperties updated_persist_props; - for (auto const& [prop_name, prop_value] : persist_props_map) { - AddPersistentProperty(prop_name, prop_value, &updated_persist_props); - } - + PersistentProperties props = LoadPersistentPropertiesFromMemory(); // write current updated persist prop file - auto result = WritePersistentPropertyFile(updated_persist_props); + auto result = WritePersistentPropertyFile(props); if (!result.ok()) { LOG(ERROR) << "Could not store persistent property: " << result.error(); }