diff --git a/init/Android.bp b/init/Android.bp index 672942e62..0e580fc43 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -73,6 +73,7 @@ cc_library_static { "log.cpp", "parser.cpp", "persistent_properties.cpp", + "persistent_properties.proto", "property_service.cpp", "security.cpp", "selinux.cpp", @@ -90,11 +91,15 @@ cc_library_static { "liblog", "libprocessgroup", "libfs_mgr", + "libprotobuf-cpp-lite", ], include_dirs: [ "system/core/mkbootimg", ], - + proto: { + type: "lite", + export_proto_headers: true, + }, } /* @@ -179,6 +184,7 @@ cc_test { "libinit", "libselinux", "libcrypto", + "libprotobuf-cpp-lite", ], } diff --git a/init/Android.mk b/init/Android.mk index 6c28517e0..dd0f1bfd7 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -82,6 +82,7 @@ LOCAL_STATIC_LIBRARIES := \ libprocessgroup \ libavb \ libkeyutils \ + libprotobuf-cpp-lite \ LOCAL_REQUIRED_MODULES := \ e2fsdroid \ diff --git a/init/persistent_properties.cpp b/init/persistent_properties.cpp index 66fa011b0..71f235532 100644 --- a/init/persistent_properties.cpp +++ b/init/persistent_properties.cpp @@ -46,14 +46,21 @@ namespace { constexpr const uint32_t kMagic = 0x8495E0B4; constexpr const char kLegacyPersistentPropertyDir[] = "/data/property"; -Result>> LoadLegacyPersistentProperties() { +void AddPersistentProperty(const std::string& name, const std::string& value, + PersistentProperties* persistent_properties) { + auto persistent_property_record = persistent_properties->add_properties(); + persistent_property_record->set_name(name); + persistent_property_record->set_value(value); +} + +Result LoadLegacyPersistentProperties() { std::unique_ptr dir(opendir(kLegacyPersistentPropertyDir), closedir); if (!dir) { return ErrnoError() << "Unable to open persistent property directory \"" << kLegacyPersistentPropertyDir << "\""; } - std::vector> persistent_properties; + PersistentProperties persistent_properties; dirent* entry; while ((entry = readdir(dir.get())) != nullptr) { if (!StartsWith(entry->d_name, "persist.")) { @@ -87,7 +94,7 @@ Result>> LoadLegacyPersistentPro std::string value; if (ReadFdToString(fd, &value)) { - persistent_properties.emplace_back(entry->d_name, value); + AddPersistentProperty(entry->d_name, value, &persistent_properties); } else { PLOG(ERROR) << "Unable to read persistent property file " << entry->d_name; } @@ -115,30 +122,28 @@ void RemoveLegacyPersistentPropertyFiles() { } } -std::vector> LoadPersistentPropertiesFromMemory() { - std::vector> properties; +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); - properties->emplace_back(name, value); + auto properties = reinterpret_cast(cookie); + AddPersistentProperty(name, value, properties); } }, cookie); }, - &properties); - return properties; + &persistent_properties); + return persistent_properties; } class PersistentPropertyFileParser { public: PersistentPropertyFileParser(const std::string& contents) : contents_(contents), position_(0) {} - Result>> Parse(); + Result Parse(); private: Result ReadString(); @@ -148,9 +153,7 @@ class PersistentPropertyFileParser { size_t position_; }; -Result>> PersistentPropertyFileParser::Parse() { - std::vector> result; - +Result PersistentPropertyFileParser::Parse() { if (auto magic = ReadUint32(); magic) { if (*magic != kMagic) { return Error() << "Magic value '0x" << std::hex << *magic @@ -174,24 +177,20 @@ Result>> PersistentPropertyFileP return Error() << "Could not read num_properties: " << num_properties.error(); } + PersistentProperties result; while (position_ < contents_.size()) { - auto key = ReadString(); - if (!key) { - return Error() << "Could not read key: " << key.error(); + auto name = ReadString(); + if (!name) { + return Error() << "Could not read name: " << name.error(); } - if (!StartsWith(*key, "persist.")) { - return Error() << "Property '" << *key << "' does not starts with 'persist.'"; + if (!StartsWith(*name, "persist.")) { + return Error() << "Property '" << *name << "' does not starts with 'persist.'"; } auto value = ReadString(); if (!value) { return Error() << "Could not read value: " << value.error(); } - result.emplace_back(*key, *value); - } - - if (result.size() != *num_properties) { - return Error() << "Mismatch of number of persistent properties read, " << result.size() - << " and number of persistent properties expected, " << *num_properties; + AddPersistentProperty(*name, *value, &result); } return result; @@ -220,9 +219,7 @@ Result PersistentPropertyFileParser::ReadUint32() { return result; } -} // namespace - -Result>> LoadPersistentPropertyFile() { +Result ReadPersistentPropertyFile() { const std::string temp_filename = persistent_property_filename + ".tmp"; if (access(temp_filename.c_str(), F_OK) == 0) { LOG(INFO) @@ -234,51 +231,47 @@ Result>> LoadPersistentPropertyF if (!file_contents) { return Error() << "Unable to read persistent property file: " << file_contents.error(); } + return *file_contents; +} + +} // namespace + +Result LoadPersistentPropertyFile() { + auto file_contents = ReadPersistentPropertyFile(); + if (!file_contents) return file_contents.error(); + + // Check the intermediate "I should have used protobufs from the start" format. + // TODO: Remove this. auto parsed_contents = PersistentPropertyFileParser(*file_contents).Parse(); - if (!parsed_contents) { - // If the file cannot be parsed, then we don't have any recovery mechanisms, so we delete - // it to allow for future writes to take place successfully. - unlink(persistent_property_filename.c_str()); - return Error() << "Unable to parse persistent property file: " << parsed_contents.error(); + if (parsed_contents) { + LOG(INFO) << "Intermediate format persistent property file found, converting to protobuf"; + + // Update to the protobuf format + WritePersistentPropertyFile(*parsed_contents); + return parsed_contents; } - return parsed_contents; + + PersistentProperties persistent_properties; + if (persistent_properties.ParseFromString(*file_contents)) return persistent_properties; + + // If the file cannot be parsed in either format, then we don't have any recovery + // mechanisms, so we delete it to allow for future writes to take place successfully. + unlink(persistent_property_filename.c_str()); + return Error() << "Unable to parse persistent property file: " << parsed_contents.error(); } -std::string GenerateFileContents( - const std::vector>& persistent_properties) { - std::string result; - - uint32_t magic = kMagic; - result.append(reinterpret_cast(&magic), sizeof(uint32_t)); - - uint32_t version = 1; - result.append(reinterpret_cast(&version), sizeof(uint32_t)); - - uint32_t num_properties = persistent_properties.size(); - result.append(reinterpret_cast(&num_properties), sizeof(uint32_t)); - - for (const auto& [key, value] : persistent_properties) { - uint32_t key_length = key.length(); - result.append(reinterpret_cast(&key_length), sizeof(uint32_t)); - result.append(key); - uint32_t value_length = value.length(); - result.append(reinterpret_cast(&value_length), sizeof(uint32_t)); - result.append(value); - } - return result; -} - -Result WritePersistentPropertyFile( - const std::vector>& persistent_properties) { - auto file_contents = GenerateFileContents(persistent_properties); - +Result WritePersistentPropertyFile(const PersistentProperties& persistent_properties) { const std::string temp_filename = persistent_property_filename + ".tmp"; unique_fd fd(TEMP_FAILURE_RETRY( open(temp_filename.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC | O_CLOEXEC, 0600))); if (fd == -1) { return ErrnoError() << "Could not open temporary properties file"; } - if (!WriteStringToFd(file_contents, fd)) { + std::string serialized_string; + if (!persistent_properties.SerializeToString(&serialized_string)) { + return Error() << "Unable to serialize properties"; + } + if (!WriteStringToFd(serialized_string, fd)) { return ErrnoError() << "Unable to write file contents"; } fsync(fd); @@ -295,26 +288,30 @@ Result WritePersistentPropertyFile( // 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) { - auto persistent_properties = LoadPersistentPropertyFile(); - if (!persistent_properties) { + auto file_contents = ReadPersistentPropertyFile(); + PersistentProperties persistent_properties; + + if (!file_contents || !persistent_properties.ParseFromString(*file_contents)) { LOG(ERROR) << "Recovering persistent properties from memory: " - << persistent_properties.error(); + << (!file_contents ? file_contents.error_string() : "Could not parse protobuf"); persistent_properties = LoadPersistentPropertiesFromMemory(); } - auto it = std::find_if(persistent_properties->begin(), persistent_properties->end(), - [&name](const auto& entry) { return entry.first == name; }); - if (it != persistent_properties->end()) { - *it = {name, value}; + auto it = std::find_if(persistent_properties.mutable_properties()->begin(), + persistent_properties.mutable_properties()->end(), + [&name](const auto& record) { return record.name() == name; }); + if (it != persistent_properties.mutable_properties()->end()) { + it->set_name(name); + it->set_value(value); } else { - persistent_properties->emplace_back(name, value); + AddPersistentProperty(name, value, &persistent_properties); } - if (auto result = WritePersistentPropertyFile(*persistent_properties); !result) { + if (auto result = WritePersistentPropertyFile(persistent_properties); !result) { LOG(ERROR) << "Could not store persistent property: " << result.error(); } } -std::vector> LoadPersistentProperties() { +PersistentProperties LoadPersistentProperties() { auto persistent_properties = LoadPersistentPropertyFile(); if (!persistent_properties) { diff --git a/init/persistent_properties.h b/init/persistent_properties.h index d84d9db4f..5f4df85bd 100644 --- a/init/persistent_properties.h +++ b/init/persistent_properties.h @@ -18,22 +18,19 @@ #define _INIT_PERSISTENT_PROPERTIES_H #include -#include #include "result.h" +#include "system/core/init/persistent_properties.pb.h" namespace android { namespace init { -std::vector> LoadPersistentProperties(); +PersistentProperties LoadPersistentProperties(); void WritePersistentProperty(const std::string& name, const std::string& value); // Exposed only for testing -Result>> LoadPersistentPropertyFile(); -std::string GenerateFileContents( - const std::vector>& persistent_properties); -Result WritePersistentPropertyFile( - const std::vector>& persistent_properties); +Result LoadPersistentPropertyFile(); +Result WritePersistentPropertyFile(const PersistentProperties& persistent_properties); extern std::string persistent_property_filename; } // namespace init diff --git a/init/persistent_properties.proto b/init/persistent_properties.proto new file mode 100644 index 000000000..c8d2e3ae9 --- /dev/null +++ b/init/persistent_properties.proto @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +syntax = "proto2"; +option optimize_for = LITE_RUNTIME; + +message PersistentProperties { + message PersistentPropertyRecord { + optional string name = 1; + optional string value = 2; + } + + repeated PersistentPropertyRecord properties = 1; +} diff --git a/init/persistent_properties_test.cpp b/init/persistent_properties_test.cpp index 9ab5b229c..875a4f384 100644 --- a/init/persistent_properties_test.cpp +++ b/init/persistent_properties_test.cpp @@ -18,6 +18,8 @@ #include +#include + #include #include @@ -28,34 +30,40 @@ using namespace std::string_literals; namespace android { namespace init { -TEST(persistent_properties, GeneratedContents) { - const std::vector> persistent_properties = { - {"persist.abc", ""}, - {"persist.def", "test_success"}, +PersistentProperties VectorToPersistentProperties( + const std::vector>& input_properties) { + PersistentProperties persistent_properties; + + for (const auto& [name, value] : input_properties) { + auto persistent_property_record = persistent_properties.add_properties(); + persistent_property_record->set_name(name); + persistent_property_record->set_value(value); + } + + return persistent_properties; +} + +void CheckPropertiesEqual(std::vector> expected, + const PersistentProperties& persistent_properties) { + for (const auto& persistent_property_record : persistent_properties.properties()) { + auto it = std::find_if(expected.begin(), expected.end(), + [persistent_property_record](const auto& entry) { + return entry.first == persistent_property_record.name() && + entry.second == persistent_property_record.value(); + }); + ASSERT_TRUE(it != expected.end()) + << "Found unexpected proprety (" << persistent_property_record.name() << ", " + << persistent_property_record.value() << ")"; + expected.erase(it); + } + auto joiner = [](const std::vector>& vector) { + std::string result; + for (const auto& [name, value] : vector) { + result += " (" + name + ", " + value + ")"; + } + return result; }; - auto generated_contents = GenerateFileContents(persistent_properties); - - // Manually serialized contents below: - std::string file_contents; - // All values below are written and read as little endian. - // Add magic value: 0x8495E0B4 - file_contents += "\xB4\xE0\x95\x84"s; - // Add version: 1 - file_contents += "\x01\x00\x00\x00"s; - // Add number of properties: 2 - file_contents += "\x02\x00\x00\x00"s; - - // Add first key: persist.abc - file_contents += "\x0B\x00\x00\x00persist.abc"s; - // Add first value: (empty string) - file_contents += "\x00\x00\x00\x00"s; - - // Add second key: persist.def - file_contents += "\x0B\x00\x00\x00persist.def"s; - // Add second value: test_success - file_contents += "\x0C\x00\x00\x00test_success"s; - - EXPECT_EQ(file_contents, generated_contents); + EXPECT_TRUE(expected.empty()) << "Did not find expected properties:" << joiner(expected); } TEST(persistent_properties, EndToEnd) { @@ -70,41 +78,15 @@ TEST(persistent_properties, EndToEnd) { {"persist.test.new.line", "abc\n\n\nabc"}, {"persist.test.numbers", "1234567890"}, {"persist.test.non.ascii", "\x00\x01\x02\xFF\xFE\xFD\x7F\x8F\x9F"}, - // We don't currently allow for non-ascii keys for system properties, but this is a policy + // We don't currently allow for non-ascii names for system properties, but this is a policy // decision, not a technical limitation. - {"persist.\x00\x01\x02\xFF\xFE\xFD\x7F\x8F\x9F", "non-ascii-key"}, + {"persist.\x00\x01\x02\xFF\xFE\xFD\x7F\x8F\x9F", "non-ascii-name"}, }; - ASSERT_TRUE(WritePersistentPropertyFile(persistent_properties)); + ASSERT_TRUE(WritePersistentPropertyFile(VectorToPersistentProperties(persistent_properties))); auto read_back_properties = LoadPersistentProperties(); - EXPECT_EQ(persistent_properties, read_back_properties); -} - -TEST(persistent_properties, BadMagic) { - TemporaryFile tf; - ASSERT_TRUE(tf.fd != -1); - persistent_property_filename = tf.path; - - ASSERT_TRUE(WriteFile(tf.path, "ab")); - - auto read_back_properties = LoadPersistentPropertyFile(); - - ASSERT_FALSE(read_back_properties); - EXPECT_EQ( - "Unable to parse persistent property file: Could not read magic value: Input buffer not " - "large enough to read uint32_t", - read_back_properties.error_string()); - - ASSERT_TRUE(WriteFile(tf.path, "\xFF\xFF\xFF\xFF")); - - read_back_properties = LoadPersistentPropertyFile(); - - ASSERT_FALSE(read_back_properties); - EXPECT_EQ( - "Unable to parse persistent property file: Magic value '0xffffffff' does not match " - "expected value '0x8495e0b4'", - read_back_properties.error_string()); + CheckPropertiesEqual(persistent_properties, read_back_properties); } TEST(persistent_properties, AddProperty) { @@ -115,7 +97,7 @@ TEST(persistent_properties, AddProperty) { std::vector> persistent_properties = { {"persist.sys.timezone", "America/Los_Angeles"}, }; - ASSERT_TRUE(WritePersistentPropertyFile(persistent_properties)); + ASSERT_TRUE(WritePersistentPropertyFile(VectorToPersistentProperties(persistent_properties))); WritePersistentProperty("persist.sys.locale", "pt-BR"); @@ -125,7 +107,7 @@ TEST(persistent_properties, AddProperty) { }; auto read_back_properties = LoadPersistentProperties(); - EXPECT_EQ(persistent_properties_expected, read_back_properties); + CheckPropertiesEqual(persistent_properties_expected, read_back_properties); } TEST(persistent_properties, UpdateProperty) { @@ -137,7 +119,7 @@ TEST(persistent_properties, UpdateProperty) { {"persist.sys.locale", "en-US"}, {"persist.sys.timezone", "America/Los_Angeles"}, }; - ASSERT_TRUE(WritePersistentPropertyFile(persistent_properties)); + ASSERT_TRUE(WritePersistentPropertyFile(VectorToPersistentProperties(persistent_properties))); WritePersistentProperty("persist.sys.locale", "pt-BR"); @@ -147,7 +129,7 @@ TEST(persistent_properties, UpdateProperty) { }; auto read_back_properties = LoadPersistentProperties(); - EXPECT_EQ(persistent_properties_expected, read_back_properties); + CheckPropertiesEqual(persistent_properties_expected, read_back_properties); } TEST(persistent_properties, UpdatePropertyBadParse) { @@ -160,13 +142,14 @@ TEST(persistent_properties, UpdatePropertyBadParse) { WritePersistentProperty("persist.sys.locale", "pt-BR"); auto read_back_properties = LoadPersistentProperties(); - EXPECT_GT(read_back_properties.size(), 0U); + EXPECT_GT(read_back_properties.properties().size(), 0); - auto it = std::find_if( - read_back_properties.begin(), read_back_properties.end(), [](const auto& entry) { - return entry.first == "persist.sys.locale" && entry.second == "pt-BR"; - }); - EXPECT_FALSE(it == read_back_properties.end()); + auto it = + std::find_if(read_back_properties.properties().begin(), + read_back_properties.properties().end(), [](const auto& entry) { + return entry.name() == "persist.sys.locale" && entry.value() == "pt-BR"; + }); + EXPECT_FALSE(it == read_back_properties.properties().end()); } } // namespace init diff --git a/init/property_service.cpp b/init/property_service.cpp index 5f5ed400a..db2d47230 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -641,8 +641,8 @@ void load_persist_props(void) { load_override_properties(); /* Read persistent properties after all default values have been loaded. */ auto persistent_properties = LoadPersistentProperties(); - for (const auto& [name, value] : persistent_properties) { - property_set(name, value); + for (const auto& persistent_property_record : persistent_properties.properties()) { + property_set(persistent_property_record.name(), persistent_property_record.value()); } persistent_properties_loaded = true; property_set("ro.persistent_properties.ready", "true");