Merge "libsnapshot:Snapuserd: IO path support with dm-snapshot target"
diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp
index d6fd513..47961d3 100644
--- a/fs_mgr/fs_mgr.cpp
+++ b/fs_mgr/fs_mgr.cpp
@@ -1296,14 +1296,15 @@
// When multiple fstab records share the same mount_point, it will try to mount each
// one in turn, and ignore any duplicates after a first successful mount.
// Returns -1 on error, and FS_MGR_MNTALL_* otherwise.
-int fs_mgr_mount_all(Fstab* fstab, int mount_mode) {
+MountAllResult fs_mgr_mount_all(Fstab* fstab, int mount_mode) {
int encryptable = FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE;
int error_count = 0;
CheckpointManager checkpoint_manager;
AvbUniquePtr avb_handle(nullptr);
+ bool userdata_mounted = false;
if (fstab->empty()) {
- return FS_MGR_MNTALL_FAIL;
+ return {FS_MGR_MNTALL_FAIL, userdata_mounted};
}
// Keep i int to prevent unsigned integer overflow from (i = top_idx - 1),
@@ -1343,7 +1344,7 @@
}
// Terrible hack to make it possible to remount /data.
- // TODO: refact fs_mgr_mount_all and get rid of this.
+ // TODO: refactor fs_mgr_mount_all and get rid of this.
if (mount_mode == MOUNT_MODE_ONLY_USERDATA && current_entry.mount_point != "/data") {
continue;
}
@@ -1380,7 +1381,7 @@
if (!avb_handle) {
LERROR << "Failed to open AvbHandle";
set_type_property(encryptable);
- return FS_MGR_MNTALL_FAIL;
+ return {FS_MGR_MNTALL_FAIL, userdata_mounted};
}
}
if (avb_handle->SetUpAvbHashtree(¤t_entry, true /* wait_for_verity_dev */) ==
@@ -1422,7 +1423,7 @@
if (status == FS_MGR_MNTALL_FAIL) {
// Fatal error - no point continuing.
- return status;
+ return {status, userdata_mounted};
}
if (status != FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE) {
@@ -1437,11 +1438,14 @@
nullptr)) {
LERROR << "Encryption failed";
set_type_property(encryptable);
- return FS_MGR_MNTALL_FAIL;
+ return {FS_MGR_MNTALL_FAIL, userdata_mounted};
}
}
}
+ if (current_entry.mount_point == "/data") {
+ userdata_mounted = true;
+ }
// Success! Go get the next one.
continue;
}
@@ -1541,9 +1545,9 @@
#endif
if (error_count) {
- return FS_MGR_MNTALL_FAIL;
+ return {FS_MGR_MNTALL_FAIL, userdata_mounted};
} else {
- return encryptable;
+ return {encryptable, userdata_mounted};
}
}
@@ -1765,8 +1769,8 @@
}
LINFO << "Remounting /data";
// TODO(b/143970043): remove this hack after fs_mgr_mount_all is refactored.
- int result = fs_mgr_mount_all(fstab, MOUNT_MODE_ONLY_USERDATA);
- return result == FS_MGR_MNTALL_FAIL ? -1 : 0;
+ auto result = fs_mgr_mount_all(fstab, MOUNT_MODE_ONLY_USERDATA);
+ return result.code == FS_MGR_MNTALL_FAIL ? -1 : 0;
}
return 0;
}
diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h
index 2a67b8c..11e3664 100644
--- a/fs_mgr/include/fs_mgr.h
+++ b/fs_mgr/include/fs_mgr.h
@@ -60,8 +60,20 @@
#define FS_MGR_MNTALL_DEV_NOT_ENCRYPTED 1
#define FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE 0
#define FS_MGR_MNTALL_FAIL (-1)
+
+struct MountAllResult {
+ // One of the FS_MGR_MNTALL_* returned code defined above.
+ int code;
+ // Whether userdata was mounted as a result of |fs_mgr_mount_all| call.
+ bool userdata_mounted;
+};
+
// fs_mgr_mount_all() updates fstab entries that reference device-mapper.
-int fs_mgr_mount_all(android::fs_mgr::Fstab* fstab, int mount_mode);
+// Returns a |MountAllResult|. The first element is one of the FS_MNG_MNTALL_* return codes
+// defined above, and the second element tells whether this call to fs_mgr_mount_all was responsible
+// for mounting userdata. Later is required for init to correctly enqueue fs-related events as part
+// of userdata remount during userspace reboot.
+MountAllResult fs_mgr_mount_all(android::fs_mgr::Fstab* fstab, int mount_mode);
#define FS_MGR_DOMNT_FAILED (-1)
#define FS_MGR_DOMNT_BUSY (-2)
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 597c32d..d00d1b1 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -662,18 +662,26 @@
}
}
- auto mount_fstab_return_code = fs_mgr_mount_all(&fstab, mount_all->mode);
+ auto mount_fstab_result = fs_mgr_mount_all(&fstab, mount_all->mode);
SetProperty(prop_name, std::to_string(t.duration().count()));
if (mount_all->import_rc) {
import_late(mount_all->rc_paths);
}
+ if (mount_fstab_result.userdata_mounted) {
+ // This call to fs_mgr_mount_all mounted userdata. Keep the result in
+ // order for userspace reboot to correctly remount userdata.
+ LOG(INFO) << "Userdata mounted using "
+ << (mount_all->fstab_path.empty() ? "(default fstab)" : mount_all->fstab_path)
+ << " result : " << mount_fstab_result.code;
+ initial_mount_fstab_return_code = mount_fstab_result.code;
+ }
+
if (queue_event) {
/* queue_fs_event will queue event based on mount_fstab return code
* and return processed return code*/
- initial_mount_fstab_return_code = mount_fstab_return_code;
- auto queue_fs_result = queue_fs_event(mount_fstab_return_code, false);
+ auto queue_fs_result = queue_fs_event(mount_fstab_result.code, false);
if (!queue_fs_result.ok()) {
return Error() << "queue_fs_event() failed: " << queue_fs_result.error();
}
diff --git a/libutils/Android.bp b/libutils/Android.bp
index 9c012a9..926e3d7 100644
--- a/libutils/Android.bp
+++ b/libutils/Android.bp
@@ -132,7 +132,6 @@
"JenkinsHash.cpp",
"NativeHandle.cpp",
"Printer.cpp",
- "PropertyMap.cpp",
"RefBase.cpp",
"SharedBuffer.cpp",
"StopWatch.cpp",
@@ -270,12 +269,6 @@
}
cc_fuzz {
- name: "libutils_fuzz_propertymap",
- defaults: ["libutils_fuzz_defaults"],
- srcs: ["PropertyMap_fuzz.cpp"],
-}
-
-cc_fuzz {
name: "libutils_fuzz_rwlock",
defaults: ["libutils_fuzz_defaults"],
srcs: ["RWLock_fuzz.cpp"],
diff --git a/libutils/PropertyMap.cpp b/libutils/PropertyMap.cpp
deleted file mode 100644
index f00272a..0000000
--- a/libutils/PropertyMap.cpp
+++ /dev/null
@@ -1,214 +0,0 @@
-/*
- * Copyright (C) 2008 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.
- */
-
-#define LOG_TAG "PropertyMap"
-
-#include <utils/PropertyMap.h>
-
-// Enables debug output for the parser.
-#define DEBUG_PARSER 0
-
-// Enables debug output for parser performance.
-#define DEBUG_PARSER_PERFORMANCE 0
-
-
-namespace android {
-
-static const char* WHITESPACE = " \t\r";
-static const char* WHITESPACE_OR_PROPERTY_DELIMITER = " \t\r=";
-
-
-// --- PropertyMap ---
-
-PropertyMap::PropertyMap() {
-}
-
-PropertyMap::~PropertyMap() {
-}
-
-void PropertyMap::clear() {
- mProperties.clear();
-}
-
-void PropertyMap::addProperty(const String8& key, const String8& value) {
- mProperties.add(key, value);
-}
-
-bool PropertyMap::hasProperty(const String8& key) const {
- return mProperties.indexOfKey(key) >= 0;
-}
-
-bool PropertyMap::tryGetProperty(const String8& key, String8& outValue) const {
- ssize_t index = mProperties.indexOfKey(key);
- if (index < 0) {
- return false;
- }
-
- outValue = mProperties.valueAt(index);
- return true;
-}
-
-bool PropertyMap::tryGetProperty(const String8& key, bool& outValue) const {
- int32_t intValue;
- if (!tryGetProperty(key, intValue)) {
- return false;
- }
-
- outValue = intValue;
- return true;
-}
-
-bool PropertyMap::tryGetProperty(const String8& key, int32_t& outValue) const {
- String8 stringValue;
- if (! tryGetProperty(key, stringValue) || stringValue.length() == 0) {
- return false;
- }
-
- char* end;
- int value = strtol(stringValue.string(), & end, 10);
- if (*end != '\0') {
- ALOGW("Property key '%s' has invalid value '%s'. Expected an integer.",
- key.string(), stringValue.string());
- return false;
- }
- outValue = value;
- return true;
-}
-
-bool PropertyMap::tryGetProperty(const String8& key, float& outValue) const {
- String8 stringValue;
- if (! tryGetProperty(key, stringValue) || stringValue.length() == 0) {
- return false;
- }
-
- char* end;
- float value = strtof(stringValue.string(), & end);
- if (*end != '\0') {
- ALOGW("Property key '%s' has invalid value '%s'. Expected a float.",
- key.string(), stringValue.string());
- return false;
- }
- outValue = value;
- return true;
-}
-
-void PropertyMap::addAll(const PropertyMap* map) {
- for (size_t i = 0; i < map->mProperties.size(); i++) {
- mProperties.add(map->mProperties.keyAt(i), map->mProperties.valueAt(i));
- }
-}
-
-status_t PropertyMap::load(const String8& filename, PropertyMap** outMap) {
- *outMap = nullptr;
-
- Tokenizer* tokenizer;
- status_t status = Tokenizer::open(filename, &tokenizer);
- if (status) {
- ALOGE("Error %d opening property file %s.", status, filename.string());
- } else {
- PropertyMap* map = new PropertyMap();
- if (!map) {
- ALOGE("Error allocating property map.");
- status = NO_MEMORY;
- } else {
-#if DEBUG_PARSER_PERFORMANCE
- nsecs_t startTime = systemTime(SYSTEM_TIME_MONOTONIC);
-#endif
- Parser parser(map, tokenizer);
- status = parser.parse();
-#if DEBUG_PARSER_PERFORMANCE
- nsecs_t elapsedTime = systemTime(SYSTEM_TIME_MONOTONIC) - startTime;
- ALOGD("Parsed property file '%s' %d lines in %0.3fms.",
- tokenizer->getFilename().string(), tokenizer->getLineNumber(),
- elapsedTime / 1000000.0);
-#endif
- if (status) {
- delete map;
- } else {
- *outMap = map;
- }
- }
- delete tokenizer;
- }
- return status;
-}
-
-
-// --- PropertyMap::Parser ---
-
-PropertyMap::Parser::Parser(PropertyMap* map, Tokenizer* tokenizer) :
- mMap(map), mTokenizer(tokenizer) {
-}
-
-PropertyMap::Parser::~Parser() {
-}
-
-status_t PropertyMap::Parser::parse() {
- while (!mTokenizer->isEof()) {
-#if DEBUG_PARSER
- ALOGD("Parsing %s: '%s'.", mTokenizer->getLocation().string(),
- mTokenizer->peekRemainderOfLine().string());
-#endif
-
- mTokenizer->skipDelimiters(WHITESPACE);
-
- if (!mTokenizer->isEol() && mTokenizer->peekChar() != '#') {
- String8 keyToken = mTokenizer->nextToken(WHITESPACE_OR_PROPERTY_DELIMITER);
- if (keyToken.isEmpty()) {
- ALOGE("%s: Expected non-empty property key.", mTokenizer->getLocation().string());
- return BAD_VALUE;
- }
-
- mTokenizer->skipDelimiters(WHITESPACE);
-
- if (mTokenizer->nextChar() != '=') {
- ALOGE("%s: Expected '=' between property key and value.",
- mTokenizer->getLocation().string());
- return BAD_VALUE;
- }
-
- mTokenizer->skipDelimiters(WHITESPACE);
-
- String8 valueToken = mTokenizer->nextToken(WHITESPACE);
- if (valueToken.find("\\", 0) >= 0 || valueToken.find("\"", 0) >= 0) {
- ALOGE("%s: Found reserved character '\\' or '\"' in property value.",
- mTokenizer->getLocation().string());
- return BAD_VALUE;
- }
-
- mTokenizer->skipDelimiters(WHITESPACE);
- if (!mTokenizer->isEol()) {
- ALOGE("%s: Expected end of line, got '%s'.",
- mTokenizer->getLocation().string(),
- mTokenizer->peekRemainderOfLine().string());
- return BAD_VALUE;
- }
-
- if (mMap->hasProperty(keyToken)) {
- ALOGE("%s: Duplicate property value for key '%s'.",
- mTokenizer->getLocation().string(), keyToken.string());
- return BAD_VALUE;
- }
-
- mMap->addProperty(keyToken, valueToken);
- }
-
- mTokenizer->nextLine();
- }
- return OK;
-}
-
-} // namespace android
diff --git a/libutils/PropertyMap_fuzz.cpp b/libutils/PropertyMap_fuzz.cpp
deleted file mode 100755
index fd50729..0000000
--- a/libutils/PropertyMap_fuzz.cpp
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * Copyright 2020 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.
- */
-
-#include "android-base/file.h"
-#include "fuzzer/FuzzedDataProvider.h"
-#include "utils/PropertyMap.h"
-#include "utils/String8.h"
-
-static constexpr int MAX_FILE_SIZE = 256;
-static constexpr int MAX_STR_LEN = 2048;
-static constexpr int MAX_OPERATIONS = 1000;
-
-static const std::vector<std::function<void(FuzzedDataProvider*, android::PropertyMap)>>
- operations = {
- [](FuzzedDataProvider*, android::PropertyMap propertyMap) -> void {
- propertyMap.getProperties();
- },
- [](FuzzedDataProvider*, android::PropertyMap propertyMap) -> void {
- propertyMap.clear();
- },
- [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void {
- std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN);
- android::String8 key = android::String8(keyStr.c_str());
- propertyMap.hasProperty(key);
- },
- [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void {
- std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN);
- android::String8 key = android::String8(keyStr.c_str());
- android::String8 out;
- propertyMap.tryGetProperty(key, out);
- },
- [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void {
- TemporaryFile tf;
- // Generate file contents
- std::string contents = dataProvider->ConsumeRandomLengthString(MAX_FILE_SIZE);
- // If we have string contents, dump them into the file.
- // Otherwise, just leave it as an empty file.
- if (contents.length() > 0) {
- const char* bytes = contents.c_str();
- android::base::WriteStringToFd(bytes, tf.fd);
- }
- android::PropertyMap* mapPtr = &propertyMap;
- android::PropertyMap::load(android::String8(tf.path), &mapPtr);
- },
- [](FuzzedDataProvider* dataProvider, android::PropertyMap propertyMap) -> void {
- std::string keyStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN);
- std::string valStr = dataProvider->ConsumeRandomLengthString(MAX_STR_LEN);
- android::String8 key = android::String8(keyStr.c_str());
- android::String8 val = android::String8(valStr.c_str());
- propertyMap.addProperty(key, val);
- },
-};
-extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
- FuzzedDataProvider dataProvider(data, size);
- android::PropertyMap proprtyMap = android::PropertyMap();
-
- int opsRun = 0;
- while (dataProvider.remaining_bytes() > 0 && opsRun++ < MAX_OPERATIONS) {
- uint8_t op = dataProvider.ConsumeIntegralInRange<uint8_t>(0, operations.size() - 1);
- operations[op](&dataProvider, proprtyMap);
- }
- return 0;
-}
diff --git a/libutils/include/utils/PropertyMap.h b/libutils/include/utils/PropertyMap.h
deleted file mode 100644
index a9e674f..0000000
--- a/libutils/include/utils/PropertyMap.h
+++ /dev/null
@@ -1,106 +0,0 @@
-/*
- * Copyright (C) 2010 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.
- */
-
-#ifndef _UTILS_PROPERTY_MAP_H
-#define _UTILS_PROPERTY_MAP_H
-
-#include <utils/KeyedVector.h>
-#include <utils/String8.h>
-#include <utils/Errors.h>
-#include <utils/Tokenizer.h>
-
-namespace android {
-
-/*
- * Provides a mechanism for passing around string-based property key / value pairs
- * and loading them from property files.
- *
- * The property files have the following simple structure:
- *
- * # Comment
- * key = value
- *
- * Keys and values are any sequence of printable ASCII characters.
- * The '=' separates the key from the value.
- * The key and value may not contain whitespace.
- *
- * The '\' character is reserved for escape sequences and is not currently supported.
- * The '"" character is reserved for quoting and is not currently supported.
- * Files that contain the '\' or '"' character will fail to parse.
- *
- * The file must not contain duplicate keys.
- *
- * TODO Support escape sequences and quoted values when needed.
- */
-class PropertyMap {
-public:
- /* Creates an empty property map. */
- PropertyMap();
- ~PropertyMap();
-
- /* Clears the property map. */
- void clear();
-
- /* Adds a property.
- * Replaces the property with the same key if it is already present.
- */
- void addProperty(const String8& key, const String8& value);
-
- /* Returns true if the property map contains the specified key. */
- bool hasProperty(const String8& key) const;
-
- /* Gets the value of a property and parses it.
- * Returns true and sets outValue if the key was found and its value was parsed successfully.
- * Otherwise returns false and does not modify outValue. (Also logs a warning.)
- */
- bool tryGetProperty(const String8& key, String8& outValue) const;
- bool tryGetProperty(const String8& key, bool& outValue) const;
- bool tryGetProperty(const String8& key, int32_t& outValue) const;
- bool tryGetProperty(const String8& key, float& outValue) const;
-
- /* Adds all values from the specified property map. */
- void addAll(const PropertyMap* map);
-
- /* Gets the underlying property map. */
- inline const KeyedVector<String8, String8>& getProperties() const { return mProperties; }
-
- /* Loads a property map from a file. */
- static status_t load(const String8& filename, PropertyMap** outMap);
-
-private:
- class Parser {
- PropertyMap* mMap;
- Tokenizer* mTokenizer;
-
- public:
- Parser(PropertyMap* map, Tokenizer* tokenizer);
- ~Parser();
- status_t parse();
-
- private:
- status_t parseType();
- status_t parseKey();
- status_t parseKeyProperty();
- status_t parseModifier(const String8& token, int32_t* outMetaState);
- status_t parseCharacterLiteral(char16_t* outCharacter);
- };
-
- KeyedVector<String8, String8> mProperties;
-};
-
-} // namespace android
-
-#endif // _UTILS_PROPERTY_MAP_H
diff --git a/logd/SerializedLogChunk.cpp b/logd/SerializedLogChunk.cpp
index de641d6..e4d8945 100644
--- a/logd/SerializedLogChunk.cpp
+++ b/logd/SerializedLogChunk.cpp
@@ -25,12 +25,10 @@
}
void SerializedLogChunk::Compress() {
- if (compressed_log_.size() == 0) {
- CompressionEngine::GetInstance().Compress(contents_, write_offset_, compressed_log_);
- LOG(INFO) << "Compressed Log, buffer max size: " << contents_.size()
- << " size used: " << write_offset_
- << " compressed size: " << compressed_log_.size();
- }
+ CHECK_EQ(compressed_log_.size(), 0U);
+ CompressionEngine::GetInstance().Compress(contents_, write_offset_, compressed_log_);
+ LOG(INFO) << "Compressed Log, buffer max size: " << contents_.size()
+ << " size used: " << write_offset_ << " compressed size: " << compressed_log_.size();
}
// TODO: Develop a better reference counting strategy to guard against the case where the writer is
@@ -89,9 +87,11 @@
// Clear the old compressed logs and set write_offset_ appropriately to compress the new
// partially cleared log.
if (new_write_offset != write_offset_) {
- compressed_log_.Resize(0);
write_offset_ = new_write_offset;
- Compress();
+ if (!writer_active_) {
+ compressed_log_.Resize(0);
+ Compress();
+ }
}
DecReaderRefCount();
diff --git a/logd/SerializedLogChunkTest.cpp b/logd/SerializedLogChunkTest.cpp
index f10b9c6..3b45125 100644
--- a/logd/SerializedLogChunkTest.cpp
+++ b/logd/SerializedLogChunkTest.cpp
@@ -281,3 +281,27 @@
}
INSTANTIATE_TEST_CASE_P(UidClearTests, UidClearTest, testing::Values(true, false));
+
+// b/166187079: ClearUidLogs() should not compress the log if writer_active_ is true
+TEST(SerializedLogChunk, ClearUidLogs_then_FinishWriting) {
+ static constexpr size_t kChunkSize = 10 * 4096;
+ static constexpr uid_t kUidToClear = 1000;
+ static constexpr uid_t kOtherUid = 1234;
+
+ SerializedLogChunk chunk{kChunkSize};
+ static const char msg1[] = "saved first message";
+ static const char msg2[] = "cleared interior message";
+ static const char msg3[] = "last message stays";
+ ASSERT_NE(nullptr, chunk.Log(1, log_time(), kOtherUid, 1, 2, msg1, sizeof(msg1)));
+ ASSERT_NE(nullptr, chunk.Log(2, log_time(), kUidToClear, 1, 2, msg2, sizeof(msg2)));
+ ASSERT_NE(nullptr, chunk.Log(3, log_time(), kOtherUid, 1, 2, msg3, sizeof(msg3)));
+
+ chunk.ClearUidLogs(kUidToClear, LOG_ID_MAIN, nullptr);
+
+ ASSERT_NE(nullptr, chunk.Log(4, log_time(), kOtherUid, 1, 2, msg3, sizeof(msg3)));
+
+ chunk.FinishWriting();
+ // The next line would violate a CHECK() during decompression with the faulty code.
+ chunk.IncReaderRefCount();
+ chunk.DecReaderRefCount();
+}