Merge "remount: Fix failure for system-as-root"
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index 4634283..24804d0 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -51,6 +51,7 @@
#include <android-base/test_utils.h>
#include <android-base/unique_fd.h>
#include <cutils/sockets.h>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <libminijail.h>
@@ -65,6 +66,7 @@
using android::base::SendFileDescriptors;
using android::base::unique_fd;
+using ::testing::HasSubstr;
#if defined(__LP64__)
#define ARCH_SUFFIX "64"
@@ -307,6 +309,19 @@
*output = std::move(result);
}
+class LogcatCollector {
+ public:
+ LogcatCollector() { system("logcat -c"); }
+
+ void Collect(std::string* output) {
+ FILE* cmd_stdout = popen("logcat -d '*:S DEBUG'", "r");
+ ASSERT_NE(cmd_stdout, nullptr);
+ unique_fd tmp_fd(TEMP_FAILURE_RETRY(dup(fileno(cmd_stdout))));
+ ConsumeFd(std::move(tmp_fd), output);
+ pclose(cmd_stdout);
+ }
+};
+
TEST_F(CrasherTest, smoke) {
int intercept_result;
unique_fd output_fd;
@@ -441,6 +456,7 @@
}
GwpAsanTestParameters params = GetParam();
+ LogcatCollector logcat_collector;
int intercept_result;
unique_fd output_fd;
@@ -460,17 +476,18 @@
ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
- std::string result;
- ConsumeFd(std::move(output_fd), &result);
+ std::vector<std::string> log_sources(2);
+ ConsumeFd(std::move(output_fd), &log_sources[0]);
+ logcat_collector.Collect(&log_sources[1]);
- ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 2 \(SEGV_ACCERR\))");
- ASSERT_MATCH(result, R"(Cause: \[GWP-ASan\]: )" + params.cause_needle);
- if (params.free_before_access) {
- ASSERT_MATCH(result, R"(deallocated by thread .*
- #00 pc)");
+ for (const auto& result : log_sources) {
+ ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 2 \(SEGV_ACCERR\))");
+ ASSERT_MATCH(result, R"(Cause: \[GWP-ASan\]: )" + params.cause_needle);
+ if (params.free_before_access) {
+ ASSERT_MATCH(result, R"(deallocated by thread .*\n.*#00 pc)");
+ }
+ ASSERT_MATCH(result, R"((^|\s)allocated by thread .*\n.*#00 pc)");
}
- ASSERT_MATCH(result, R"(allocated by thread .*
- #00 pc)");
}
struct SizeParamCrasherTest : CrasherTest, testing::WithParamInterface<size_t> {};
@@ -488,6 +505,8 @@
return;
}
+ LogcatCollector logcat_collector;
+
int intercept_result;
unique_fd output_fd;
StartProcess([&]() {
@@ -504,16 +523,17 @@
ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
- std::string result;
- ConsumeFd(std::move(output_fd), &result);
+ std::vector<std::string> log_sources(2);
+ ConsumeFd(std::move(output_fd), &log_sources[0]);
+ logcat_collector.Collect(&log_sources[1]);
- ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
- ASSERT_MATCH(result, R"(Cause: \[MTE\]: Use After Free, 0 bytes into a )" +
- std::to_string(GetParam()) + R"(-byte allocation)");
- ASSERT_MATCH(result, R"(deallocated by thread .*
- #00 pc)");
- ASSERT_MATCH(result, R"(allocated by thread .*
- #00 pc)");
+ for (const auto& result : log_sources) {
+ ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
+ ASSERT_MATCH(result, R"(Cause: \[MTE\]: Use After Free, 0 bytes into a )" +
+ std::to_string(GetParam()) + R"(-byte allocation)");
+ ASSERT_MATCH(result, R"(deallocated by thread .*?\n.*#00 pc)");
+ ASSERT_MATCH(result, R"((^|\s)allocated by thread .*?\n.*#00 pc)");
+ }
#else
GTEST_SKIP() << "Requires aarch64";
#endif
@@ -557,6 +577,7 @@
GTEST_SKIP() << "Requires MTE";
}
+ LogcatCollector logcat_collector;
int intercept_result;
unique_fd output_fd;
StartProcess([&]() {
@@ -572,14 +593,16 @@
ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
- std::string result;
- ConsumeFd(std::move(output_fd), &result);
+ std::vector<std::string> log_sources(2);
+ ConsumeFd(std::move(output_fd), &log_sources[0]);
+ logcat_collector.Collect(&log_sources[1]);
- ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
- ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a )" +
- std::to_string(GetParam()) + R"(-byte allocation)");
- ASSERT_MATCH(result, R"(allocated by thread .*
- #00 pc)");
+ for (const auto& result : log_sources) {
+ ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
+ ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a )" +
+ std::to_string(GetParam()) + R"(-byte allocation)");
+ ASSERT_MATCH(result, R"((^|\s)allocated by thread .*?\n.*#00 pc)");
+ }
#else
GTEST_SKIP() << "Requires aarch64";
#endif
@@ -612,7 +635,7 @@
ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\), code 9 \(SEGV_MTESERR\))");
ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Underflow, 4 bytes left of a )" +
std::to_string(GetParam()) + R"(-byte allocation)");
- ASSERT_MATCH(result, R"(allocated by thread .*
+ ASSERT_MATCH(result, R"((^|\s)allocated by thread .*
#00 pc)");
#else
GTEST_SKIP() << "Requires aarch64";
@@ -625,6 +648,8 @@
GTEST_SKIP() << "Requires MTE";
}
+ LogcatCollector logcat_collector;
+
int intercept_result;
unique_fd output_fd;
StartProcess([]() {
@@ -657,17 +682,23 @@
ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
- std::string result;
- ConsumeFd(std::move(output_fd), &result);
+ std::vector<std::string> log_sources(2);
+ ConsumeFd(std::move(output_fd), &log_sources[0]);
+ logcat_collector.Collect(&log_sources[1]);
- ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
- ASSERT_MATCH(
- result,
- R"(Note: multiple potential causes for this crash were detected, listing them in decreasing order of probability.)");
-
- // Adjacent untracked allocations may cause us to see the wrong underflow here (or only
- // overflows), so we can't match explicitly for an underflow message.
- ASSERT_MATCH(result, R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a 16-byte allocation)");
+ for (const auto& result : log_sources) {
+ ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))");
+ ASSERT_THAT(result, HasSubstr("Note: multiple potential causes for this crash were detected, "
+ "listing them in decreasing order of probability."));
+ // Adjacent untracked allocations may cause us to see the wrong underflow here (or only
+ // overflows), so we can't match explicitly for an underflow message.
+ ASSERT_MATCH(result,
+ R"(Cause: \[MTE\]: Buffer Overflow, 0 bytes right of a 16-byte allocation)");
+ // Ensure there's at least two allocation traces (one for each cause).
+ ASSERT_MATCH(
+ result,
+ R"((^|\s)allocated by thread .*?\n.*#00 pc(.|\n)*?(^|\s)allocated by thread .*?\n.*#00 pc)");
+ }
#else
GTEST_SKIP() << "Requires aarch64";
#endif
diff --git a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp
index b780b22..a932d48 100644
--- a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp
+++ b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp
@@ -272,14 +272,14 @@
if (tombstone.causes_size() > 1) {
CBS("");
- CBS("Note: multiple potential causes for this crash were detected, listing them in decreasing "
+ CBL("Note: multiple potential causes for this crash were detected, listing them in decreasing "
"order of probability.");
}
for (const Cause& cause : tombstone.causes()) {
if (tombstone.causes_size() > 1) {
CBS("");
- CBS("Cause: %s", cause.human_readable().c_str());
+ CBL("Cause: %s", cause.human_readable().c_str());
}
if (cause.has_memory_error() && cause.memory_error().has_heap()) {
@@ -287,14 +287,14 @@
if (heap_object.deallocation_backtrace_size() != 0) {
CBS("");
- CBS("deallocated by thread %" PRIu64 ":", heap_object.deallocation_tid());
- print_backtrace(callback, tombstone, heap_object.deallocation_backtrace(), false);
+ CBL("deallocated by thread %" PRIu64 ":", heap_object.deallocation_tid());
+ print_backtrace(callback, tombstone, heap_object.deallocation_backtrace(), true);
}
if (heap_object.allocation_backtrace_size() != 0) {
CBS("");
- CBS("allocated by thread %" PRIu64 ":", heap_object.allocation_tid());
- print_backtrace(callback, tombstone, heap_object.allocation_backtrace(), false);
+ CBL("allocated by thread %" PRIu64 ":", heap_object.allocation_tid());
+ print_backtrace(callback, tombstone, heap_object.allocation_backtrace(), true);
}
}
}
diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp
index 08ead7a..af71fe6 100644
--- a/fs_mgr/fs_mgr.cpp
+++ b/fs_mgr/fs_mgr.cpp
@@ -2266,6 +2266,26 @@
return LP_METADATA_DEFAULT_PARTITION_NAME;
}
+bool fs_mgr_create_canonical_mount_point(const std::string& mount_point) {
+ auto saved_errno = errno;
+ auto ok = true;
+ auto created_mount_point = !mkdir(mount_point.c_str(), 0755);
+ std::string real_mount_point;
+ if (!Realpath(mount_point, &real_mount_point)) {
+ ok = false;
+ PERROR << "failed to realpath(" << mount_point << ")";
+ } else if (mount_point != real_mount_point) {
+ ok = false;
+ LERROR << "mount point is not canonical: realpath(" << mount_point << ") -> "
+ << real_mount_point;
+ }
+ if (!ok && created_mount_point) {
+ rmdir(mount_point.c_str());
+ }
+ errno = saved_errno;
+ return ok;
+}
+
bool fs_mgr_mount_overlayfs_fstab_entry(const FstabEntry& entry) {
auto overlayfs_valid_result = fs_mgr_overlayfs_valid();
if (overlayfs_valid_result == OverlayfsValidResult::kNotSupported) {
@@ -2298,18 +2318,7 @@
}
#endif // ALLOW_ADBD_DISABLE_VERITY == 0
- // Create the mount point in case it doesn't exist.
- mkdir(entry.mount_point.c_str(), 0755);
-
- // Ensure that mount point exists and doesn't contain symbolic link or /../.
- std::string mount_point;
- if (!Realpath(entry.mount_point, &mount_point)) {
- PERROR << __FUNCTION__ << "(): failed to realpath " << entry.mount_point;
- return false;
- }
- if (entry.mount_point != mount_point) {
- LERROR << __FUNCTION__ << "(): mount point must be a canonicalized path: realpath "
- << entry.mount_point << " = " << mount_point;
+ if (!fs_mgr_create_canonical_mount_point(entry.mount_point)) {
return false;
}
diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp
index 853b24d..d0c89b9 100644
--- a/fs_mgr/fs_mgr_fstab.cpp
+++ b/fs_mgr/fs_mgr_fstab.cpp
@@ -127,15 +127,16 @@
}
fs_options.append(flag);
- if (entry->fs_type == "f2fs" && StartsWith(flag, "reserve_root=")) {
- std::string arg;
- if (auto equal_sign = flag.find('='); equal_sign != std::string::npos) {
- arg = flag.substr(equal_sign + 1);
- }
- if (!ParseInt(arg, &entry->reserved_size)) {
- LWARNING << "Warning: reserve_root= flag malformed: " << arg;
- } else {
- entry->reserved_size <<= 12;
+ if (auto equal_sign = flag.find('='); equal_sign != std::string::npos) {
+ const auto arg = flag.substr(equal_sign + 1);
+ if (entry->fs_type == "f2fs" && StartsWith(flag, "reserve_root=")) {
+ if (!ParseInt(arg, &entry->reserved_size)) {
+ LWARNING << "Warning: reserve_root= flag malformed: " << arg;
+ } else {
+ entry->reserved_size <<= 12;
+ }
+ } else if (StartsWith(flag, "lowerdir=")) {
+ entry->lowerdir = std::move(arg);
}
}
}
@@ -298,8 +299,6 @@
if (!ParseByteCount(arg, &entry->zram_backingdev_size)) {
LWARNING << "Warning: zram_backingdev_size= flag malformed: " << arg;
}
- } else if (StartsWith(flag, "lowerdir=")) {
- entry->lowerdir = arg;
} else {
LWARNING << "Warning: unknown flag: " << flag;
}
diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp
index cb09383..4d32bda 100644
--- a/fs_mgr/fs_mgr_overlayfs.cpp
+++ b/fs_mgr/fs_mgr_overlayfs.cpp
@@ -92,10 +92,6 @@
return false;
}
-std::vector<std::string> fs_mgr_overlayfs_required_devices(Fstab*) {
- return {};
-}
-
bool fs_mgr_overlayfs_setup(const char*, const char*, bool* change, bool) {
if (change) *change = false;
return false;
diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h
index b8ebd63..4d3ecc9 100644
--- a/fs_mgr/include/fs_mgr.h
+++ b/fs_mgr/include/fs_mgr.h
@@ -132,6 +132,10 @@
// empty string
std::string fs_mgr_find_bow_device(const std::string& block_device);
+// Creates mount point if not already existed, and checks that mount point is a
+// canonical path that doesn't contain any symbolic link or /../.
+bool fs_mgr_create_canonical_mount_point(const std::string& mount_point);
+
// Like fs_mgr_do_mount_one() but for overlayfs fstab entries.
// Unlike fs_mgr_overlayfs, mount overlayfs without upperdir and workdir, so the
// filesystem cannot be remount read-write.
diff --git a/fs_mgr/include/fs_mgr_overlayfs.h b/fs_mgr/include/fs_mgr_overlayfs.h
index d45e2de..6caab1f 100644
--- a/fs_mgr/include/fs_mgr_overlayfs.h
+++ b/fs_mgr/include/fs_mgr_overlayfs.h
@@ -27,7 +27,6 @@
android::fs_mgr::Fstab fs_mgr_overlayfs_candidate_list(const android::fs_mgr::Fstab& fstab);
bool fs_mgr_overlayfs_mount_all(android::fs_mgr::Fstab* fstab);
-std::vector<std::string> fs_mgr_overlayfs_required_devices(android::fs_mgr::Fstab* fstab);
bool fs_mgr_overlayfs_setup(const char* backing = nullptr, const char* mount_point = nullptr,
bool* change = nullptr, bool force = true);
bool fs_mgr_overlayfs_teardown(const char* mount_point = nullptr, bool* change = nullptr);
diff --git a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto
index 92aa55c..77ed92c 100644
--- a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto
+++ b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto
@@ -213,6 +213,7 @@
// Time from sys.boot_completed to merge start, in milliseconds.
uint32 boot_complete_to_merge_start_time_ms = 8;
- // Merge failure code, filled if state == MergeFailed.
+ // Merge failure code, filled if the merge failed at any time (regardless
+ // of whether it succeeded at a later time).
MergeFailureCode merge_failure_code = 9;
}
diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh
index 0c5fe3d..9542bc1 100755
--- a/fs_mgr/tests/adb-remount-test.sh
+++ b/fs_mgr/tests/adb-remount-test.sh
@@ -762,15 +762,15 @@
"ramdumpfs" "binder" "securityfs" "functionfs" "rootfs"
)
local exclude_devices=(
- "[/]sys[/]kernel[/]debug" "[/]data[/]media" "[/]dev[/]block[/]loop[0-9]*"
+ "\/sys\/kernel\/debug" "\/data\/media" "\/dev\/block\/loop[0-9]*"
"${exclude_filesystems[@]}"
)
local exclude_mount_points=(
- "[/]cache" "[/]mnt[/]scratch" "[/]mnt[/]vendor[/]persist" "[/]persist"
- "[/]metadata"
+ "\/cache" "\/mnt\/scratch" "\/mnt\/vendor\/persist" "\/persist"
+ "\/metadata"
)
if [ "data" = "${1}" ]; then
- exclude_mount_points+=("[/]data")
+ exclude_mount_points+=("\/data")
fi
awk '$1 !~ /^('"$(join_with "|" "${exclude_devices[@]}")"')$/ &&
$2 !~ /^('"$(join_with "|" "${exclude_mount_points[@]}")"')$/ &&
@@ -934,7 +934,7 @@
PARTITIONS=`adb_su cat /vendor/etc/fstab* </dev/null |
grep -v "^[#${SPACE}${TAB}]" |
skip_administrative_mounts |
- awk '$1 ~ /^[^/]+$/ && "/"$1 == $2 && $4 ~ /(^|,)ro(,|$)/ { print $1 }' |
+ awk '$1 ~ /^[^\/]+$/ && "/"$1 == $2 && $4 ~ /(^|,)ro(,|$)/ { print $1 }' |
sort -u |
tr '\n' ' '`
PARTITIONS="${PARTITIONS:-system vendor}"
diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp
index eb2919b..a1b020a 100644
--- a/fs_mgr/tests/fs_mgr_test.cpp
+++ b/fs_mgr/tests/fs_mgr_test.cpp
@@ -120,7 +120,7 @@
};
const std::string bootconfig =
- "androidboot.bootdevice = \" \"1d84000.ufshc\"\n"
+ "androidboot.bootdevice = \"1d84000.ufshc\"\n"
"androidboot.boot_devices = \"dev1\", \"dev2,withcomma\", \"dev3\"\n"
"androidboot.baseband = \"sdy\"\n"
"androidboot.keymaster = \"1\"\n"
diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp
index 546ea8e..f5c10bb 100644
--- a/init/first_stage_mount.cpp
+++ b/init/first_stage_mount.cpp
@@ -420,6 +420,10 @@
*end = begin + 1;
}
+ if (!fs_mgr_create_canonical_mount_point(begin->mount_point)) {
+ return false;
+ }
+
if (begin->fs_mgr_flags.logical) {
if (!fs_mgr_update_logical_partition(&(*begin))) {
return false;
diff --git a/init/ueventd.cpp b/init/ueventd.cpp
index 331255b..f9ee9e3 100644
--- a/init/ueventd.cpp
+++ b/init/ueventd.cpp
@@ -268,14 +268,27 @@
}
static UeventdConfiguration GetConfiguration() {
- // TODO: Remove these legacy paths once Android S is no longer supported.
+ auto hardware = android::base::GetProperty("ro.hardware", "");
+ std::vector<std::string> legacy_paths{"/vendor/ueventd.rc", "/odm/ueventd.rc",
+ "/ueventd." + hardware + ".rc"};
+
+ std::vector<std::string> canonical{"/system/etc/ueventd.rc"};
+
if (android::base::GetIntProperty("ro.product.first_api_level", 10000) <= __ANDROID_API_S__) {
- auto hardware = android::base::GetProperty("ro.hardware", "");
- return ParseConfig({"/system/etc/ueventd.rc", "/vendor/ueventd.rc", "/odm/ueventd.rc",
- "/ueventd." + hardware + ".rc"});
+ // TODO: Remove these legacy paths once Android S is no longer supported.
+ canonical.insert(canonical.end(), legacy_paths.begin(), legacy_paths.end());
+ } else {
+ // Warn if newer device is using legacy paths.
+ for (const auto& path : legacy_paths) {
+ if (access(path.c_str(), F_OK) == 0) {
+ LOG(FATAL_WITHOUT_ABORT)
+ << "Legacy ueventd configuration file detected and will not be parsed: "
+ << path;
+ }
+ }
}
- return ParseConfig({"/system/etc/ueventd.rc"});
+ return ParseConfig(canonical);
}
int ueventd_main(int argc, char** argv) {