From 7418252a4e82e580439c04cedd3f480163a78a8e Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Fri, 11 Jun 2021 12:58:53 +0900 Subject: [PATCH 01/27] Completely migrate init first stage to Soong adb_debug.prop is migrated too. And ramdisk_available is added to all dependencies. Bug: 187196593 Test: boot Change-Id: I59cd149e0021211b8fd59c44b93bbf18dc8637bf Merged-In: I59cd149e0021211b8fd59c44b93bbf18dc8637bf --- fs_mgr/Android.bp | 2 + fs_mgr/libfiemap/Android.bp | 1 + fs_mgr/libfs_avb/Android.bp | 1 + fs_mgr/liblp/Android.bp | 1 + fs_mgr/libsnapshot/Android.bp | 1 + fs_mgr/libstorage_literals/Android.bp | 1 + init/Android.bp | 45 +++++++- init/Android.mk | 151 +------------------------- libcrypto_utils/Android.bp | 1 + libkeyutils/Android.bp | 1 + libmodprobe/Android.bp | 1 + rootdir/Android.bp | 9 +- rootdir/Android.mk | 11 -- 13 files changed, 62 insertions(+), 164 deletions(-) diff --git a/fs_mgr/Android.bp b/fs_mgr/Android.bp index 5356b006d..3d63a4437 100644 --- a/fs_mgr/Android.bp +++ b/fs_mgr/Android.bp @@ -141,6 +141,7 @@ cc_library { // Do not ever allow this library to be vendor_available as a shared library. // It does not have a stable interface. name: "libfs_mgr", + ramdisk_available: true, recovery_available: true, defaults: [ "libfs_mgr_defaults", @@ -165,6 +166,7 @@ cc_library_static { // It does not have a stable interface. name: "libfstab", vendor_available: true, + ramdisk_available: true, recovery_available: true, host_supported: true, defaults: ["fs_mgr_defaults"], diff --git a/fs_mgr/libfiemap/Android.bp b/fs_mgr/libfiemap/Android.bp index 1c5872e6f..b62e33fdd 100644 --- a/fs_mgr/libfiemap/Android.bp +++ b/fs_mgr/libfiemap/Android.bp @@ -20,6 +20,7 @@ package { cc_library_headers { name: "libfiemap_headers", + ramdisk_available: true, recovery_available: true, export_include_dirs: ["include"], } diff --git a/fs_mgr/libfs_avb/Android.bp b/fs_mgr/libfs_avb/Android.bp index 6892025a1..62493ebd9 100644 --- a/fs_mgr/libfs_avb/Android.bp +++ b/fs_mgr/libfs_avb/Android.bp @@ -27,6 +27,7 @@ package { cc_library_static { name: "libfs_avb", defaults: ["fs_mgr_defaults"], + ramdisk_available: true, recovery_available: true, host_supported: true, export_include_dirs: ["include"], diff --git a/fs_mgr/liblp/Android.bp b/fs_mgr/liblp/Android.bp index 7e528b164..86ca8f35f 100644 --- a/fs_mgr/liblp/Android.bp +++ b/fs_mgr/liblp/Android.bp @@ -30,6 +30,7 @@ liblp_lib_deps = [ cc_library { name: "liblp", host_supported: true, + ramdisk_available: true, recovery_available: true, defaults: ["fs_mgr_defaults"], cppflags: [ diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index 6a764e4fa..aa1f4154c 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -118,6 +118,7 @@ cc_library_static { native_coverage : true, defaults: ["libsnapshot_defaults"], srcs: [":libsnapshot_sources"], + ramdisk_available: true, recovery_available: true, cflags: [ "-DLIBSNAPSHOT_NO_COW_WRITE", diff --git a/fs_mgr/libstorage_literals/Android.bp b/fs_mgr/libstorage_literals/Android.bp index 5b0716851..fd7ea0473 100644 --- a/fs_mgr/libstorage_literals/Android.bp +++ b/fs_mgr/libstorage_literals/Android.bp @@ -6,6 +6,7 @@ package { cc_library_headers { name: "libstorage_literals_headers", host_supported: true, + ramdisk_available: true, recovery_available: true, export_include_dirs: ["."], target: { diff --git a/init/Android.bp b/init/Android.bp index 7eeafa24b..d0b58caf7 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -253,11 +253,32 @@ cc_binary { visibility: ["//packages/modules/Virtualization/microdroid"], } -// This currently is only for the VM usecase. -// TODO(jiyong): replace init_first_stage in Android.mk with this +soong_config_module_type { + name: "init_first_stage_cc_defaults", + module_type: "cc_defaults", + config_namespace: "ANDROID", + bool_variables: ["BOARD_BUILD_SYSTEM_ROOT_IMAGE", "BOARD_USES_RECOVERY_AS_BOOT"], + properties: ["installable"], +} + +// Do not install init_first_stage even with mma if we're system-as-root. +// Otherwise, it will overwrite the symlink. +init_first_stage_cc_defaults { + name: "init_first_stage_defaults", + soong_config_variables: { + BOARD_BUILD_SYSTEM_ROOT_IMAGE: { + installable: false, + }, + BOARD_USES_RECOVERY_AS_BOOT: { + installable: false, + }, + }, +} + cc_binary { - name: "init_first_stage_soong", - stem: "init_vendor", + name: "init_first_stage", + stem: "init", + defaults: ["init_first_stage_defaults"], srcs: [ "block_dev_initializer.cpp", @@ -313,6 +334,7 @@ cc_binary { ], static_executable: true, + system_shared_libs: [], cflags: [ "-Wall", @@ -363,8 +385,23 @@ cc_binary { sanitize: { misc_undefined: ["signed-integer-overflow"], + + // First stage init is weird: it may start without stdout/stderr, and no /proc. hwaddress: false, }, + + // Install adb_debug.prop into debug ramdisk. + // This allows adb root on a user build, when debug ramdisk is used. + required: ["adb_debug.prop"], + + ramdisk: true, + + install_in_root: true, +} + +phony { + name: "init_system", + required: ["init_second_stage"], } // Tests diff --git a/init/Android.mk b/init/Android.mk index 3c7d95acf..c08fe0393 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -2,153 +2,6 @@ LOCAL_PATH:= $(call my-dir) --include system/sepolicy/policy_version.mk - -# -- - -ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT))) -init_options += \ - -DALLOW_FIRST_STAGE_CONSOLE=1 \ - -DALLOW_LOCAL_PROP_OVERRIDE=1 \ - -DALLOW_PERMISSIVE_SELINUX=1 \ - -DREBOOT_BOOTLOADER_ON_PANIC=1 \ - -DWORLD_WRITABLE_KMSG=1 \ - -DDUMP_ON_UMOUNT_FAILURE=1 -else -init_options += \ - -DALLOW_FIRST_STAGE_CONSOLE=0 \ - -DALLOW_LOCAL_PROP_OVERRIDE=0 \ - -DALLOW_PERMISSIVE_SELINUX=0 \ - -DREBOOT_BOOTLOADER_ON_PANIC=0 \ - -DWORLD_WRITABLE_KMSG=0 \ - -DDUMP_ON_UMOUNT_FAILURE=0 -endif - -ifneq (,$(filter eng,$(TARGET_BUILD_VARIANT))) -init_options += \ - -DSHUTDOWN_ZERO_TIMEOUT=1 -else -init_options += \ - -DSHUTDOWN_ZERO_TIMEOUT=0 -endif - -init_options += -DLOG_UEVENTS=0 \ - -DSEPOLICY_VERSION=$(POLICYVERS) - -init_cflags += \ - $(init_options) \ - -Wall -Wextra \ - -Wno-unused-parameter \ - -Werror \ - -# -- - -# Do not build this even with mmma if we're system-as-root, otherwise it will overwrite the symlink. -ifneq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true) -include $(CLEAR_VARS) -LOCAL_CPPFLAGS := $(init_cflags) -LOCAL_SRC_FILES := \ - block_dev_initializer.cpp \ - devices.cpp \ - first_stage_console.cpp \ - first_stage_init.cpp \ - first_stage_main.cpp \ - first_stage_mount.cpp \ - reboot_utils.cpp \ - selabel.cpp \ - selinux.cpp \ - service_utils.cpp \ - snapuserd_transition.cpp \ - switch_root.cpp \ - uevent_listener.cpp \ - util.cpp \ - -LOCAL_MODULE := init_first_stage -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE -LOCAL_MODULE_STEM := init - -LOCAL_FORCE_STATIC_EXECUTABLE := true - -LOCAL_MODULE_PATH := $(TARGET_RAMDISK_OUT) -LOCAL_UNSTRIPPED_PATH := $(TARGET_RAMDISK_OUT_UNSTRIPPED) - -# Install adb_debug.prop into debug ramdisk. -# This allows adb root on a user build, when debug ramdisk is used. -LOCAL_REQUIRED_MODULES := \ - adb_debug.prop \ - -# Set up the directories that first stage init mounts on. - -my_ramdisk_dirs := \ - debug_ramdisk \ - dev \ - metadata \ - mnt \ - proc \ - second_stage_resources \ - sys \ - -LOCAL_POST_INSTALL_CMD := mkdir -p $(addprefix $(TARGET_RAMDISK_OUT)/,$(my_ramdisk_dirs)) -ifeq (true,$(BOARD_USES_GENERIC_KERNEL_IMAGE)) - LOCAL_POST_INSTALL_CMD += $(addprefix $(TARGET_RAMDISK_OUT)/first_stage_ramdisk/,$(my_ramdisk_dirs)) -endif - -my_ramdisk_dirs := - -LOCAL_STATIC_LIBRARIES := \ - libc++fs \ - libfs_avb \ - libfs_mgr \ - libfec \ - libfec_rs \ - libsquashfs_utils \ - liblogwrap \ - libext4_utils \ - libcrypto_utils \ - libsparse \ - libavb \ - libkeyutils \ - liblp \ - libcutils \ - libbase \ - liblog \ - libcrypto_static \ - libdl \ - libz \ - libselinux \ - libcap \ - libgsi \ - libcom.android.sysprop.apex \ - liblzma \ - libunwindstack_no_dex \ - libbacktrace_no_dex \ - libmodprobe \ - libext2_uuid \ - libprotobuf-cpp-lite \ - libsnapshot_cow \ - libsnapshot_init \ - update_metadata-protos \ - libprocinfo \ - -LOCAL_SANITIZE := signed-integer-overflow -# First stage init is weird: it may start without stdout/stderr, and no /proc. -LOCAL_NOSANITIZE := hwaddress -include $(BUILD_EXECUTABLE) -endif - -include $(CLEAR_VARS) - -LOCAL_MODULE := init_system -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE -LOCAL_REQUIRED_MODULES := \ - init_second_stage \ - -include $(BUILD_PHONY_PACKAGE) - include $(CLEAR_VARS) LOCAL_MODULE := init_vendor @@ -156,8 +9,10 @@ LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 LOCAL_LICENSE_CONDITIONS := notice LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE ifneq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true) +ifneq ($(BOARD_USES_RECOVERY_AS_BOOT),true) LOCAL_REQUIRED_MODULES := \ init_first_stage \ -endif +endif # BOARD_USES_RECOVERY_AS_BOOT +endif # BOARD_BUILD_SYSTEM_ROOT_IMAGE include $(BUILD_PHONY_PACKAGE) diff --git a/libcrypto_utils/Android.bp b/libcrypto_utils/Android.bp index b33d46d50..c8a183bf1 100644 --- a/libcrypto_utils/Android.bp +++ b/libcrypto_utils/Android.bp @@ -21,6 +21,7 @@ package { cc_library { name: "libcrypto_utils", vendor_available: true, + ramdisk_available: true, recovery_available: true, vndk: { enabled: true, diff --git a/libkeyutils/Android.bp b/libkeyutils/Android.bp index 86f68fb57..a940b8cec 100644 --- a/libkeyutils/Android.bp +++ b/libkeyutils/Android.bp @@ -15,6 +15,7 @@ cc_library { name: "libkeyutils", cflags: ["-Werror"], defaults: ["linux_bionic_supported"], + ramdisk_available: true, recovery_available: true, export_include_dirs: ["include/"], local_include_dirs: ["include/"], diff --git a/libmodprobe/Android.bp b/libmodprobe/Android.bp index ba11dc920..525a88063 100644 --- a/libmodprobe/Android.bp +++ b/libmodprobe/Android.bp @@ -8,6 +8,7 @@ cc_library_static { "-Werror", ], vendor_available: true, + ramdisk_available: true, recovery_available: true, srcs: [ "libmodprobe.cpp", diff --git a/rootdir/Android.bp b/rootdir/Android.bp index ae21633da..e98733ada 100644 --- a/rootdir/Android.bp +++ b/rootdir/Android.bp @@ -45,4 +45,11 @@ prebuilt_etc { src: "etc/public.libraries.android.txt", filename: "public.libraries.txt", installable: false, -} \ No newline at end of file +} + +// adb_debug.prop in debug ramdisk +prebuilt_root { + name: "adb_debug.prop", + src: "adb_debug.prop", + debug_ramdisk: true, +} diff --git a/rootdir/Android.mk b/rootdir/Android.mk index 99d8f9a83..9b80575ef 100644 --- a/rootdir/Android.mk +++ b/rootdir/Android.mk @@ -210,15 +210,4 @@ $(LOCAL_BUILT_MODULE): $(hide) $(foreach lib,$(PRIVATE_SANITIZER_RUNTIME_LIBRARIES), \ echo $(lib) >> $@;) -####################################### -# adb_debug.prop in debug ramdisk -include $(CLEAR_VARS) -LOCAL_MODULE := adb_debug.prop -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_SRC_FILES := $(LOCAL_MODULE) -LOCAL_MODULE_CLASS := ETC -LOCAL_MODULE_PATH := $(TARGET_DEBUG_RAMDISK_OUT) -include $(BUILD_PREBUILT) - include $(call all-makefiles-under,$(LOCAL_PATH)) From 2d93a2a100abfec22b6bf8a477e0e1a8f466f731 Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Fri, 18 Jun 2021 14:47:25 -0700 Subject: [PATCH 02/27] reboot_utils: Check bootconfig for reboot parameters Androidboot parameters have moved from /proc/cmdline to /proc/bootconfig so we need to check both places in reboot_utils. "ro.boot.*" properties can not be used because this is initialized before the properties are set. Test: boot Cuttlefish with init_fatal_panic and init_fatal_reboot_target in bootconfig and in cmdline Bug: 191494101 Merged-In: I6c230496ec1c3632470d20ff4a31f28db96ea71b Change-Id: I6c230496ec1c3632470d20ff4a31f28db96ea71b --- init/reboot_utils.cpp | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/init/reboot_utils.cpp b/init/reboot_utils.cpp index 98f6857f5..b3fa9fd3b 100644 --- a/init/reboot_utils.cpp +++ b/init/reboot_utils.cpp @@ -31,6 +31,7 @@ #include "capabilities.h" #include "reboot_utils.h" +#include "util.h" namespace android { namespace init { @@ -38,31 +39,51 @@ namespace init { static std::string init_fatal_reboot_target = "bootloader"; static bool init_fatal_panic = false; +// this needs to read the /proc/* files directly because it is called before +// ro.boot.* properties are initialized void SetFatalRebootTarget(const std::optional& reboot_target) { std::string cmdline; android::base::ReadFileToString("/proc/cmdline", &cmdline); cmdline = android::base::Trim(cmdline); - const char kInitFatalPanicString[] = "androidboot.init_fatal_panic=true"; - init_fatal_panic = cmdline.find(kInitFatalPanicString) != std::string::npos; + 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; + } + }); + } else { + const std::string kInitFatalPanicString = kInitFatalPanicParamString + "=true"; + init_fatal_panic = cmdline.find(kInitFatalPanicString) != std::string::npos; + } if (reboot_target) { init_fatal_reboot_target = *reboot_target; return; } - const char kRebootTargetString[] = "androidboot.init_fatal_reboot_target="; + const std::string kRebootTargetString = "androidboot.init_fatal_reboot_target"; auto start_pos = cmdline.find(kRebootTargetString); if (start_pos == std::string::npos) { - return; // We already default to bootloader if no setting is provided. - } - start_pos += sizeof(kRebootTargetString) - 1; + ImportBootconfig([kRebootTargetString](const std::string& key, const std::string& value) { + if (key == kRebootTargetString) { + init_fatal_reboot_target = value; + } + }); + // We already default to bootloader if no setting is provided. + } else { + const std::string kRebootTargetStringPattern = kRebootTargetString + "="; + start_pos += sizeof(kRebootTargetStringPattern) - 1; - auto end_pos = cmdline.find(' ', start_pos); - // if end_pos isn't found, then we've run off the end, but this is okay as this is the last - // entry, and -1 is a valid size for string::substr(); - auto size = end_pos == std::string::npos ? -1 : end_pos - start_pos; - init_fatal_reboot_target = cmdline.substr(start_pos, size); + auto end_pos = cmdline.find(' ', start_pos); + // if end_pos isn't found, then we've run off the end, but this is okay as this is the last + // entry, and -1 is a valid size for string::substr(); + auto size = end_pos == std::string::npos ? -1 : end_pos - start_pos; + init_fatal_reboot_target = cmdline.substr(start_pos, size); + } } bool IsRebootCapable() { From 91b351ea7be56ff8a52e8344a871604288db9bbc Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 15 Jun 2021 17:09:00 -0700 Subject: [PATCH 03/27] Perform a consistency check before deleting snapshots. If for some reason the COW state is not fully synced to disk, but dm-snapshot has flushed its pending merges, we do not want to delete snapshots. Doing so could potentially leave blocks unmerged. This situation is quite unexpected so we label it as a merge failure. The device can recover by completely syncing the COW state, and then rebooting, which will attempt to make forward progress on the merge. Bug: 190582627 Test: vts_libsnapshot_test full OTA on bramble incremental OTA on bramble Change-Id: Ib887f1d9e4397a712ed2f800cc1222cf9305a039 Merged-In: Ib887f1d9e4397a712ed2f800cc1222cf9305a039 --- .../android/snapshot/snapshot.proto | 7 ++ .../include/libsnapshot/cow_reader.h | 5 +- .../include/libsnapshot/snapshot.h | 2 + fs_mgr/libsnapshot/snapshot.cpp | 99 +++++++++++++++++-- 4 files changed, 102 insertions(+), 11 deletions(-) diff --git a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto index 92aa55c07..9f227c970 100644 --- a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto +++ b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto @@ -158,6 +158,13 @@ enum MergeFailureCode { ExpectedMergeTarget = 11; UnmergedSectorsAfterCompletion = 12; UnexpectedMergeState = 13; + GetCowPathConsistencyCheck = 14; + OpenCowConsistencyCheck = 15; + ParseCowConsistencyCheck = 16; + OpenCowDirectConsistencyCheck = 17; + MemAlignConsistencyCheck = 18; + DirectReadConsistencyCheck = 19; + WrongMergeCountConsistencyCheck = 20; }; // Next: 8 diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h index 9ebcfd983..669e58ac6 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_reader.h @@ -143,12 +143,11 @@ class CowReader : public ICowReader { void InitializeMerge(); + // Number of copy, replace, and zero ops. Set if InitializeMerge is called. void set_total_data_ops(uint64_t size) { total_data_ops_ = size; } - uint64_t total_data_ops() { return total_data_ops_; } - + // Number of copy ops. Set if InitializeMerge is called. void set_copy_ops(uint64_t size) { copy_ops_ = size; } - uint64_t total_copy_ops() { return copy_ops_; } void CloseCowFd() { owned_fd_ = {}; } diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 603e89694..65034f71e 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -603,6 +603,8 @@ class SnapshotManager final : public ISnapshotManager { MergeResult CheckMergeState(LockedFile* lock, const std::function& before_cancel); MergeResult CheckTargetMergeState(LockedFile* lock, const std::string& name, const SnapshotUpdateStatus& update_status); + MergeFailureCode CheckMergeConsistency(LockedFile* lock, const std::string& name, + const SnapshotStatus& update_status); // Interact with status files under /metadata/ota/snapshots. bool WriteSnapshotStatus(LockedFile* lock, const SnapshotStatus& status); diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index e2c03aedd..be732ece9 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1126,6 +1126,11 @@ auto SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::string& return MergeResult(UpdateState::Merging); } + auto code = CheckMergeConsistency(lock, name, snapshot_status); + if (code != MergeFailureCode::Ok) { + return MergeResult(UpdateState::MergeFailed, code); + } + // Merging is done. First, update the status file to indicate the merge // is complete. We do this before calling OnSnapshotMergeComplete, even // though this means the write is potentially wasted work (since in the @@ -1144,6 +1149,91 @@ auto SnapshotManager::CheckTargetMergeState(LockedFile* lock, const std::string& return MergeResult(UpdateState::MergeCompleted, MergeFailureCode::Ok); } +// This returns the backing device, not the dm-user layer. +static std::string GetMappedCowDeviceName(const std::string& snapshot, + const SnapshotStatus& status) { + // If no partition was created (the COW exists entirely on /data), the + // device-mapper layering is different than if we had a partition. + if (status.cow_partition_size() == 0) { + return GetCowImageDeviceName(snapshot); + } + return GetCowName(snapshot); +} + +MergeFailureCode SnapshotManager::CheckMergeConsistency(LockedFile* lock, const std::string& name, + const SnapshotStatus& status) { + CHECK(lock); + + if (!status.compression_enabled()) { + // Do not try to verify old-style COWs yet. + return MergeFailureCode::Ok; + } + + auto& dm = DeviceMapper::Instance(); + + std::string cow_image_name = GetMappedCowDeviceName(name, status); + std::string cow_image_path; + if (!dm.GetDmDevicePathByName(cow_image_name, &cow_image_path)) { + LOG(ERROR) << "Failed to get path for cow device: " << cow_image_name; + return MergeFailureCode::GetCowPathConsistencyCheck; + } + + // First pass, count # of ops. + size_t num_ops = 0; + { + unique_fd fd(open(cow_image_path.c_str(), O_RDONLY | O_CLOEXEC)); + if (fd < 0) { + PLOG(ERROR) << "Failed to open " << cow_image_name; + return MergeFailureCode::OpenCowConsistencyCheck; + } + + CowReader reader; + if (!reader.Parse(std::move(fd))) { + LOG(ERROR) << "Failed to parse cow " << cow_image_path; + return MergeFailureCode::ParseCowConsistencyCheck; + } + + for (auto iter = reader.GetOpIter(); !iter->Done(); iter->Next()) { + if (!IsMetadataOp(iter->Get())) { + num_ops++; + } + } + } + + // Second pass, try as hard as we can to get the actual number of blocks + // the system thinks is merged. + unique_fd fd(open(cow_image_path.c_str(), O_RDONLY | O_DIRECT | O_SYNC | O_CLOEXEC)); + if (fd < 0) { + PLOG(ERROR) << "Failed to open direct " << cow_image_name; + return MergeFailureCode::OpenCowDirectConsistencyCheck; + } + + void* addr; + size_t page_size = getpagesize(); + if (posix_memalign(&addr, page_size, page_size) < 0) { + PLOG(ERROR) << "posix_memalign with page size " << page_size; + return MergeFailureCode::MemAlignConsistencyCheck; + } + + // COWs are always at least 2MB, this is guaranteed in snapshot creation. + std::unique_ptr buffer(addr, ::free); + if (!android::base::ReadFully(fd, buffer.get(), page_size)) { + PLOG(ERROR) << "Direct read failed " << cow_image_name; + return MergeFailureCode::DirectReadConsistencyCheck; + } + + auto header = reinterpret_cast(buffer.get()); + if (header->num_merge_ops != num_ops) { + LOG(ERROR) << "COW consistency check failed, expected " << num_ops << " to be merged, " + << "but " << header->num_merge_ops << " were actually recorded."; + LOG(ERROR) << "Aborting merge progress for snapshot " << name + << ", will try again next boot"; + return MergeFailureCode::WrongMergeCountConsistencyCheck; + } + + return MergeFailureCode::Ok; +} + MergeFailureCode SnapshotManager::MergeSecondPhaseSnapshots(LockedFile* lock) { std::vector snapshots; if (!ListSnapshots(lock, &snapshots)) { @@ -1429,14 +1519,7 @@ bool SnapshotManager::PerformInitTransition(InitTransition transition, continue; } - // If no partition was created (the COW exists entirely on /data), the - // device-mapper layering is different than if we had a partition. - std::string cow_image_name; - if (snapshot_status.cow_partition_size() == 0) { - cow_image_name = GetCowImageDeviceName(snapshot); - } else { - cow_image_name = GetCowName(snapshot); - } + std::string cow_image_name = GetMappedCowDeviceName(snapshot, snapshot_status); std::string cow_image_device; if (!dm.GetDmDevicePathByName(cow_image_name, &cow_image_device)) { From 72d0f9b6e7942e1366f0388c97864df864a82397 Mon Sep 17 00:00:00 2001 From: yidong zhang Date: Thu, 24 Jun 2021 17:51:56 +0800 Subject: [PATCH 04/27] Avoid using thread cache in unwinder. Using thread cache will cause SIGSEGV for 32bit+kernel4.9 device. Bug: 190579082 Bug: 189803009 Test: run cts -m CtsSeccompHostTestCases Change-Id: I47b13d02674aadbacd8dac36d8382eed0885413c Merged-In: I47b13d02674aadbacd8dac36d8382eed0885413c Signed-off-by: yidong zhang (cherry picked from commit cbf7c466e65eba2b285b28292a5e4934b6655c8a) --- debuggerd/libdebuggerd/tombstone.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp index ad903cee4..9c01f15eb 100644 --- a/debuggerd/libdebuggerd/tombstone.cpp +++ b/debuggerd/libdebuggerd/tombstone.cpp @@ -593,6 +593,9 @@ void engrave_tombstone_ucontext(int tombstone_fd, int proto_fd, uint64_t abort_m }; unwindstack::UnwinderFromPid unwinder(kMaxFrames, pid, unwindstack::Regs::CurrentArch()); + auto process_memory = + unwindstack::Memory::CreateProcessMemoryCached(getpid()); + unwinder.SetProcessMemory(process_memory); if (!unwinder.Init()) { async_safe_fatal("failed to init unwinder object"); } From b4e79853cd89afcd37da7410c5f279f4f547c92f Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 24 Jun 2021 14:10:15 -0700 Subject: [PATCH 05/27] init.rc: remove system cgroup migraion We never use CONFIG_RT_GROUP_SCHED in GKI kernel, but that could be set on legacy devices. Remove system cgroup migration and also RT settings as we should not have any task under those groups. Bug: 191925901 Test: Build Signed-off-by: Wei Wang Change-Id: I492833975e28e9888e412711e80670ca0901010d --- rootdir/init.rc | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/rootdir/init.rc b/rootdir/init.rc index 6e85da58b..8da019979 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -191,33 +191,6 @@ on init chown system system /dev/cpuctl/camera-daemon/tasks chmod 0664 /dev/cpuctl/camera-daemon/tasks - # Android only use global RT throttling and doesn't use CONFIG_RT_GROUP_SCHED - # for RT group throttling. These values here are just to make sure RT threads - # can be migrated to those groups. These settings can be removed once we migrate - # to GKI kernel. - write /dev/cpuctl/cpu.rt_period_us 1000000 - write /dev/cpuctl/cpu.rt_runtime_us 950000 - # Surfaceflinger is in FG group so giving it a bit more - write /dev/cpuctl/foreground/cpu.rt_runtime_us 450000 - write /dev/cpuctl/foreground/cpu.rt_period_us 1000000 - write /dev/cpuctl/background/cpu.rt_runtime_us 50000 - write /dev/cpuctl/background/cpu.rt_period_us 1000000 - write /dev/cpuctl/top-app/cpu.rt_runtime_us 100000 - write /dev/cpuctl/top-app/cpu.rt_period_us 1000000 - write /dev/cpuctl/rt/cpu.rt_runtime_us 100000 - write /dev/cpuctl/rt/cpu.rt_period_us 1000000 - write /dev/cpuctl/system/cpu.rt_runtime_us 100000 - write /dev/cpuctl/system/cpu.rt_period_us 1000000 - write /dev/cpuctl/system-background/cpu.rt_runtime_us 50000 - write /dev/cpuctl/system-background/cpu.rt_period_us 1000000 - write /dev/cpuctl/nnapi-hal/cpu.rt_runtime_us 50000 - write /dev/cpuctl/nnapi-hal/cpu.rt_period_us 1000000 - write /dev/cpuctl/camera-daemon/cpu.rt_runtime_us 50000 - write /dev/cpuctl/camera-daemon/cpu.rt_period_us 1000000 - - # Migrate root group to system subgroup - copy_per_line /dev/cpuctl/tasks /dev/cpuctl/system/tasks - # Create an stune group for camera-specific processes mkdir /dev/stune/camera-daemon chown system system /dev/stune/camera-daemon @@ -1275,7 +1248,3 @@ on userspace-reboot-resume on property:sys.boot_completed=1 && property:sys.init.userspace_reboot.in_progress=1 setprop sys.init.userspace_reboot.in_progress "" - -# Migrate tasks again in case kernel threads are created during boot -on property:sys.boot_completed=1 - copy_per_line /dev/cpuctl/tasks /dev/cpuctl/system/tasks From 68bb5c41952d5f379019cdbc71d6d9ae55c0de4a Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Fri, 18 Jun 2021 09:43:56 -0700 Subject: [PATCH 06/27] Revert "Handle "hardware" bootconfig parameter as "androidboot.hardware"" This reverts commit 0a799bdfd607b729e94796e414d406839d6ad6ad. Now that the kernel bootconfig feature has been to updated to handle mixed subkeys and values, androidboot.hardware parameter is supported. Test: build and boot Cuttlefish with "androidboot.hardware=cutf_vm" Bug: 191502832 Merged-In: I0e436a27730d20689bc6974562c3e88d744385db Change-Id: I0e436a27730d20689bc6974562c3e88d744385db --- fs_mgr/fs_mgr_boot_config.cpp | 6 ------ fs_mgr/tests/fs_mgr_test.cpp | 4 ++-- init/property_service.cpp | 10 ---------- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/fs_mgr/fs_mgr_boot_config.cpp b/fs_mgr/fs_mgr_boot_config.cpp index e3ef2321a..75d1e0db6 100644 --- a/fs_mgr/fs_mgr_boot_config.cpp +++ b/fs_mgr/fs_mgr_boot_config.cpp @@ -91,12 +91,6 @@ bool fs_mgr_get_boot_config_from_bootconfig(const std::string& bootconfig, if (key == bootconfig_key) { *out_val = value; return true; - } else if (android_key == "hardware" && android_key == key) { - // bootconfig doesn't allow subkeys and values to coexist, so - // "androidboot.hardware" cannot be used. It is replaced in - // bootconfig with "hardware" - *out_val = value; - return true; } } diff --git a/fs_mgr/tests/fs_mgr_test.cpp b/fs_mgr/tests/fs_mgr_test.cpp index eb2919b01..953574b66 100644 --- a/fs_mgr/tests/fs_mgr_test.cpp +++ b/fs_mgr/tests/fs_mgr_test.cpp @@ -127,7 +127,7 @@ const std::string bootconfig = "androidboot.serialno = \"BLAHBLAHBLAH\"\n" "androidboot.slot_suffix = \"_a\"\n" "androidboot.hardware.platform = \"sdw813\"\n" - "hardware = \"foo\"\n" + "androidboot.hardware = \"foo\"\n" "androidboot.revision = \"EVT1.0\"\n" "androidboot.bootloader = \"burp-0.1-7521\"\n" "androidboot.hardware.sku = \"mary\"\n" @@ -159,7 +159,7 @@ const std::vector> bootconfig_result_space = {"androidboot.serialno", "BLAHBLAHBLAH"}, {"androidboot.slot_suffix", "_a"}, {"androidboot.hardware.platform", "sdw813"}, - {"hardware", "foo"}, + {"androidboot.hardware", "foo"}, {"androidboot.revision", "EVT1.0"}, {"androidboot.bootloader", "burp-0.1-7521"}, {"androidboot.hardware.sku", "mary"}, diff --git a/init/property_service.cpp b/init/property_service.cpp index ff9da4227..2d67bf5d7 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -1238,21 +1238,11 @@ static void ProcessKernelCmdline() { }); } -// bootconfig does not allow to populate `key=value` simultaneously with -// `key.subkey=value` which does not work with the existing code for -// `hardware` (e.g. we want both `ro.boot.hardware=value` and -// `ro.boot.hardware.sku=value`) and for `qemu` (Android Stidio Emulator -// specific). -static bool IsAllowedBootconfigKey(const std::string_view key) { - return (key == "hardware"sv) || (key == "qemu"sv); -} static void ProcessBootconfig() { ImportBootconfig([&](const std::string& key, const std::string& value) { if (StartsWith(key, ANDROIDBOOT_PREFIX)) { InitPropertySet("ro.boot." + key.substr(ANDROIDBOOT_PREFIX.size()), value); - } else if (IsAllowedBootconfigKey(key)) { - InitPropertySet("ro.boot." + key, value); } }); } From 729e08f6eac3dda94481ade1c5b4ef56def59a90 Mon Sep 17 00:00:00 2001 From: Mitch Phillips Date: Mon, 19 Apr 2021 09:59:17 -0700 Subject: [PATCH 07/27] [MTE] Add a HWASan-style tag dump to tombstones. We already dump the tags in the regigster dump section by appending the tag to the memory address. You only get 2 granules before each register and 13 after. The HWASan-style tag dump is extremely useful for debugging, as it gives a pretty comprehensive overview of the memory subsystem. It also provides enough context bytes (256) to give you a reasonable intuition about a particular bug. The tag dump shows up only if PTRACE_PEEKTAGS returns at least one value in the 256 requested. If the start of end of the region is untagged, it's omitted. The tag dump looks like this: Change-Id: Icc33fb97542d9b1fa3ae9e58aba34d524c6ba7b5 --- Memory tags around the fault address (0x60000704414d340), one tag per 16 bytes: 0x704414d000: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d100: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d200: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 =>0x704414d300: 0 0 0 0 [2] 2 0 0 0 0 0 0 0 0 0 0 0x704414d400: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d500: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d600: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d700: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d800: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414d900: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0x704414da00: 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 --- Bug: 183992164 Test: atest debuggerd_test on MTE+QEMU and sunfish. Change-Id: I8d5842e4803ca30b407e866c99eef56f2cb36600 Merged-In: I8d5842e4803ca30b407e866c99eef56f2cb36600 --- debuggerd/debuggerd_test.cpp | 139 ++++++++++++++++-- .../include/libdebuggerd/utility.h | 4 + debuggerd/libdebuggerd/tombstone_proto.cpp | 68 ++++++++- .../libdebuggerd/tombstone_proto_to_text.cpp | 64 +++++++- debuggerd/proto/tombstone.proto | 17 ++- 5 files changed, 273 insertions(+), 19 deletions(-) diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index 24804d046..abda0713a 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -58,6 +58,7 @@ #include #include "debuggerd/handler.h" +#include "libdebuggerd/utility.h" #include "protocol.h" #include "tombstoned/tombstoned.h" #include "util.h" @@ -526,6 +527,8 @@ TEST_P(SizeParamCrasherTest, mte_uaf) { std::vector log_sources(2); ConsumeFd(std::move(output_fd), &log_sources[0]); logcat_collector.Collect(&log_sources[1]); + // Tag dump only available in the tombstone, not logcat. + ASSERT_MATCH(log_sources[0], "Memory tags around the fault address"); for (const auto& result : log_sources) { ASSERT_MATCH(result, R"(signal 11 \(SIGSEGV\))"); @@ -597,6 +600,12 @@ TEST_P(SizeParamCrasherTest, mte_overflow) { ConsumeFd(std::move(output_fd), &log_sources[0]); logcat_collector.Collect(&log_sources[1]); + // Tag dump only in tombstone, not logcat, and tagging is not used for + // overflow protection in the scudo secondary (guard pages are used instead). + if (GetParam() < 0x10000) { + ASSERT_MATCH(log_sources[0], "Memory tags around the fault address"); + } + 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 )" + @@ -637,6 +646,7 @@ TEST_P(SizeParamCrasherTest, mte_underflow) { std::to_string(GetParam()) + R"(-byte allocation)"); ASSERT_MATCH(result, R"((^|\s)allocated by thread .* #00 pc)"); + ASSERT_MATCH(result, "Memory tags around the fault address"); #else GTEST_SKIP() << "Requires aarch64"; #endif @@ -686,6 +696,9 @@ TEST_F(CrasherTest, mte_multiple_causes) { ConsumeFd(std::move(output_fd), &log_sources[0]); logcat_collector.Collect(&log_sources[1]); + // Tag dump only in the tombstone, not logcat. + ASSERT_MATCH(log_sources[0], "Memory tags around the fault address"); + 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, " @@ -706,21 +719,26 @@ TEST_F(CrasherTest, mte_multiple_causes) { #if defined(__aarch64__) static uintptr_t CreateTagMapping() { - uintptr_t mapping = - reinterpret_cast(mmap(nullptr, getpagesize(), PROT_READ | PROT_WRITE | PROT_MTE, - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); - if (reinterpret_cast(mapping) == MAP_FAILED) { + // Some of the MTE tag dump tests assert that there is an inaccessible page to the left and right + // of the PROT_MTE page, so map three pages and set the two guard pages to PROT_NONE. + size_t page_size = getpagesize(); + void* mapping = mmap(nullptr, page_size * 3, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + uintptr_t mapping_uptr = reinterpret_cast(mapping); + if (mapping == MAP_FAILED) { return 0; } - __asm__ __volatile__(".arch_extension mte; stg %0, [%0]" - : - : "r"(mapping + (1ULL << 56)) - : "memory"); - return mapping; + mprotect(reinterpret_cast(mapping_uptr + page_size), page_size, + PROT_READ | PROT_WRITE | PROT_MTE); + // Stripe the mapping, where even granules get tag '1', and odd granules get tag '0'. + for (uintptr_t offset = 0; offset < page_size; offset += 2 * kTagGranuleSize) { + uintptr_t tagged_addr = mapping_uptr + page_size + offset + (1ULL << 56); + __asm__ __volatile__(".arch_extension mte; stg %0, [%0]" : : "r"(tagged_addr) : "memory"); + } + return mapping_uptr + page_size; } #endif -TEST_F(CrasherTest, mte_tag_dump) { +TEST_F(CrasherTest, mte_register_tag_dump) { #if defined(__aarch64__) if (!mte_supported()) { GTEST_SKIP() << "Requires MTE"; @@ -753,6 +771,107 @@ TEST_F(CrasherTest, mte_tag_dump) { #endif } +TEST_F(CrasherTest, mte_fault_tag_dump_front_truncated) { +#if defined(__aarch64__) + if (!mte_supported()) { + GTEST_SKIP() << "Requires MTE"; + } + + int intercept_result; + unique_fd output_fd; + StartProcess([&]() { + SetTagCheckingLevelSync(); + volatile char* p = reinterpret_cast(CreateTagMapping()); + p[0] = 0; // Untagged pointer, tagged memory. + }); + + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGSEGV); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + + ASSERT_MATCH(result, R"(Memory tags around the fault address.* +\s*=>0x[0-9a-f]+000:\[1\] 0 1 0)"); +#else + GTEST_SKIP() << "Requires aarch64"; +#endif +} + +TEST_F(CrasherTest, mte_fault_tag_dump) { +#if defined(__aarch64__) + if (!mte_supported()) { + GTEST_SKIP() << "Requires MTE"; + } + + int intercept_result; + unique_fd output_fd; + StartProcess([&]() { + SetTagCheckingLevelSync(); + volatile char* p = reinterpret_cast(CreateTagMapping()); + p[320] = 0; // Untagged pointer, tagged memory. + }); + + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGSEGV); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + + ASSERT_MATCH(result, R"(Memory tags around the fault address.* +\s*0x[0-9a-f]+: 1 0 1 0 1 0 1 0 1 0 1 0 1 0 1 0 +\s*=>0x[0-9a-f]+: 1 0 1 0 \[1\] 0 1 0 1 0 1 0 1 0 1 0 +\s*0x[0-9a-f]+: 1 0 1 0 1 0 1 0 1 0 1 0 1 0 1 0 +)"); +#else + GTEST_SKIP() << "Requires aarch64"; +#endif +} + +TEST_F(CrasherTest, mte_fault_tag_dump_rear_truncated) { +#if defined(__aarch64__) + if (!mte_supported()) { + GTEST_SKIP() << "Requires MTE"; + } + + int intercept_result; + unique_fd output_fd; + StartProcess([&]() { + SetTagCheckingLevelSync(); + size_t page_size = getpagesize(); + volatile char* p = reinterpret_cast(CreateTagMapping()); + p[page_size - kTagGranuleSize * 2] = 0; // Untagged pointer, tagged memory. + }); + + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGSEGV); + FinishIntercept(&intercept_result); + + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + + ASSERT_MATCH(result, R"(Memory tags around the fault address)"); + ASSERT_MATCH(result, + R"(\s*0x[0-9a-f]+: 1 0 1 0 1 0 1 0 1 0 1 0 1 0 1 0 +\s*=>0x[0-9a-f]+: 1 0 1 0 1 0 1 0 1 0 1 0 1 0 \[1\] 0 + +)"); // Ensure truncation happened and there's a newline after the tag fault. +#else + GTEST_SKIP() << "Requires aarch64"; +#endif +} + TEST_F(CrasherTest, LD_PRELOAD) { int intercept_result; unique_fd output_fd; diff --git a/debuggerd/libdebuggerd/include/libdebuggerd/utility.h b/debuggerd/libdebuggerd/include/libdebuggerd/utility.h index c490fb1b9..24ae16949 100644 --- a/debuggerd/libdebuggerd/include/libdebuggerd/utility.h +++ b/debuggerd/libdebuggerd/include/libdebuggerd/utility.h @@ -96,4 +96,8 @@ const char* get_sigcode(const siginfo_t*); // Number of bytes per MTE granule. constexpr size_t kTagGranuleSize = 16; +// Number of rows and columns to display in an MTE tag dump. +constexpr size_t kNumTagColumns = 16; +constexpr size_t kNumTagRows = 16; + #endif // _DEBUGGERD_UTILITY_H diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp index abd1f1287..ff12017f7 100644 --- a/debuggerd/libdebuggerd/tombstone_proto.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto.cpp @@ -362,8 +362,10 @@ static void dump_thread(Tombstone* tombstone, unwindstack::Unwinder* unwinder, dump.set_mapping_name(map_info->name()); } - char buf[256]; - uint8_t tags[256 / kTagGranuleSize]; + constexpr size_t kNumBytesAroundRegister = 256; + constexpr size_t kNumTagsAroundRegister = kNumBytesAroundRegister / kTagGranuleSize; + char buf[kNumBytesAroundRegister]; + uint8_t tags[kNumTagsAroundRegister]; size_t start_offset = 0; ssize_t bytes = dump_memory(buf, sizeof(buf), tags, sizeof(tags), &value, memory); if (bytes == -1) { @@ -377,7 +379,19 @@ static void dump_thread(Tombstone* tombstone, unwindstack::Unwinder* unwinder, } dump.set_memory(buf, bytes); - dump.set_tags(tags, bytes / kTagGranuleSize); + + bool has_tags = false; +#if defined(__aarch64__) + for (size_t i = 0; i < kNumTagsAroundRegister; ++i) { + if (tags[i] != 0) { + has_tags = true; + } + } +#endif // defined(__aarch64__) + + if (has_tags) { + dump.mutable_arm_mte_metadata()->set_memory_tags(tags, kNumTagsAroundRegister); + } *thread.add_memory_dump() = std::move(dump); } @@ -531,6 +545,50 @@ static void dump_logcat(Tombstone* tombstone, pid_t pid) { dump_log_file(tombstone, "main", pid); } +static void dump_tags_around_fault_addr(Signal* signal, const Tombstone& tombstone, + unwindstack::Unwinder* unwinder, uintptr_t fault_addr) { + if (tombstone.arch() != Architecture::ARM64) return; + + fault_addr = untag_address(fault_addr); + constexpr size_t kNumGranules = kNumTagRows * kNumTagColumns; + constexpr size_t kBytesToRead = kNumGranules * kTagGranuleSize; + + // If the low part of the tag dump would underflow to the high address space, it's probably not + // a valid address for us to dump tags from. + if (fault_addr < kBytesToRead / 2) return; + + unwindstack::Memory* memory = unwinder->GetProcessMemory().get(); + + constexpr uintptr_t kRowStartMask = ~(kNumTagColumns * kTagGranuleSize - 1); + size_t start_address = (fault_addr & kRowStartMask) - kBytesToRead / 2; + MemoryDump tag_dump; + size_t granules_to_read = kNumGranules; + + // Attempt to read the first tag. If reading fails, this likely indicates the + // lowest touched page is inaccessible or not marked with PROT_MTE. + // Fast-forward over pages until one has tags, or we exhaust the search range. + while (memory->ReadTag(start_address) < 0) { + size_t page_size = sysconf(_SC_PAGE_SIZE); + size_t bytes_to_next_page = page_size - (start_address % page_size); + if (bytes_to_next_page >= granules_to_read * kTagGranuleSize) return; + start_address += bytes_to_next_page; + granules_to_read -= bytes_to_next_page / kTagGranuleSize; + } + tag_dump.set_begin_address(start_address); + + std::string* mte_tags = tag_dump.mutable_arm_mte_metadata()->mutable_memory_tags(); + + for (size_t i = 0; i < granules_to_read; ++i) { + long tag = memory->ReadTag(start_address + i * kTagGranuleSize); + if (tag < 0) break; + mte_tags->push_back(static_cast(tag)); + } + + if (!mte_tags->empty()) { + *signal->mutable_fault_adjacent_metadata() = tag_dump; + } +} + static std::optional read_uptime_secs() { std::string uptime; if (!android::base::ReadFileToString("/proc/uptime", &uptime)) { @@ -594,7 +652,9 @@ void engrave_tombstone_proto(Tombstone* tombstone, unwindstack::Unwinder* unwind if (process_info.has_fault_address) { sig.set_has_fault_address(true); - sig.set_fault_address(process_info.maybe_tagged_fault_address); + uintptr_t fault_addr = process_info.maybe_tagged_fault_address; + sig.set_fault_address(fault_addr); + dump_tags_around_fault_addr(&sig, result, unwinder, fault_addr); } *result.mutable_signal_info() = sig; diff --git a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp index a932d4831..053299a81 100644 --- a/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto_to_text.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include "tombstone.pb.h" @@ -193,8 +194,11 @@ static void print_thread_memory_dump(CallbackType callback, const Tombstone& tom uint64_t addr = mem.begin_address(); for (size_t offset = 0; offset < mem.memory().size(); offset += bytes_per_line) { uint64_t tagged_addr = addr; - if (mem.tags().size() > offset / kTagGranuleSize) { - tagged_addr |= static_cast(mem.tags()[offset / kTagGranuleSize]) << 56; + if (mem.has_arm_mte_metadata() && + mem.arm_mte_metadata().memory_tags().size() > offset / kTagGranuleSize) { + tagged_addr |= + static_cast(mem.arm_mte_metadata().memory_tags()[offset / kTagGranuleSize]) + << 56; } std::string line = StringPrintf(" %0*" PRIx64, word_size * 2, tagged_addr + offset); @@ -232,6 +236,60 @@ static void print_thread(CallbackType callback, const Tombstone& tombstone, cons print_thread_memory_dump(callback, tombstone, thread); } +static void print_tag_dump(CallbackType callback, const Tombstone& tombstone) { + if (!tombstone.has_signal_info()) return; + + const Signal& signal = tombstone.signal_info(); + + if (!signal.has_fault_address() || !signal.has_fault_adjacent_metadata()) { + return; + } + + const MemoryDump& memory_dump = signal.fault_adjacent_metadata(); + + if (!memory_dump.has_arm_mte_metadata() || memory_dump.arm_mte_metadata().memory_tags().empty()) { + return; + } + + const std::string& tags = memory_dump.arm_mte_metadata().memory_tags(); + + CBS(""); + CBS("Memory tags around the fault address (0x%" PRIx64 "), one tag per %zu bytes:", + signal.fault_address(), kTagGranuleSize); + constexpr uintptr_t kRowStartMask = ~(kNumTagColumns * kTagGranuleSize - 1); + + size_t tag_index = 0; + size_t num_tags = tags.length(); + uintptr_t fault_granule = untag_address(signal.fault_address()) & ~(kTagGranuleSize - 1); + for (size_t row = 0; tag_index < num_tags; ++row) { + uintptr_t row_addr = + (memory_dump.begin_address() + row * kNumTagColumns * kTagGranuleSize) & kRowStartMask; + std::string row_contents; + bool row_has_fault = false; + + for (size_t column = 0; column < kNumTagColumns; ++column) { + uintptr_t granule_addr = row_addr + column * kTagGranuleSize; + if (granule_addr < memory_dump.begin_address() || + granule_addr >= memory_dump.begin_address() + num_tags * kTagGranuleSize) { + row_contents += " . "; + } else if (granule_addr == fault_granule) { + row_contents += StringPrintf("[%1hhx]", tags[tag_index++]); + row_has_fault = true; + } else { + row_contents += StringPrintf(" %1hhx ", tags[tag_index++]); + } + } + + if (row_contents.back() == ' ') row_contents.pop_back(); + + if (row_has_fault) { + CBS(" =>0x%" PRIxPTR ":%s", row_addr, row_contents.c_str()); + } else { + CBS(" 0x%" PRIxPTR ":%s", row_addr, row_contents.c_str()); + } + } +} + static void print_main_thread(CallbackType callback, const Tombstone& tombstone, const Thread& thread) { print_thread_header(callback, tombstone, thread, true); @@ -299,6 +357,8 @@ static void print_main_thread(CallbackType callback, const Tombstone& tombstone, } } + print_tag_dump(callback, tombstone); + print_thread_memory_dump(callback, tombstone, thread); CBS(""); diff --git a/debuggerd/proto/tombstone.proto b/debuggerd/proto/tombstone.proto index 22fc30eb1..a701212d8 100644 --- a/debuggerd/proto/tombstone.proto +++ b/debuggerd/proto/tombstone.proto @@ -56,8 +56,11 @@ message Signal { bool has_fault_address = 8; uint64 fault_address = 9; + // Note, may or may not contain the dump of the actual memory contents. Currently, on arm64, we + // only include metadata, and not the contents. + MemoryDump fault_adjacent_metadata = 10; - reserved 10 to 999; + reserved 11 to 999; } message HeapObject { @@ -142,14 +145,22 @@ message BacktraceFrame { reserved 9 to 999; } +message ArmMTEMetadata { + // One memory tag per granule (e.g. every 16 bytes) of regular memory. + bytes memory_tags = 1; + reserved 2 to 999; +} + message MemoryDump { string register_name = 1; string mapping_name = 2; uint64 begin_address = 3; bytes memory = 4; - bytes tags = 5; + oneof metadata { + ArmMTEMetadata arm_mte_metadata = 6; + } - reserved 6 to 999; + reserved 5, 7 to 999; } message MemoryMapping { From 49b3a5c8914db6dd3d809ac110b111b07121ba1f Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Wed, 30 Jun 2021 00:20:01 +0100 Subject: [PATCH 08/27] Only run RebootTest under root This test requires running test services, which causes test to crash (and still incorrectly be reported as passing) when running on non-rooted device. Ignore-AOSP-First: reboot_test is not in AOSP yet Bug: 190958734 Test: atest CtsInitTestCases Change-Id: I3c5c9917d0a787d66272ccf4aefc57e6573841bc --- init/reboot_test.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/init/reboot_test.cpp b/init/reboot_test.cpp index 06702e3a6..b3d038d14 100644 --- a/init/reboot_test.cpp +++ b/init/reboot_test.cpp @@ -113,6 +113,11 @@ service $name /system/bin/yes } TEST_F(RebootTest, StopServicesSIGTERM) { + if (getuid() != 0) { + GTEST_SKIP() << "Skipping test, must be run as root."; + return; + } + AddTestService("A"); AddTestService("B"); @@ -148,6 +153,11 @@ TEST_F(RebootTest, StopServicesSIGTERM) { } TEST_F(RebootTest, StopServicesSIGKILL) { + if (getuid() != 0) { + GTEST_SKIP() << "Skipping test, must be run as root."; + return; + } + AddTestService("A"); AddTestService("B"); From 94c2593ea065f91783601b1858dcac430f110c68 Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Wed, 30 Jun 2021 20:24:30 +0900 Subject: [PATCH 09/27] Remove RECOVERY_AS_BOOT check for init_first_stage This has kept adb_debug.prop from being installed. Ignore-AOSP-First: fixes sc-release test breakage Bug: 192432810 Test: build ramdisk-debug.img and see contents Change-Id: I254579d2c6427213f40e9ae8e50d046e19390ba5 --- init/Android.bp | 5 +---- init/Android.mk | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/init/Android.bp b/init/Android.bp index d0b58caf7..7a4916ea2 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -257,7 +257,7 @@ soong_config_module_type { name: "init_first_stage_cc_defaults", module_type: "cc_defaults", config_namespace: "ANDROID", - bool_variables: ["BOARD_BUILD_SYSTEM_ROOT_IMAGE", "BOARD_USES_RECOVERY_AS_BOOT"], + bool_variables: ["BOARD_BUILD_SYSTEM_ROOT_IMAGE"], properties: ["installable"], } @@ -269,9 +269,6 @@ init_first_stage_cc_defaults { BOARD_BUILD_SYSTEM_ROOT_IMAGE: { installable: false, }, - BOARD_USES_RECOVERY_AS_BOOT: { - installable: false, - }, }, } diff --git a/init/Android.mk b/init/Android.mk index c08fe0393..c1b0cf9d2 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -9,10 +9,8 @@ LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 LOCAL_LICENSE_CONDITIONS := notice LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE ifneq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true) -ifneq ($(BOARD_USES_RECOVERY_AS_BOOT),true) LOCAL_REQUIRED_MODULES := \ init_first_stage \ -endif # BOARD_USES_RECOVERY_AS_BOOT endif # BOARD_BUILD_SYSTEM_ROOT_IMAGE include $(BUILD_PHONY_PACKAGE) From 9fa041c9a433b0e64e522f2b653be2def9695bb7 Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Thu, 1 Jul 2021 02:33:53 +0000 Subject: [PATCH 10/27] Revert "Remove RECOVERY_AS_BOOT check for init_first_stage" This reverts commit 94c2593ea065f91783601b1858dcac430f110c68. Reason for revert: build breakage Change-Id: I270a56bb33d19a2747298c69f6ec1b24746d97bf --- init/Android.bp | 5 ++++- init/Android.mk | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/init/Android.bp b/init/Android.bp index 7a4916ea2..d0b58caf7 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -257,7 +257,7 @@ soong_config_module_type { name: "init_first_stage_cc_defaults", module_type: "cc_defaults", config_namespace: "ANDROID", - bool_variables: ["BOARD_BUILD_SYSTEM_ROOT_IMAGE"], + bool_variables: ["BOARD_BUILD_SYSTEM_ROOT_IMAGE", "BOARD_USES_RECOVERY_AS_BOOT"], properties: ["installable"], } @@ -269,6 +269,9 @@ init_first_stage_cc_defaults { BOARD_BUILD_SYSTEM_ROOT_IMAGE: { installable: false, }, + BOARD_USES_RECOVERY_AS_BOOT: { + installable: false, + }, }, } diff --git a/init/Android.mk b/init/Android.mk index c1b0cf9d2..c08fe0393 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -9,8 +9,10 @@ LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 LOCAL_LICENSE_CONDITIONS := notice LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE ifneq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true) +ifneq ($(BOARD_USES_RECOVERY_AS_BOOT),true) LOCAL_REQUIRED_MODULES := \ init_first_stage \ +endif # BOARD_USES_RECOVERY_AS_BOOT endif # BOARD_BUILD_SYSTEM_ROOT_IMAGE include $(BUILD_PHONY_PACKAGE) From 651db0935d0f7ce424c04a24ecf8c36d151b2631 Mon Sep 17 00:00:00 2001 From: Inseob Kim Date: Thu, 1 Jul 2021 06:50:40 +0000 Subject: [PATCH 11/27] Revert "Completely migrate init first stage to Soong" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Update init_first_stage" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add ramdisk_available to init_first_stage's deps" Revert "Add BOARD_BUILD_SYSTEM_ROOT_IMAGE to config vars" Revert "Add install_in_root to cc_binary" Revert "Add ramdisk_available to init_first_stage's deps" Revert submission 15071196-init_first_stage_soong Reason for revert: fixes b/192248690 Reverted Changes: I23cf4f975:Add ramdisk_available to init_first_stage's deps Icd98c7e24:Add ramdisk_available to init_first_stage's deps If9da9ba16:Add ramdisk_available to init_first_stage's deps Ibc8668029:Add ramdisk_available to init_first_stage's deps I3b4b8c475:Add ramdisk_available to init_first_stage's deps I59cd149e0:Completely migrate init first stage to Soong I36d789578:Add ramdisk_available to init_first_stage's deps I2a0daa612:Add BUILD_USES_RECOVERY_AS_BOOT to soong config Ic76c325ce:Directly create ramdisk dirs in ramdisk image rule... I4c5374deb:Add BOARD_BUILD_SYSTEM_ROOT_IMAGE to config vars I8aab5faf3:Add ramdisk_available to init_first_stage's deps I9d5a10661:Add ramdisk_available to init_first_stage's deps Iaa2edeb4a:Add ramdisk_available to init_first_stage's deps I7cb582ca0:Update init_first_stage I06091d15e:Add ramdisk_available to init_first_stage's deps I8bdb8dda3:Add ramdisk_available to init_first_stage's deps I7436b8dd1:Add ramdisk_available to init_first_stage's deps I39693fd86:Add ramdisk_available to init_first_stage's deps I0a9ba90f0:Add ramdisk_available to init_first_stage's deps Ib66b4c4ea:Add ramdisk_available to init_first_stage's deps I31ce63d23:Add ramdisk_available to init_first_stage's deps Icb580f97c:Add ramdisk_available to init_first_stage's deps I044a075b7:Add ramdisk_available to init_first_stage's deps I33164a7e7:Fix ndk and aml arch order Ib8d92904a:Add ramdisk_available to sysprop_library Ibc3516453:Add install_in_root to cc_binary Change-Id: I147777bb1c4a3b818bc0118c6cf44ccfbf7970a0 --- fs_mgr/Android.bp | 2 - fs_mgr/libfiemap/Android.bp | 1 - fs_mgr/libfs_avb/Android.bp | 1 - fs_mgr/liblp/Android.bp | 1 - fs_mgr/libsnapshot/Android.bp | 1 - fs_mgr/libstorage_literals/Android.bp | 1 - init/Android.bp | 45 +------- init/Android.mk | 151 +++++++++++++++++++++++++- libcrypto_utils/Android.bp | 1 - libkeyutils/Android.bp | 1 - libmodprobe/Android.bp | 1 - rootdir/Android.bp | 9 +- rootdir/Android.mk | 11 ++ 13 files changed, 164 insertions(+), 62 deletions(-) diff --git a/fs_mgr/Android.bp b/fs_mgr/Android.bp index 3d63a4437..5356b006d 100644 --- a/fs_mgr/Android.bp +++ b/fs_mgr/Android.bp @@ -141,7 +141,6 @@ cc_library { // Do not ever allow this library to be vendor_available as a shared library. // It does not have a stable interface. name: "libfs_mgr", - ramdisk_available: true, recovery_available: true, defaults: [ "libfs_mgr_defaults", @@ -166,7 +165,6 @@ cc_library_static { // It does not have a stable interface. name: "libfstab", vendor_available: true, - ramdisk_available: true, recovery_available: true, host_supported: true, defaults: ["fs_mgr_defaults"], diff --git a/fs_mgr/libfiemap/Android.bp b/fs_mgr/libfiemap/Android.bp index b62e33fdd..1c5872e6f 100644 --- a/fs_mgr/libfiemap/Android.bp +++ b/fs_mgr/libfiemap/Android.bp @@ -20,7 +20,6 @@ package { cc_library_headers { name: "libfiemap_headers", - ramdisk_available: true, recovery_available: true, export_include_dirs: ["include"], } diff --git a/fs_mgr/libfs_avb/Android.bp b/fs_mgr/libfs_avb/Android.bp index 62493ebd9..6892025a1 100644 --- a/fs_mgr/libfs_avb/Android.bp +++ b/fs_mgr/libfs_avb/Android.bp @@ -27,7 +27,6 @@ package { cc_library_static { name: "libfs_avb", defaults: ["fs_mgr_defaults"], - ramdisk_available: true, recovery_available: true, host_supported: true, export_include_dirs: ["include"], diff --git a/fs_mgr/liblp/Android.bp b/fs_mgr/liblp/Android.bp index 86ca8f35f..7e528b164 100644 --- a/fs_mgr/liblp/Android.bp +++ b/fs_mgr/liblp/Android.bp @@ -30,7 +30,6 @@ liblp_lib_deps = [ cc_library { name: "liblp", host_supported: true, - ramdisk_available: true, recovery_available: true, defaults: ["fs_mgr_defaults"], cppflags: [ diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index aa1f4154c..6a764e4fa 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -118,7 +118,6 @@ cc_library_static { native_coverage : true, defaults: ["libsnapshot_defaults"], srcs: [":libsnapshot_sources"], - ramdisk_available: true, recovery_available: true, cflags: [ "-DLIBSNAPSHOT_NO_COW_WRITE", diff --git a/fs_mgr/libstorage_literals/Android.bp b/fs_mgr/libstorage_literals/Android.bp index fd7ea0473..5b0716851 100644 --- a/fs_mgr/libstorage_literals/Android.bp +++ b/fs_mgr/libstorage_literals/Android.bp @@ -6,7 +6,6 @@ package { cc_library_headers { name: "libstorage_literals_headers", host_supported: true, - ramdisk_available: true, recovery_available: true, export_include_dirs: ["."], target: { diff --git a/init/Android.bp b/init/Android.bp index d0b58caf7..7eeafa24b 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -253,32 +253,11 @@ cc_binary { visibility: ["//packages/modules/Virtualization/microdroid"], } -soong_config_module_type { - name: "init_first_stage_cc_defaults", - module_type: "cc_defaults", - config_namespace: "ANDROID", - bool_variables: ["BOARD_BUILD_SYSTEM_ROOT_IMAGE", "BOARD_USES_RECOVERY_AS_BOOT"], - properties: ["installable"], -} - -// Do not install init_first_stage even with mma if we're system-as-root. -// Otherwise, it will overwrite the symlink. -init_first_stage_cc_defaults { - name: "init_first_stage_defaults", - soong_config_variables: { - BOARD_BUILD_SYSTEM_ROOT_IMAGE: { - installable: false, - }, - BOARD_USES_RECOVERY_AS_BOOT: { - installable: false, - }, - }, -} - +// This currently is only for the VM usecase. +// TODO(jiyong): replace init_first_stage in Android.mk with this cc_binary { - name: "init_first_stage", - stem: "init", - defaults: ["init_first_stage_defaults"], + name: "init_first_stage_soong", + stem: "init_vendor", srcs: [ "block_dev_initializer.cpp", @@ -334,7 +313,6 @@ cc_binary { ], static_executable: true, - system_shared_libs: [], cflags: [ "-Wall", @@ -385,23 +363,8 @@ cc_binary { sanitize: { misc_undefined: ["signed-integer-overflow"], - - // First stage init is weird: it may start without stdout/stderr, and no /proc. hwaddress: false, }, - - // Install adb_debug.prop into debug ramdisk. - // This allows adb root on a user build, when debug ramdisk is used. - required: ["adb_debug.prop"], - - ramdisk: true, - - install_in_root: true, -} - -phony { - name: "init_system", - required: ["init_second_stage"], } // Tests diff --git a/init/Android.mk b/init/Android.mk index c08fe0393..3c7d95acf 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -2,6 +2,153 @@ LOCAL_PATH:= $(call my-dir) +-include system/sepolicy/policy_version.mk + +# -- + +ifneq (,$(filter userdebug eng,$(TARGET_BUILD_VARIANT))) +init_options += \ + -DALLOW_FIRST_STAGE_CONSOLE=1 \ + -DALLOW_LOCAL_PROP_OVERRIDE=1 \ + -DALLOW_PERMISSIVE_SELINUX=1 \ + -DREBOOT_BOOTLOADER_ON_PANIC=1 \ + -DWORLD_WRITABLE_KMSG=1 \ + -DDUMP_ON_UMOUNT_FAILURE=1 +else +init_options += \ + -DALLOW_FIRST_STAGE_CONSOLE=0 \ + -DALLOW_LOCAL_PROP_OVERRIDE=0 \ + -DALLOW_PERMISSIVE_SELINUX=0 \ + -DREBOOT_BOOTLOADER_ON_PANIC=0 \ + -DWORLD_WRITABLE_KMSG=0 \ + -DDUMP_ON_UMOUNT_FAILURE=0 +endif + +ifneq (,$(filter eng,$(TARGET_BUILD_VARIANT))) +init_options += \ + -DSHUTDOWN_ZERO_TIMEOUT=1 +else +init_options += \ + -DSHUTDOWN_ZERO_TIMEOUT=0 +endif + +init_options += -DLOG_UEVENTS=0 \ + -DSEPOLICY_VERSION=$(POLICYVERS) + +init_cflags += \ + $(init_options) \ + -Wall -Wextra \ + -Wno-unused-parameter \ + -Werror \ + +# -- + +# Do not build this even with mmma if we're system-as-root, otherwise it will overwrite the symlink. +ifneq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true) +include $(CLEAR_VARS) +LOCAL_CPPFLAGS := $(init_cflags) +LOCAL_SRC_FILES := \ + block_dev_initializer.cpp \ + devices.cpp \ + first_stage_console.cpp \ + first_stage_init.cpp \ + first_stage_main.cpp \ + first_stage_mount.cpp \ + reboot_utils.cpp \ + selabel.cpp \ + selinux.cpp \ + service_utils.cpp \ + snapuserd_transition.cpp \ + switch_root.cpp \ + uevent_listener.cpp \ + util.cpp \ + +LOCAL_MODULE := init_first_stage +LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 +LOCAL_LICENSE_CONDITIONS := notice +LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE +LOCAL_MODULE_STEM := init + +LOCAL_FORCE_STATIC_EXECUTABLE := true + +LOCAL_MODULE_PATH := $(TARGET_RAMDISK_OUT) +LOCAL_UNSTRIPPED_PATH := $(TARGET_RAMDISK_OUT_UNSTRIPPED) + +# Install adb_debug.prop into debug ramdisk. +# This allows adb root on a user build, when debug ramdisk is used. +LOCAL_REQUIRED_MODULES := \ + adb_debug.prop \ + +# Set up the directories that first stage init mounts on. + +my_ramdisk_dirs := \ + debug_ramdisk \ + dev \ + metadata \ + mnt \ + proc \ + second_stage_resources \ + sys \ + +LOCAL_POST_INSTALL_CMD := mkdir -p $(addprefix $(TARGET_RAMDISK_OUT)/,$(my_ramdisk_dirs)) +ifeq (true,$(BOARD_USES_GENERIC_KERNEL_IMAGE)) + LOCAL_POST_INSTALL_CMD += $(addprefix $(TARGET_RAMDISK_OUT)/first_stage_ramdisk/,$(my_ramdisk_dirs)) +endif + +my_ramdisk_dirs := + +LOCAL_STATIC_LIBRARIES := \ + libc++fs \ + libfs_avb \ + libfs_mgr \ + libfec \ + libfec_rs \ + libsquashfs_utils \ + liblogwrap \ + libext4_utils \ + libcrypto_utils \ + libsparse \ + libavb \ + libkeyutils \ + liblp \ + libcutils \ + libbase \ + liblog \ + libcrypto_static \ + libdl \ + libz \ + libselinux \ + libcap \ + libgsi \ + libcom.android.sysprop.apex \ + liblzma \ + libunwindstack_no_dex \ + libbacktrace_no_dex \ + libmodprobe \ + libext2_uuid \ + libprotobuf-cpp-lite \ + libsnapshot_cow \ + libsnapshot_init \ + update_metadata-protos \ + libprocinfo \ + +LOCAL_SANITIZE := signed-integer-overflow +# First stage init is weird: it may start without stdout/stderr, and no /proc. +LOCAL_NOSANITIZE := hwaddress +include $(BUILD_EXECUTABLE) +endif + +include $(CLEAR_VARS) + +LOCAL_MODULE := init_system +LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 +LOCAL_LICENSE_CONDITIONS := notice +LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE +LOCAL_REQUIRED_MODULES := \ + init_second_stage \ + +include $(BUILD_PHONY_PACKAGE) + include $(CLEAR_VARS) LOCAL_MODULE := init_vendor @@ -9,10 +156,8 @@ LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 LOCAL_LICENSE_CONDITIONS := notice LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE ifneq ($(BOARD_BUILD_SYSTEM_ROOT_IMAGE),true) -ifneq ($(BOARD_USES_RECOVERY_AS_BOOT),true) LOCAL_REQUIRED_MODULES := \ init_first_stage \ -endif # BOARD_USES_RECOVERY_AS_BOOT -endif # BOARD_BUILD_SYSTEM_ROOT_IMAGE +endif include $(BUILD_PHONY_PACKAGE) diff --git a/libcrypto_utils/Android.bp b/libcrypto_utils/Android.bp index c8a183bf1..b33d46d50 100644 --- a/libcrypto_utils/Android.bp +++ b/libcrypto_utils/Android.bp @@ -21,7 +21,6 @@ package { cc_library { name: "libcrypto_utils", vendor_available: true, - ramdisk_available: true, recovery_available: true, vndk: { enabled: true, diff --git a/libkeyutils/Android.bp b/libkeyutils/Android.bp index a940b8cec..86f68fb57 100644 --- a/libkeyutils/Android.bp +++ b/libkeyutils/Android.bp @@ -15,7 +15,6 @@ cc_library { name: "libkeyutils", cflags: ["-Werror"], defaults: ["linux_bionic_supported"], - ramdisk_available: true, recovery_available: true, export_include_dirs: ["include/"], local_include_dirs: ["include/"], diff --git a/libmodprobe/Android.bp b/libmodprobe/Android.bp index 525a88063..ba11dc920 100644 --- a/libmodprobe/Android.bp +++ b/libmodprobe/Android.bp @@ -8,7 +8,6 @@ cc_library_static { "-Werror", ], vendor_available: true, - ramdisk_available: true, recovery_available: true, srcs: [ "libmodprobe.cpp", diff --git a/rootdir/Android.bp b/rootdir/Android.bp index e98733ada..ae21633da 100644 --- a/rootdir/Android.bp +++ b/rootdir/Android.bp @@ -45,11 +45,4 @@ prebuilt_etc { src: "etc/public.libraries.android.txt", filename: "public.libraries.txt", installable: false, -} - -// adb_debug.prop in debug ramdisk -prebuilt_root { - name: "adb_debug.prop", - src: "adb_debug.prop", - debug_ramdisk: true, -} +} \ No newline at end of file diff --git a/rootdir/Android.mk b/rootdir/Android.mk index 9b80575ef..99d8f9a83 100644 --- a/rootdir/Android.mk +++ b/rootdir/Android.mk @@ -210,4 +210,15 @@ $(LOCAL_BUILT_MODULE): $(hide) $(foreach lib,$(PRIVATE_SANITIZER_RUNTIME_LIBRARIES), \ echo $(lib) >> $@;) +####################################### +# adb_debug.prop in debug ramdisk +include $(CLEAR_VARS) +LOCAL_MODULE := adb_debug.prop +LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 +LOCAL_LICENSE_CONDITIONS := notice +LOCAL_SRC_FILES := $(LOCAL_MODULE) +LOCAL_MODULE_CLASS := ETC +LOCAL_MODULE_PATH := $(TARGET_DEBUG_RAMDISK_OUT) +include $(BUILD_PREBUILT) + include $(call all-makefiles-under,$(LOCAL_PATH)) From 94c4e237e51743750e8372292d33a8ade37c32f7 Mon Sep 17 00:00:00 2001 From: Janis Danisevskis Date: Thu, 1 Jul 2021 14:20:06 -0700 Subject: [PATCH 12/27] Forward HAT and ConfirmationToken to TA on finish. The Trusty KeyMint HAL did not forward auth tokens and confirmation tokens to the TA. This broke all per-op-bound key operations. Ignore-AOSP-First: No mergepath from AOSP. Test: CtsVerifier biometrics tests. Bug: 192201272 Change-Id: Ifb2b08514acab78ff3d4fec4bc928260820d4ce0 --- .../keymint/TrustyKeyMintOperation.cpp | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/trusty/keymaster/keymint/TrustyKeyMintOperation.cpp b/trusty/keymaster/keymint/TrustyKeyMintOperation.cpp index 41a21e9f4..9440724da 100644 --- a/trusty/keymaster/keymint/TrustyKeyMintOperation.cpp +++ b/trusty/keymaster/keymint/TrustyKeyMintOperation.cpp @@ -34,6 +34,7 @@ using ::keymaster::FinishOperationRequest; using ::keymaster::FinishOperationResponse; using ::keymaster::TAG_ASSOCIATED_DATA; using ::keymaster::TAG_AUTH_TOKEN; +using ::keymaster::TAG_CONFIRMATION_TOKEN; using ::keymaster::UpdateOperationRequest; using ::keymaster::UpdateOperationResponse; using km_utils::authToken2AidlVec; @@ -106,12 +107,12 @@ ScopedAStatus TrustyKeyMintOperation::update(const vector& input, return ScopedAStatus::ok(); } -ScopedAStatus TrustyKeyMintOperation::finish( - const optional>& input, // - const optional>& signature, // - const optional& authToken, - const optional& /* timestampToken */, - const optional>& /* confirmationToken */, vector* output) { +ScopedAStatus TrustyKeyMintOperation::finish(const optional>& input, // + const optional>& signature, // + const optional& authToken, + const optional& /* timestampToken */, + const optional>& confirmationToken, + vector* output) { if (!output) { return ScopedAStatus(AStatus_fromServiceSpecificError( static_cast(ErrorCode::OUTPUT_PARAMETER_NULL))); @@ -119,6 +120,16 @@ ScopedAStatus TrustyKeyMintOperation::finish( output->clear(); FinishOperationRequest request(impl_->message_version()); + + if (authToken) { + auto tokenAsVec(authToken2AidlVec(*authToken)); + request.additional_params.push_back(TAG_AUTH_TOKEN, tokenAsVec.data(), tokenAsVec.size()); + } + if (confirmationToken) { + request.additional_params.push_back(TAG_CONFIRMATION_TOKEN, confirmationToken->data(), + confirmationToken->size()); + } + request.op_handle = opHandle_; if (signature) request.signature.Reinitialize(signature->data(), signature->size()); size_t serialized_size = request.SerializedSize(); From e00a5670476b06b1f85886eb1862ac217ca4384a Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 29 Jun 2021 19:54:54 -0700 Subject: [PATCH 13/27] libsnapshot: Add a source build fingerprint to the update state. Bug: 188909957 Test: manual test Change-Id: I9aa155eee25dd49f48baede4f0a2e4ab2ab76980 Merged-In: I9aa155eee25dd49f48baede4f0a2e4ab2ab76980 --- .../android/snapshot/snapshot.proto | 6 ++++ .../include/libsnapshot/mock_snapshot.h | 1 + .../libsnapshot/mock_snapshot_merge_stats.h | 3 ++ .../include/libsnapshot/snapshot.h | 4 +++ .../include/libsnapshot/snapshot_stats.h | 10 +++++- .../include/libsnapshot/snapshot_stub.h | 1 + fs_mgr/libsnapshot/snapshot.cpp | 34 +++++++++++++++---- fs_mgr/libsnapshot/snapshot_stats.cpp | 9 ++++- fs_mgr/libsnapshot/snapshot_stub.cpp | 10 +++++- 9 files changed, 68 insertions(+), 10 deletions(-) diff --git a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto index 9f227c970..de8768c71 100644 --- a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto +++ b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto @@ -191,6 +191,9 @@ message SnapshotUpdateStatus { // Merge failure code, filled if state == MergeFailed. MergeFailureCode merge_failure_code = 7; + + // Source build fingerprint. + string source_build_fingerprint = 8; } // Next: 10 @@ -222,4 +225,7 @@ message SnapshotMergeReport { // Merge failure code, filled if state == MergeFailed. MergeFailureCode merge_failure_code = 9; + + // The source fingerprint at the time the OTA was downloaded. + string source_build_fingerprint = 10; } diff --git a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h index 94d5055c1..ec58cca2e 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot.h @@ -60,6 +60,7 @@ class MockSnapshotManager : public ISnapshotManager { MOCK_METHOD(bool, Dump, (std::ostream & os), (override)); MOCK_METHOD(std::unique_ptr, EnsureMetadataMounted, (), (override)); MOCK_METHOD(ISnapshotMergeStats*, GetSnapshotMergeStatsInstance, (), (override)); + MOCK_METHOD(std::string, ReadSourceBuildFingerprint, (), (override)); }; } // namespace android::snapshot diff --git a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h index 067f99c5c..3d384cc04 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/mock_snapshot_merge_stats.h @@ -35,13 +35,16 @@ class MockSnapshotMergeStats final : public ISnapshotMergeStats { MOCK_METHOD(void, set_boot_complete_time_ms, (uint32_t), (override)); MOCK_METHOD(void, set_boot_complete_to_merge_start_time_ms, (uint32_t), (override)); MOCK_METHOD(void, set_merge_failure_code, (MergeFailureCode), (override)); + MOCK_METHOD(void, set_source_build_fingerprint, (const std::string&), (override)); MOCK_METHOD(uint64_t, cow_file_size, (), (override)); MOCK_METHOD(uint64_t, total_cow_size_bytes, (), (override)); MOCK_METHOD(uint64_t, estimated_cow_size_bytes, (), (override)); MOCK_METHOD(uint32_t, boot_complete_time_ms, (), (override)); MOCK_METHOD(uint32_t, boot_complete_to_merge_start_time_ms, (), (override)); + MOCK_METHOD(std::string, source_build_fingerprint, (), (override)); MOCK_METHOD(MergeFailureCode, merge_failure_code, (), (override)); MOCK_METHOD(std::unique_ptr, Finish, (), (override)); + MOCK_METHOD(bool, WriteState, (), (override)); using ISnapshotMergeStats::Result; // Return nullptr if any failure. diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 65034f71e..15882b382 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -177,6 +177,9 @@ class ISnapshotManager { // code. Otherwise, MergeFailureCode::Ok is returned. virtual MergeFailureCode ReadMergeFailureCode() = 0; + // If an update is in progress, return the source build fingerprint. + virtual std::string ReadSourceBuildFingerprint() = 0; + // Find the status of the current update, if any. // // |progress| depends on the returned status: @@ -369,6 +372,7 @@ class SnapshotManager final : public ISnapshotManager { ISnapshotMergeStats* GetSnapshotMergeStatsInstance() override; bool MapAllSnapshots(const std::chrono::milliseconds& timeout_ms = {}) override; bool UnmapAllSnapshots() override; + std::string ReadSourceBuildFingerprint() override; // We can't use WaitForFile during first-stage init, because ueventd is not // running and therefore will not automatically create symlinks. Instead, diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h index 4ce50778d..8c2fec726 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h @@ -35,12 +35,14 @@ class ISnapshotMergeStats { virtual void set_boot_complete_time_ms(uint32_t ms) = 0; virtual void set_boot_complete_to_merge_start_time_ms(uint32_t ms) = 0; virtual void set_merge_failure_code(MergeFailureCode code) = 0; + virtual void set_source_build_fingerprint(const std::string& fingerprint) = 0; virtual uint64_t cow_file_size() = 0; virtual uint64_t total_cow_size_bytes() = 0; virtual uint64_t estimated_cow_size_bytes() = 0; virtual uint32_t boot_complete_time_ms() = 0; virtual uint32_t boot_complete_to_merge_start_time_ms() = 0; virtual MergeFailureCode merge_failure_code() = 0; + virtual std::string source_build_fingerprint() = 0; // Called when merge ends. Properly clean up permanent storage. class Result { @@ -52,6 +54,10 @@ class ISnapshotMergeStats { }; // Return nullptr if any failure. virtual std::unique_ptr Finish() = 0; + + // Write out the current state. This should be called when data might be lost that + // cannot be recovered (eg the COW sizes). + virtual bool WriteState() = 0; }; class SnapshotMergeStats : public ISnapshotMergeStats { @@ -74,11 +80,13 @@ class SnapshotMergeStats : public ISnapshotMergeStats { uint32_t boot_complete_to_merge_start_time_ms() override; void set_merge_failure_code(MergeFailureCode code) override; MergeFailureCode merge_failure_code() override; + void set_source_build_fingerprint(const std::string& fingerprint) override; + std::string source_build_fingerprint() override; std::unique_ptr Finish() override; + bool WriteState() override; private: bool ReadState(); - bool WriteState(); bool DeleteState(); SnapshotMergeStats(const std::string& path); diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h index a7cd939b5..74b78c59b 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stub.h @@ -57,6 +57,7 @@ class SnapshotManagerStub : public ISnapshotManager { ISnapshotMergeStats* GetSnapshotMergeStatsInstance() override; bool MapAllSnapshots(const std::chrono::milliseconds& timeout_ms) override; bool UnmapAllSnapshots() override; + std::string ReadSourceBuildFingerprint() override; }; } // namespace android::snapshot diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index be732ece9..0e36da151 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -716,7 +716,7 @@ bool SnapshotManager::InitiateMerge() { } } - SnapshotUpdateStatus initial_status; + SnapshotUpdateStatus initial_status = ReadSnapshotUpdateStatus(lock.get()); initial_status.set_state(UpdateState::Merging); initial_status.set_sectors_allocated(initial_target_values.sectors_allocated); initial_status.set_total_sectors(initial_target_values.total_sectors); @@ -2515,15 +2515,25 @@ bool SnapshotManager::WriteUpdateState(LockedFile* lock, UpdateState state, SnapshotUpdateStatus status; status.set_state(state); - if (state == UpdateState::MergeFailed) { - status.set_merge_failure_code(failure_code); + switch (state) { + case UpdateState::MergeFailed: + status.set_merge_failure_code(failure_code); + break; + case UpdateState::Initiated: + status.set_source_build_fingerprint( + android::base::GetProperty("ro.build.fingerprint", "")); + break; + default: + break; } // If we're transitioning between two valid states (eg, we're not beginning - // or ending an OTA), then make sure to propagate the compression bit. + // or ending an OTA), then make sure to propagate the compression bit and + // build fingerprint. if (!(state == UpdateState::Initiated || state == UpdateState::None)) { SnapshotUpdateStatus old_status = ReadSnapshotUpdateStatus(lock); status.set_compression_enabled(old_status.compression_enabled()); + status.set_source_build_fingerprint(old_status.source_build_fingerprint()); } return WriteSnapshotUpdateStatus(lock, status); } @@ -2838,7 +2848,7 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife } } - SnapshotUpdateStatus status = {}; + SnapshotUpdateStatus status = ReadSnapshotUpdateStatus(lock.get()); status.set_state(update_state); status.set_compression_enabled(cow_creator.compression_enabled); if (!WriteSnapshotUpdateStatus(lock.get(), status)) { @@ -3264,9 +3274,10 @@ bool SnapshotManager::Dump(std::ostream& os) { std::stringstream ss; + auto update_status = ReadSnapshotUpdateStatus(file.get()); + ss << "Update state: " << ReadUpdateState(file.get()) << std::endl; - ss << "Compression: " << ReadSnapshotUpdateStatus(file.get()).compression_enabled() - << std::endl; + ss << "Compression: " << update_status.compression_enabled() << std::endl; ss << "Current slot: " << device_->GetSlotSuffix() << std::endl; ss << "Boot indicator: booting from " << GetCurrentSlot() << " slot" << std::endl; ss << "Rollback indicator: " @@ -3275,6 +3286,7 @@ bool SnapshotManager::Dump(std::ostream& os) { ss << "Forward merge indicator: " << (access(GetForwardMergeIndicatorPath().c_str(), F_OK) == 0 ? "exists" : strerror(errno)) << std::endl; + ss << "Source build fingerprint: " << update_status.source_build_fingerprint() << std::endl; bool ok = true; std::vector snapshots; @@ -3792,5 +3804,13 @@ MergeFailureCode SnapshotManager::ReadMergeFailureCode() { return status.merge_failure_code(); } +std::string SnapshotManager::ReadSourceBuildFingerprint() { + auto lock = LockExclusive(); + if (!lock) return {}; + + SnapshotUpdateStatus status = ReadSnapshotUpdateStatus(lock.get()); + return status.source_build_fingerprint(); +} + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot_stats.cpp b/fs_mgr/libsnapshot/snapshot_stats.cpp index 4a93d655e..712eafba9 100644 --- a/fs_mgr/libsnapshot/snapshot_stats.cpp +++ b/fs_mgr/libsnapshot/snapshot_stats.cpp @@ -91,7 +91,6 @@ void SnapshotMergeStats::set_state(android::snapshot::UpdateState state, bool us void SnapshotMergeStats::set_cow_file_size(uint64_t cow_file_size) { report_.set_cow_file_size(cow_file_size); - WriteState(); } uint64_t SnapshotMergeStats::cow_file_size() { @@ -138,6 +137,14 @@ MergeFailureCode SnapshotMergeStats::merge_failure_code() { return report_.merge_failure_code(); } +void SnapshotMergeStats::set_source_build_fingerprint(const std::string& fingerprint) { + report_.set_source_build_fingerprint(fingerprint); +} + +std::string SnapshotMergeStats::source_build_fingerprint() { + return report_.source_build_fingerprint(); +} + class SnapshotMergeStatsResultImpl : public SnapshotMergeStats::Result { public: SnapshotMergeStatsResultImpl(const SnapshotMergeReport& report, diff --git a/fs_mgr/libsnapshot/snapshot_stub.cpp b/fs_mgr/libsnapshot/snapshot_stub.cpp index 1a9eda5e6..a8d5b8a1a 100644 --- a/fs_mgr/libsnapshot/snapshot_stub.cpp +++ b/fs_mgr/libsnapshot/snapshot_stub.cpp @@ -136,7 +136,10 @@ class SnapshotMergeStatsStub : public ISnapshotMergeStats { void set_boot_complete_to_merge_start_time_ms(uint32_t) override {} uint32_t boot_complete_to_merge_start_time_ms() override { return 0; } void set_merge_failure_code(MergeFailureCode) override {} - MergeFailureCode merge_failure_code() { return MergeFailureCode::Ok; } + MergeFailureCode merge_failure_code() override { return MergeFailureCode::Ok; } + void set_source_build_fingerprint(const std::string&) override {} + std::string source_build_fingerprint() override { return {}; } + bool WriteState() override { return false; } }; ISnapshotMergeStats* SnapshotManagerStub::GetSnapshotMergeStatsInstance() { @@ -170,4 +173,9 @@ auto SnapshotManagerStub::ReadMergeFailureCode() -> MergeFailureCode { return MergeFailureCode::Ok; } +std::string SnapshotManagerStub::ReadSourceBuildFingerprint() { + LOG(ERROR) << __FUNCTION__ << " should never be called."; + return {}; +} + } // namespace android::snapshot From cc25244b774a7063c0a212b883b251e47573af1c Mon Sep 17 00:00:00 2001 From: Li Li Date: Thu, 1 Jul 2021 15:10:05 -0700 Subject: [PATCH 14/27] libprocessgroup: Do not remove uid cgroups directory In some rare cases, race happens between 2 processes in the same uid. 1. Process A is dying 2. system_server calls RemoveProcessGroup() for A 3. Zygote forks Process B with the same uid of A 4. system_server calls MkdirAndChown(uid) for B 5. system_server calls MkdirAndChown(uid, pid) for B As 2 & 4/5 belong to different threads, 2 might happens before or after step 4/5, or even in the middle of 4/5. In such a case, 4 or 5 will fail, leaving process B in wrong (Zygote) group. The uid dir is only created when the corresponding apps have been launched at least once. It's reasonable to assume one of them is going to be launched again. Deleting and recreating the uid dir just slows down applaunch. Introducing a new lock in libprocessgroup can also solve the race issue. But that will slow down the applaunch further. Therefore, reusing the uid dir is an optimized way to solve the race. Ignore-AOSP-First: Freezer is not a public feature yet Bug: 192512069 Bug: 168907513 Test: Kill corresponding apps and check the uid cgroupfs dir Change-Id: I2e91088f21f45e4eda6c709a4af65ace7e135801 --- libprocessgroup/processgroup.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index 815d2bb78..5c7a75dba 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -146,12 +146,6 @@ static int RemoveProcessGroup(const char* cgroup, uid_t uid, int pid, unsigned i std::this_thread::sleep_for(5ms); } - // With the exception of boot or shutdown, system uid_ folders are always populated. Spinning - // here would needlessly delay most pid removals. Additionally, once empty a uid_ cgroup won't - // have processes hanging on it (we've already spun for all its pid_), so there's no need to - // spin anyway. - rmdir(uid_path.c_str()); - return ret; } From 5ac2c87c7abaa8bbb07d6e998decd6a7cc298aa8 Mon Sep 17 00:00:00 2001 From: Max Bires Date: Wed, 9 Jun 2021 17:40:54 -0700 Subject: [PATCH 15/27] Client side implementation of Trusty IRPC HAL This change includes the code necessary to communicate to the IRemotelyProvisionedComponent backend implementation running in Trusty. It also makes the relevant changes to the manifest XML file to add the IRemotelyProvisionedComponent HAL. Ignore-AOSP-First: Will cherry-pick to AOSP Bug: 192228022 Test: atest VtsHalRemotelyProvisionedComponentTargetTest Change-Id: I32c30ce2dc44e95ff91574ce405f10e3b5dc9699 --- trusty/keymaster/Android.bp | 2 +- trusty/keymaster/TrustyKeymaster.cpp | 10 ++ .../trusty_keymaster/TrustyKeymaster.h | 2 + ...TrustyRemotelyProvisionedComponentDevice.h | 53 ++++++++ .../trusty_keymaster/ipc/keymaster_ipc.h | 2 + ...ustyRemotelyProvisionedComponentDevice.cpp | 120 ++++++++++++++++++ ...rdware.security.keymint-service.trusty.xml | 4 + trusty/keymaster/keymint/service.cpp | 5 +- 8 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 trusty/keymaster/include/trusty_keymaster/TrustyRemotelyProvisionedComponentDevice.h create mode 100644 trusty/keymaster/keymint/TrustyRemotelyProvisionedComponentDevice.cpp diff --git a/trusty/keymaster/Android.bp b/trusty/keymaster/Android.bp index 33eb335fb..ff6460de8 100644 --- a/trusty/keymaster/Android.bp +++ b/trusty/keymaster/Android.bp @@ -100,6 +100,7 @@ cc_binary { "ipc/trusty_keymaster_ipc.cpp", "keymint/TrustyKeyMintDevice.cpp", "keymint/TrustyKeyMintOperation.cpp", + "keymint/TrustyRemotelyProvisionedComponentDevice.cpp", "keymint/TrustySecureClock.cpp", "keymint/TrustySharedSecret.cpp", "keymint/service.cpp", @@ -118,7 +119,6 @@ cc_binary { "libtrusty", ], required: [ - "RemoteProvisioner", "android.hardware.hardware_keystore.xml", ], } diff --git a/trusty/keymaster/TrustyKeymaster.cpp b/trusty/keymaster/TrustyKeymaster.cpp index ef5fc3fc2..aee33331a 100644 --- a/trusty/keymaster/TrustyKeymaster.cpp +++ b/trusty/keymaster/TrustyKeymaster.cpp @@ -158,6 +158,16 @@ void TrustyKeymaster::GenerateKey(const GenerateKeyRequest& request, } } +void TrustyKeymaster::GenerateRkpKey(const GenerateRkpKeyRequest& request, + GenerateRkpKeyResponse* response) { + ForwardCommand(KM_GENERATE_RKP_KEY, request, response); +} + +void TrustyKeymaster::GenerateCsr(const GenerateCsrRequest& request, + GenerateCsrResponse* response) { + ForwardCommand(KM_GENERATE_CSR, request, response); +} + void TrustyKeymaster::GetKeyCharacteristics(const GetKeyCharacteristicsRequest& request, GetKeyCharacteristicsResponse* response) { ForwardCommand(KM_GET_KEY_CHARACTERISTICS, request, response); diff --git a/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h b/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h index 45ebf7fda..35eda459c 100644 --- a/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h +++ b/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h @@ -42,6 +42,8 @@ class TrustyKeymaster { void AddRngEntropy(const AddEntropyRequest& request, AddEntropyResponse* response); void Configure(const ConfigureRequest& request, ConfigureResponse* response); void GenerateKey(const GenerateKeyRequest& request, GenerateKeyResponse* response); + void GenerateRkpKey(const GenerateRkpKeyRequest& request, GenerateRkpKeyResponse* response); + void GenerateCsr(const GenerateCsrRequest& request, GenerateCsrResponse* response); void GetKeyCharacteristics(const GetKeyCharacteristicsRequest& request, GetKeyCharacteristicsResponse* response); void ImportKey(const ImportKeyRequest& request, ImportKeyResponse* response); diff --git a/trusty/keymaster/include/trusty_keymaster/TrustyRemotelyProvisionedComponentDevice.h b/trusty/keymaster/include/trusty_keymaster/TrustyRemotelyProvisionedComponentDevice.h new file mode 100644 index 000000000..d544b51d5 --- /dev/null +++ b/trusty/keymaster/include/trusty_keymaster/TrustyRemotelyProvisionedComponentDevice.h @@ -0,0 +1,53 @@ +/* + * Copyright 2021, 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. + */ + +#pragma once + +#include +#include +#include + +#include + +namespace aidl::android::hardware::security::keymint::trusty { + +using ::keymaster::TrustyKeymaster; +using ::ndk::ScopedAStatus; +using ::std::shared_ptr; + +class TrustyRemotelyProvisionedComponentDevice : public BnRemotelyProvisionedComponent { + public: + explicit TrustyRemotelyProvisionedComponentDevice(shared_ptr impl) + : impl_(std::move(impl)) {} + virtual ~TrustyRemotelyProvisionedComponentDevice() = default; + + ScopedAStatus getHardwareInfo(RpcHardwareInfo* info) override; + + ScopedAStatus generateEcdsaP256KeyPair(bool testMode, MacedPublicKey* macedPublicKey, + std::vector* privateKeyHandle) override; + + ScopedAStatus generateCertificateRequest(bool testMode, + const std::vector& keysToSign, + const std::vector& endpointEncCertChain, + const std::vector& challenge, + DeviceInfo* deviceInfo, ProtectedData* protectedData, + std::vector* keysToSignMac) override; + + private: + std::shared_ptr<::keymaster::TrustyKeymaster> impl_; +}; + +} // namespace aidl::android::hardware::security::keymint::trusty diff --git a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h index 71f3ccf5b..17fee15f3 100644 --- a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h +++ b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h @@ -56,6 +56,8 @@ enum keymaster_command : uint32_t { KM_GET_VERSION_2 = (28 << KEYMASTER_REQ_SHIFT), KM_EARLY_BOOT_ENDED = (29 << KEYMASTER_REQ_SHIFT), KM_DEVICE_LOCKED = (30 << KEYMASTER_REQ_SHIFT), + KM_GENERATE_RKP_KEY = (31 << KEYMASTER_REQ_SHIFT), + KM_GENERATE_CSR = (32 << KEYMASTER_REQ_SHIFT), // Bootloader/provisioning calls. KM_SET_BOOT_PARAMS = (0x1000 << KEYMASTER_REQ_SHIFT), diff --git a/trusty/keymaster/keymint/TrustyRemotelyProvisionedComponentDevice.cpp b/trusty/keymaster/keymint/TrustyRemotelyProvisionedComponentDevice.cpp new file mode 100644 index 000000000..5664829d8 --- /dev/null +++ b/trusty/keymaster/keymint/TrustyRemotelyProvisionedComponentDevice.cpp @@ -0,0 +1,120 @@ +/* + * Copyright 2021, 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 + +#include +#include + +#include +#include + +#include + +namespace aidl::android::hardware::security::keymint::trusty { + +using keymaster::GenerateCsrRequest; +using keymaster::GenerateCsrResponse; +using keymaster::GenerateRkpKeyRequest; +using keymaster::GenerateRkpKeyResponse; +using keymaster::KeymasterBlob; +using ::std::string; +using ::std::unique_ptr; +using ::std::vector; +using bytevec = ::std::vector; + +namespace { + +constexpr auto STATUS_FAILED = IRemotelyProvisionedComponent::STATUS_FAILED; + +struct AStatusDeleter { + void operator()(AStatus* p) { AStatus_delete(p); } +}; + +class Status { + public: + Status() : status_(AStatus_newOk()) {} + Status(int32_t errCode, const std::string& errMsg) + : status_(AStatus_fromServiceSpecificErrorWithMessage(errCode, errMsg.c_str())) {} + explicit Status(const std::string& errMsg) + : status_(AStatus_fromServiceSpecificErrorWithMessage(STATUS_FAILED, errMsg.c_str())) {} + explicit Status(AStatus* status) : status_(status ? status : AStatus_newOk()) {} + + Status(Status&&) = default; + Status(const Status&) = delete; + + operator ::ndk::ScopedAStatus() && { // NOLINT(google-explicit-constructor) + return ndk::ScopedAStatus(status_.release()); + } + + bool isOk() const { return AStatus_isOk(status_.get()); } + + const char* getMessage() const { return AStatus_getMessage(status_.get()); } + + private: + std::unique_ptr status_; +}; + +} // namespace + +ScopedAStatus TrustyRemotelyProvisionedComponentDevice::getHardwareInfo(RpcHardwareInfo* info) { + info->versionNumber = 1; + info->rpcAuthorName = "Google"; + info->supportedEekCurve = RpcHardwareInfo::CURVE_25519; + return ScopedAStatus::ok(); +} + +ScopedAStatus TrustyRemotelyProvisionedComponentDevice::generateEcdsaP256KeyPair( + bool testMode, MacedPublicKey* macedPublicKey, bytevec* privateKeyHandle) { + GenerateRkpKeyRequest request(impl_->message_version()); + request.test_mode = testMode; + GenerateRkpKeyResponse response(impl_->message_version()); + impl_->GenerateRkpKey(request, &response); + if (response.error != KM_ERROR_OK) { + return Status(-static_cast(response.error), "Failure in key generation."); + } + + macedPublicKey->macedKey = km_utils::kmBlob2vector(response.maced_public_key); + *privateKeyHandle = km_utils::kmBlob2vector(response.key_blob); + return ScopedAStatus::ok(); +} + +ScopedAStatus TrustyRemotelyProvisionedComponentDevice::generateCertificateRequest( + bool testMode, const vector& keysToSign, + const bytevec& endpointEncCertChain, const bytevec& challenge, DeviceInfo* deviceInfo, + ProtectedData* protectedData, bytevec* keysToSignMac) { + GenerateCsrRequest request(impl_->message_version()); + request.test_mode = testMode; + request.num_keys = keysToSign.size(); + request.keys_to_sign_array = new KeymasterBlob[keysToSign.size()]; + for (size_t i = 0; i < keysToSign.size(); i++) { + request.SetKeyToSign(i, keysToSign[i].macedKey.data(), keysToSign[i].macedKey.size()); + } + request.SetEndpointEncCertChain(endpointEncCertChain.data(), endpointEncCertChain.size()); + request.SetChallenge(challenge.data(), challenge.size()); + GenerateCsrResponse response(impl_->message_version()); + impl_->GenerateCsr(request, &response); + + if (response.error != KM_ERROR_OK) { + return Status(-static_cast(response.error), "Failure in CSR Generation."); + } + deviceInfo->deviceInfo = km_utils::kmBlob2vector(response.device_info_blob); + protectedData->protectedData = km_utils::kmBlob2vector(response.protected_data_blob); + *keysToSignMac = km_utils::kmBlob2vector(response.keys_to_sign_mac); + return ScopedAStatus::ok(); +} + +} // namespace aidl::android::hardware::security::keymint::trusty diff --git a/trusty/keymaster/keymint/android.hardware.security.keymint-service.trusty.xml b/trusty/keymaster/keymint/android.hardware.security.keymint-service.trusty.xml index 0ab3d64cf..7ca50507d 100644 --- a/trusty/keymaster/keymint/android.hardware.security.keymint-service.trusty.xml +++ b/trusty/keymaster/keymint/android.hardware.security.keymint-service.trusty.xml @@ -11,4 +11,8 @@ android.hardware.security.sharedsecret ISharedSecret/default + + android.hardware.security.keymint + IRemotelyProvisionedComponent/default + diff --git a/trusty/keymaster/keymint/service.cpp b/trusty/keymaster/keymint/service.cpp index 8f5f0f815..4060278d4 100644 --- a/trusty/keymaster/keymint/service.cpp +++ b/trusty/keymaster/keymint/service.cpp @@ -20,10 +20,12 @@ #include #include +#include #include #include using aidl::android::hardware::security::keymint::trusty::TrustyKeyMintDevice; +using aidl::android::hardware::security::keymint::trusty::TrustyRemotelyProvisionedComponentDevice; using aidl::android::hardware::security::secureclock::trusty::TrustySecureClock; using aidl::android::hardware::security::sharedsecret::trusty::TrustySharedSecret; @@ -52,7 +54,8 @@ int main() { auto keyMint = addService(trustyKeymaster); auto secureClock = addService(trustyKeymaster); auto sharedSecret = addService(trustyKeymaster); - + auto remotelyProvisionedComponent = + addService(trustyKeymaster); ABinderProcess_joinThreadPool(); return EXIT_FAILURE; // should not reach } From add9a253356c73bae878203686d2bd124e24dfeb Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Wed, 7 Jul 2021 10:59:59 -0700 Subject: [PATCH 16/27] libprocessgroup: Remove unnecessary permissions change in uid/pid hierarchy When a new process is launched it ensures that all files under its uid/ and uid/pid hierarchy are accessible by the user/group of that process. If the directory already exists that means the access permissions have been already set before, therefore we do not need to reset them again. This also avoids a race between two processes in the same uid with one process being launched and walking the uid/ directory while the other process is being killed and changing the content of that directory. In such a race the process walking uid/ might find the uid/pid directory of the process being killed but by the time it tries to set its permissions the directory might be removed because the process got killed. The change eliminates the possibility of this race. Bug: 192421915 Bug: 192512069 Signed-off-by: Suren Baghdasaryan Change-Id: I182298c36f6b0b4580ab59e440bd3aea16f5fbfe --- libprocessgroup/processgroup.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index 5c7a75dba..c824376e5 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -224,7 +224,11 @@ void removeAllProcessGroups() { * transferred for the user/group passed as uid/gid before system_server can properly access them. */ static bool MkdirAndChown(const std::string& path, mode_t mode, uid_t uid, gid_t gid) { - if (mkdir(path.c_str(), mode) == -1 && errno != EEXIST) { + if (mkdir(path.c_str(), mode) == -1) { + if (errno == EEXIST) { + // Directory already exists and permissions have been set at the time it was created + return true; + } return false; } From b47f266671e97a8681a7dae697b40ee5ca32fa14 Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Mon, 5 Jul 2021 14:29:17 +0100 Subject: [PATCH 17/27] Also populate lastUpdateMillis in ActivateFlattenedApexesIfPossible Test: m Bug: 192647837 Change-Id: I11dca132168d6a30372d7a68fe590894d3cc5ccf Merged-In: I11dca132168d6a30372d7a68fe590894d3cc5ccf --- init/mount_namespace.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/init/mount_namespace.cpp b/init/mount_namespace.cpp index 15252a622..2a578082b 100644 --- a/init/mount_namespace.cpp +++ b/init/mount_namespace.cpp @@ -158,7 +158,8 @@ static bool ActivateFlattenedApexesIfPossible() { auto on_activate = [&](const std::string& apex_path, const apex::proto::ApexManifest& apex_manifest) { apex_infos.emplace_back(apex_manifest.name(), apex_path, apex_path, apex_manifest.version(), - apex_manifest.versionname(), /*isFactory=*/true, /*isActive=*/true); + apex_manifest.versionname(), /*isFactory=*/true, /*isActive=*/true, + /* lastUpdateMillis= */ 0); }; for (const auto& dir : kBuiltinDirsForApexes) { From 91ef4dacce84ef72412cc033624dc66675cf52a0 Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Wed, 7 Jul 2021 16:34:06 -0700 Subject: [PATCH 18/27] init: remove extra space in list of bootconfig values If a bootconfig argument has a list of values, it has a space between them in /proc/bootconfig. Example: BOARD_BOOTCONFIG := parameter=value1,value2,value3 In /proc/bootconfig, it looks like: parameter = "value1", "value2", "value3" Before this CL, that example would end up with the value string of: "value1, value2, value3" To keep consistent behavior with kernel cmdline the value string should be: "value1,value2,value3" Test: Boot cuttlefish with test bootconfig params and verify ro.boot.* Bug: 192257482 Merged-In: Iccdec451f53330162fa2c9ad2b7c2630f32b4168 Change-Id: Iccdec451f53330162fa2c9ad2b7c2630f32b4168 --- init/util.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/init/util.cpp b/init/util.cpp index a40d10416..9f7bfdb5b 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -253,8 +253,10 @@ void ImportBootconfig(const std::function pieces = android::base::Split(entry, "="); if (pieces.size() == 2) { - pieces[1].erase(std::remove(pieces[1].begin(), pieces[1].end(), '"'), pieces[1].end()); - fn(android::base::Trim(pieces[0]), android::base::Trim(pieces[1])); + // 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)); } } } From 3745ce7aa84e9007c94cb64ac631f4fa15efe051 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Wed, 7 Jul 2021 17:46:28 -0700 Subject: [PATCH 19/27] task_profiles.json: add taskprofile for dex2oat boot_complete policy Since installation time is also critical and sometime we want a finer control on its behavior for vendor. Instead of set dex2oat to background group, add a new task profile. The default policy is to mirror SP_BACKGROUND to be compatible with legacy devices. Test: Boot Bug: 188947181 Signed-off-by: Wei Wang Change-Id: I7d5e113d1c368205f712eadad27775de226131ce --- libprocessgroup/profiles/task_profiles.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libprocessgroup/profiles/task_profiles.json b/libprocessgroup/profiles/task_profiles.json index b5fa475fa..449a50546 100644 --- a/libprocessgroup/profiles/task_profiles.json +++ b/libprocessgroup/profiles/task_profiles.json @@ -584,7 +584,7 @@ } } ] - }, + } ], "AggregateProfiles": [ @@ -635,6 +635,10 @@ { "Name": "CPUSET_SP_RESTRICTED", "Profiles": [ "ServiceCapacityRestricted", "TimerSlackNormal" ] + }, + { + "Name": "Dex2OatBootComplete", + "Profiles": [ "SCHED_SP_BACKGROUND" ] } ] } From c66e99bf24bf07fdc3a601424512d332cc206f80 Mon Sep 17 00:00:00 2001 From: Wenhao Wang Date: Thu, 15 Jul 2021 22:12:08 -0700 Subject: [PATCH 20/27] trusty:storageproxyd: Fix return paths on errors The function send_ufs_rpmb_req is missing return paths on errors. This patch fixes it so that any UFS command failure will return error code to the function caller. Bug: 193855098 Test: Trusty storage tests Merged-In: I391ecff9ed3f892b7c3adae0ceeb18930791326f Change-Id: I391ecff9ed3f892b7c3adae0ceeb18930791326f --- trusty/storage/proxy/rpmb.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/trusty/storage/proxy/rpmb.c b/trusty/storage/proxy/rpmb.c index d1ed6493f..abeacdfed 100644 --- a/trusty/storage/proxy/rpmb.c +++ b/trusty/storage/proxy/rpmb.c @@ -212,6 +212,7 @@ static int send_ufs_rpmb_req(int sg_fd, const struct storage_rpmb_send_req* req) rc = ioctl(sg_fd, SG_IO, &io_hdr); if (rc < 0) { ALOGE("%s: ufs ioctl failed: %d, %s\n", __func__, rc, strerror(errno)); + goto err_op; } write_buf += req->reliable_write_size; } @@ -225,6 +226,7 @@ static int send_ufs_rpmb_req(int sg_fd, const struct storage_rpmb_send_req* req) rc = ioctl(sg_fd, SG_IO, &io_hdr); if (rc < 0) { ALOGE("%s: ufs ioctl failed: %d, %s\n", __func__, rc, strerror(errno)); + goto err_op; } write_buf += req->write_size; } @@ -240,6 +242,8 @@ static int send_ufs_rpmb_req(int sg_fd, const struct storage_rpmb_send_req* req) ALOGE("%s: ufs ioctl failed: %d, %s\n", __func__, rc, strerror(errno)); } } + +err_op: return rc; } From 440bad0bdd18e0c1cab8ab90b41829ee7277b708 Mon Sep 17 00:00:00 2001 From: Wenhao Wang Date: Thu, 15 Jul 2021 21:37:48 -0700 Subject: [PATCH 21/27] trusty:storageproxyd: Add wakelock to the UFS commands We add a wakelock to the sequence of UFS commands so that the sequence will not be disrrupted when devices get suspended. Bug: 193456223 Test: Trusty storage tests Merged-In: Ib90f8b284017cf261d2a2aea940834a42c21de02 Change-Id: Ib90f8b284017cf261d2a2aea940834a42c21de02 --- trusty/storage/proxy/Android.bp | 5 ++++- trusty/storage/proxy/rpmb.c | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/trusty/storage/proxy/Android.bp b/trusty/storage/proxy/Android.bp index a47143538..d67089fb2 100644 --- a/trusty/storage/proxy/Android.bp +++ b/trusty/storage/proxy/Android.bp @@ -29,7 +29,10 @@ cc_binary { "proxy.c", ], - shared_libs: ["liblog"], + shared_libs: [ + "liblog", + "libhardware_legacy", + ], header_libs: ["libcutils_headers"], static_libs: [ diff --git a/trusty/storage/proxy/rpmb.c b/trusty/storage/proxy/rpmb.c index abeacdfed..b59fb67f6 100644 --- a/trusty/storage/proxy/rpmb.c +++ b/trusty/storage/proxy/rpmb.c @@ -29,6 +29,8 @@ #include #include +#include + #include "ipc.h" #include "log.h" #include "rpmb.h" @@ -100,6 +102,8 @@ static int rpmb_fd = -1; static uint8_t read_buf[4096]; static enum dev_type dev_type = UNKNOWN_RPMB; +static const char* UFS_WAKE_LOCK_NAME = "ufs_seq_wakelock"; + #ifdef RPMB_DEBUG static void print_buf(const char* prefix, const uint8_t* buf, size_t size) { @@ -194,6 +198,7 @@ static int send_mmc_rpmb_req(int mmc_fd, const struct storage_rpmb_send_req* req static int send_ufs_rpmb_req(int sg_fd, const struct storage_rpmb_send_req* req) { int rc; + int wl_rc; const uint8_t* write_buf = req->payload; /* * Meaning of member values are stated on the definition of struct sec_proto_cdb. @@ -202,6 +207,12 @@ static int send_ufs_rpmb_req(int sg_fd, const struct storage_rpmb_send_req* req) struct sec_proto_cdb out_cdb = {0xB5, 0xEC, 0x00, 0x01, 0x00, 0x00, 0, 0x00, 0x00}; unsigned char sense_buffer[32]; + wl_rc = acquire_wake_lock(PARTIAL_WAKE_LOCK, UFS_WAKE_LOCK_NAME); + if (wl_rc < 0) { + ALOGE("%s: failed to acquire wakelock: %d, %s\n", __func__, wl_rc, strerror(errno)); + return wl_rc; + } + if (req->reliable_write_size) { /* Prepare SECURITY PROTOCOL OUT command. */ out_cdb.length = __builtin_bswap32(req->reliable_write_size); @@ -244,6 +255,11 @@ static int send_ufs_rpmb_req(int sg_fd, const struct storage_rpmb_send_req* req) } err_op: + wl_rc = release_wake_lock(UFS_WAKE_LOCK_NAME); + if (wl_rc < 0) { + ALOGE("%s: failed to release wakelock: %d, %s\n", __func__, wl_rc, strerror(errno)); + } + return rc; } From 830ea32e77292d4584977ac8162dcdbc92de613a Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 13 Jul 2021 12:17:17 +0100 Subject: [PATCH 22/27] TrustyKeyMint: support boot, vendor patchlevels - Invoke TrustyKeymaster::ConfigureVendorPatchlevel() from remote keymint Initialize(), using vendor patchlevel retrieved from property. - Add TrustyKeymaster::ConfigureVendorPatchlevel() method to send the CONFIGURE_VENDOR_PATCHLEVEL message. - Add message type values for CONFIGURE_{VENDOR,BOOT}_PATCHLEVEL messages. Bug: 193423844 Test: manual VTS test on device Merged-In: Ie42345112b08ef9c669535cef2de60ea77da15b4 Change-Id: Ie42345112b08ef9c669535cef2de60ea77da15b4 Ignore-AOSP-First: manual merge from aosp --- trusty/keymaster/TrustyKeymaster.cpp | 17 +++++++++++++++++ .../include/trusty_keymaster/TrustyKeymaster.h | 2 ++ .../trusty_keymaster/ipc/keymaster_ipc.h | 4 +++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/trusty/keymaster/TrustyKeymaster.cpp b/trusty/keymaster/TrustyKeymaster.cpp index aee33331a..cdfbd9003 100644 --- a/trusty/keymaster/TrustyKeymaster.cpp +++ b/trusty/keymaster/TrustyKeymaster.cpp @@ -79,6 +79,16 @@ int TrustyKeymaster::Initialize(KmVersion version) { return -1; } + // Set the vendor patchlevel to value retrieved from system property (which + // requires SELinux permission). + ConfigureVendorPatchlevelRequest vendor_req(message_version()); + vendor_req.vendor_patchlevel = GetVendorPatchlevel(); + ConfigureVendorPatchlevelResponse vendor_rsp = ConfigureVendorPatchlevel(vendor_req); + if (vendor_rsp.error != KM_ERROR_OK) { + LOG(ERROR) << "Failed to configure keymaster vendor patchlevel: " << vendor_rsp.error; + // Don't fail if this message isn't understood. + } + return 0; } @@ -262,4 +272,11 @@ DeviceLockedResponse TrustyKeymaster::DeviceLocked(const DeviceLockedRequest& re return response; } +ConfigureVendorPatchlevelResponse TrustyKeymaster::ConfigureVendorPatchlevel( + const ConfigureVendorPatchlevelRequest& request) { + ConfigureVendorPatchlevelResponse response(message_version()); + ForwardCommand(KM_CONFIGURE_VENDOR_PATCHLEVEL, request, &response); + return response; +} + } // namespace keymaster diff --git a/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h b/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h index 35eda459c..f80e02f37 100644 --- a/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h +++ b/trusty/keymaster/include/trusty_keymaster/TrustyKeymaster.h @@ -64,6 +64,8 @@ class TrustyKeymaster { GetVersion2Response GetVersion2(const GetVersion2Request& request); EarlyBootEndedResponse EarlyBootEnded(); DeviceLockedResponse DeviceLocked(const DeviceLockedRequest& request); + ConfigureVendorPatchlevelResponse ConfigureVendorPatchlevel( + const ConfigureVendorPatchlevelRequest& request); uint32_t message_version() const { return message_version_; } diff --git a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h index 17fee15f3..fa475ae90 100644 --- a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h +++ b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h @@ -58,6 +58,7 @@ enum keymaster_command : uint32_t { KM_DEVICE_LOCKED = (30 << KEYMASTER_REQ_SHIFT), KM_GENERATE_RKP_KEY = (31 << KEYMASTER_REQ_SHIFT), KM_GENERATE_CSR = (32 << KEYMASTER_REQ_SHIFT), + KM_CONFIGURE_VENDOR_PATCHLEVEL = (33 << KEYMASTER_REQ_SHIFT), // Bootloader/provisioning calls. KM_SET_BOOT_PARAMS = (0x1000 << KEYMASTER_REQ_SHIFT), @@ -71,7 +72,8 @@ enum keymaster_command : uint32_t { KM_SET_PRODUCT_ID = (0x9000 << KEYMASTER_REQ_SHIFT), KM_CLEAR_ATTESTATION_CERT_CHAIN = (0xa000 << KEYMASTER_REQ_SHIFT), KM_SET_WRAPPED_ATTESTATION_KEY = (0xb000 << KEYMASTER_REQ_SHIFT), - KM_SET_ATTESTATION_IDS = (0xc000 << KEYMASTER_REQ_SHIFT) + KM_SET_ATTESTATION_IDS = (0xc000 << KEYMASTER_REQ_SHIFT), + KM_CONFIGURE_BOOT_PATCHLEVEL = (0xd000 << KEYMASTER_REQ_SHIFT), }; #ifdef __ANDROID__ From 94b21c0c8cc10d59df6419dea2338945a14dfc8a Mon Sep 17 00:00:00 2001 From: Bowgo Tsai Date: Thu, 15 Jul 2021 10:13:33 +0000 Subject: [PATCH 23/27] Revert "Add systrace tag for system property" Revert "Add systrace tag for system property" Revert "Add systrace tag for system property" Revert "Adding system property tracing" Revert submission 1403568-sysprop_trace Reason for revert: makes property get/set non-reentrant Reverted Changes: I6f85f3f52:Add systrace tag for system property Id2b93acb2:Adding system property tracing Id78992d23:Add systrace tag for system property I1ba9fc7bd:Add systrace tag for system property Ignore-AOSP-First: b/193050299#comment17 Bug: 193050299 Change-Id: I9305003531c6a86194d55dc72c613337d213b53d Merged-In: I9305003531c6a86194d55dc72c613337d213b53d Test: build and boot a device (cherry picked from commit 18e0f65cbf86b141bd621a1adb0f384b99f86e29) --- libcutils/include/cutils/trace.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libcutils/include/cutils/trace.h b/libcutils/include/cutils/trace.h index ef426ff7b..24c6ae629 100644 --- a/libcutils/include/cutils/trace.h +++ b/libcutils/include/cutils/trace.h @@ -75,8 +75,7 @@ __BEGIN_DECLS #define ATRACE_TAG_AIDL (1<<24) #define ATRACE_TAG_NNAPI (1<<25) #define ATRACE_TAG_RRO (1<<26) -#define ATRACE_TAG_SYSPROP (1<<27) -#define ATRACE_TAG_LAST ATRACE_TAG_SYSPROP +#define ATRACE_TAG_LAST ATRACE_TAG_RRO // Reserved for initialization. #define ATRACE_TAG_NOT_READY (1ULL<<63) From 1fda6f1bf146238b3180fc4fb20fbb926762f0f0 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 22 Jul 2021 12:56:39 -0700 Subject: [PATCH 24/27] Revert "init: make reboot_on_failure not apply to manually stopped services" This reverts commit 1c51525f6686a97e32c166facad93fc97eacf0f9 because it accidentally made reboot_on_failure be a no-op for all services. This is because Reap() itself calls KillProcessGroup() on devices with a vendor level >= R, which in turn sets SVC_STOPPING. I had overlooked this somehow, probably because I didn't consider that a service can consist of multiple processes. It turns out that real FDE devices don't actually need the above commit because FDE devices aren't allowed to have updatable apexes enabled, and without updatable apexes enabled, apexd exits automatically and therefore doesn't have to be stopped. This can be verified by using the aosp_cf_x86_phone_noapex build target, rather than aosp_cf_x86_phone which I had used for testing before. So just revert it for now. Bug: 194370048 Change-Id: I90eddf2a87397449b241e5acaaa8d4a4241d73a9 (cherry picked from commit d14a178d01fd1690cf8c9f69dd8672b29f946a10) Merged-In: I90eddf2a87397449b241e5acaaa8d4a4241d73a9 --- init/README.md | 2 -- init/service.cpp | 10 +++------- init/service.h | 1 - 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/init/README.md b/init/README.md index 75dc32858..4a262c920 100644 --- a/init/README.md +++ b/init/README.md @@ -277,8 +277,6 @@ runs the service. CLD_EXITED or an status other than '0', reboot the system with the target specified in _target_. _target_ takes the same format as the parameter to sys.powerctl. This is particularly intended to be used with the `exec_start` builtin for any must-have checks during boot. - A service being stopped by init (e.g. using the `stop` or `class_reset` commands) is not - considered a failure for the purpose of this setting. `restart_period ` > If a non-oneshot service exits, it will be restarted at its start time plus diff --git a/init/service.cpp b/init/service.cpp index 5af81bf87..c3069f5b2 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -194,8 +194,6 @@ void Service::KillProcessGroup(int signal, bool report_oneshot) { << ") process group..."; int max_processes = 0; int r; - - flags_ |= SVC_STOPPING; if (signal == SIGTERM) { r = killProcessGroupOnce(proc_attr_.uid, pid_, signal, &max_processes); } else { @@ -279,8 +277,7 @@ void Service::Reap(const siginfo_t& siginfo) { f(siginfo); } - if ((siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) && on_failure_reboot_target_ && - !(flags_ & SVC_STOPPING)) { + if ((siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) && on_failure_reboot_target_) { LOG(ERROR) << "Service with 'reboot_on_failure' option failed, shutting down system."; trigger_shutdown(*on_failure_reboot_target_); } @@ -290,7 +287,7 @@ void Service::Reap(const siginfo_t& siginfo) { if (flags_ & SVC_TEMPORARY) return; pid_ = 0; - flags_ &= ~(SVC_RUNNING | SVC_STOPPING); + flags_ &= (~SVC_RUNNING); start_order_ = 0; // Oneshot processes go into the disabled state on exit, @@ -414,8 +411,7 @@ Result Service::Start() { bool disabled = (flags_ & (SVC_DISABLED | SVC_RESET)); // Starting a service removes it from the disabled or reset state and // immediately takes it out of the restarting state if it was in there. - flags_ &= (~(SVC_DISABLED | SVC_RESTARTING | SVC_RESET | SVC_RESTART | SVC_DISABLED_START | - SVC_STOPPING)); + flags_ &= (~(SVC_DISABLED|SVC_RESTARTING|SVC_RESET|SVC_RESTART|SVC_DISABLED_START)); // Running processes require no additional work --- if they're in the // process of exiting, we've ensured that they will immediately restart diff --git a/init/service.h b/init/service.h index 89b1f0970..043555fa4 100644 --- a/init/service.h +++ b/init/service.h @@ -54,7 +54,6 @@ // should not be killed during shutdown #define SVC_TEMPORARY 0x1000 // This service was started by 'exec' and should be removed from the // service list once it is reaped. -#define SVC_STOPPING 0x2000 // service is being stopped by init #define NR_SVC_SUPP_GIDS 12 // twelve supplementary groups From 533c2f6d55f3036d21fdddfaa81633e5850848cb Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Thu, 29 Jul 2021 00:55:14 +0000 Subject: [PATCH 25/27] Crash during OTA merge may lead to blocks with stale data This is a corner case wherein a crash during OTA merge can lead to missing of some COW operations to be merged thereby some blocks may end up with stale data. Fix here is to avoid any re-ordering of COW operations. Merge the COW operations as present in the COW file. New tests have been added to cow_snapuserd. Bug: 194955361 Test: cow_snapuserd_test, Incremental OTA Signed-off-by: Akilesh Kailash Merged-In: Id895fe7a3d6b4510676490a86d0caf62dec9b079 Change-Id: I14900b9537c4deb7824547e1dfe80f15274bdda4 Ignore-AOSP-First: manual merge from aosp --- fs_mgr/libsnapshot/cow_snapuserd_test.cpp | 262 ++++++++++++++++++++-- fs_mgr/libsnapshot/snapuserd.cpp | 88 ++------ 2 files changed, 259 insertions(+), 91 deletions(-) diff --git a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp index 3888eb168..d09c6e9c9 100644 --- a/fs_mgr/libsnapshot/cow_snapuserd_test.cpp +++ b/fs_mgr/libsnapshot/cow_snapuserd_test.cpp @@ -96,6 +96,8 @@ class TempDevice { class CowSnapuserdTest final { public: bool Setup(); + bool SetupOrderedOps(); + bool SetupOrderedOpsInverted(); bool SetupCopyOverlap_1(); bool SetupCopyOverlap_2(); bool Merge(); @@ -103,6 +105,9 @@ class CowSnapuserdTest final { void ReadSnapshotDeviceAndValidate(); void Shutdown(); void MergeInterrupt(); + void MergeInterruptFixed(int duration); + void MergeInterruptRandomly(int max_duration); + void ReadDmUserBlockWithoutDaemon(); std::string snapshot_dev() const { return snapshot_dev_->path(); } @@ -116,6 +121,8 @@ class CowSnapuserdTest final { void StartMerge(); void CreateCowDevice(); + void CreateCowDeviceOrderedOps(); + void CreateCowDeviceOrderedOpsInverted(); void CreateCowDeviceWithCopyOverlap_1(); void CreateCowDeviceWithCopyOverlap_2(); bool SetupDaemon(); @@ -196,6 +203,18 @@ bool CowSnapuserdTest::Setup() { return setup_ok_; } +bool CowSnapuserdTest::SetupOrderedOps() { + CreateBaseDevice(); + CreateCowDeviceOrderedOps(); + return SetupDaemon(); +} + +bool CowSnapuserdTest::SetupOrderedOpsInverted() { + CreateBaseDevice(); + CreateCowDeviceOrderedOpsInverted(); + return SetupDaemon(); +} + bool CowSnapuserdTest::SetupCopyOverlap_1() { CreateBaseDevice(); CreateCowDeviceWithCopyOverlap_1(); @@ -382,6 +401,112 @@ void CowSnapuserdTest::CreateCowDeviceWithCopyOverlap_1() { true); } +void CowSnapuserdTest::CreateCowDeviceOrderedOpsInverted() { + unique_fd rnd_fd; + loff_t offset = 0; + + std::string path = android::base::GetExecutableDirectory(); + cow_system_ = std::make_unique(path); + + rnd_fd.reset(open("/dev/random", O_RDONLY)); + ASSERT_TRUE(rnd_fd > 0); + + std::unique_ptr random_buffer_1_ = std::make_unique(size_); + + // Fill random data + for (size_t j = 0; j < (size_ / 1_MiB); j++) { + ASSERT_EQ(ReadFullyAtOffset(rnd_fd, (char*)random_buffer_1_.get() + offset, 1_MiB, 0), + true); + + offset += 1_MiB; + } + + CowOptions options; + options.compression = "gz"; + CowWriter writer(options); + + ASSERT_TRUE(writer.Initialize(cow_system_->fd)); + + size_t num_blocks = size_ / options.block_size; + size_t blk_end_copy = num_blocks * 2; + size_t source_blk = num_blocks - 1; + size_t blk_src_copy = blk_end_copy - 1; + + size_t x = num_blocks; + while (1) { + ASSERT_TRUE(writer.AddCopy(source_blk, blk_src_copy)); + x -= 1; + if (x == 0) { + break; + } + source_blk -= 1; + blk_src_copy -= 1; + } + + // Flush operations + ASSERT_TRUE(writer.Finalize()); + // Construct the buffer required for validation + orig_buffer_ = std::make_unique(total_base_size_); + // Read the entire base device + ASSERT_EQ(android::base::ReadFullyAtOffset(base_fd_, orig_buffer_.get(), total_base_size_, 0), + true); + // Merged Buffer + memmove(orig_buffer_.get(), (char*)orig_buffer_.get() + size_, size_); +} + +void CowSnapuserdTest::CreateCowDeviceOrderedOps() { + unique_fd rnd_fd; + loff_t offset = 0; + + std::string path = android::base::GetExecutableDirectory(); + cow_system_ = std::make_unique(path); + + rnd_fd.reset(open("/dev/random", O_RDONLY)); + ASSERT_TRUE(rnd_fd > 0); + + std::unique_ptr random_buffer_1_ = std::make_unique(size_); + + // Fill random data + for (size_t j = 0; j < (size_ / 1_MiB); j++) { + ASSERT_EQ(ReadFullyAtOffset(rnd_fd, (char*)random_buffer_1_.get() + offset, 1_MiB, 0), + true); + + offset += 1_MiB; + } + + CowOptions options; + options.compression = "gz"; + CowWriter writer(options); + + ASSERT_TRUE(writer.Initialize(cow_system_->fd)); + + size_t num_blocks = size_ / options.block_size; + size_t x = num_blocks; + size_t source_blk = 0; + size_t blk_src_copy = num_blocks; + + while (1) { + ASSERT_TRUE(writer.AddCopy(source_blk, blk_src_copy)); + + x -= 1; + if (x == 0) { + break; + } + source_blk += 1; + blk_src_copy += 1; + } + + // Flush operations + ASSERT_TRUE(writer.Finalize()); + // Construct the buffer required for validation + orig_buffer_ = std::make_unique(total_base_size_); + // Read the entire base device + ASSERT_EQ(android::base::ReadFullyAtOffset(base_fd_, orig_buffer_.get(), total_base_size_, 0), + true); + // Merged Buffer + memmove(orig_buffer_.get(), (char*)orig_buffer_.get() + size_, size_); +} + void CowSnapuserdTest::CreateCowDevice() { unique_fd rnd_fd; loff_t offset = 0; @@ -481,6 +606,36 @@ void CowSnapuserdTest::CreateDmUserDevice() { ASSERT_TRUE(android::fs_mgr::WaitForFile(misc_device, 10s)); } +void CowSnapuserdTest::ReadDmUserBlockWithoutDaemon() { + DmTable dmuser_table; + std::string dm_user_name = "dm-test-device"; + unique_fd fd; + + // Create a dm-user block device + ASSERT_TRUE(dmuser_table.AddTarget(std::make_unique(0, 123456, dm_user_name))); + ASSERT_TRUE(dmuser_table.valid()); + + dmuser_dev_ = std::make_unique(dm_user_name, dmuser_table); + ASSERT_TRUE(dmuser_dev_->valid()); + ASSERT_FALSE(dmuser_dev_->path().empty()); + + fd.reset(open(dmuser_dev_->path().c_str(), O_RDONLY)); + ASSERT_GE(fd, 0); + + std::unique_ptr buffer = std::make_unique(1_MiB); + + loff_t offset = 0; + // Every IO should fail as there is no daemon to process the IO + for (size_t j = 0; j < 10; j++) { + ASSERT_EQ(ReadFullyAtOffset(fd, (char*)buffer.get() + offset, BLOCK_SZ, offset), false); + + offset += BLOCK_SZ; + } + + fd = {}; + ASSERT_TRUE(dmuser_dev_->Destroy()); +} + void CowSnapuserdTest::InitDaemon() { bool ok = client_->AttachDmUser(system_device_ctrl_name_); ASSERT_TRUE(ok); @@ -566,6 +721,7 @@ void CowSnapuserdTest::ValidateMerge() { void CowSnapuserdTest::SimulateDaemonRestart() { Shutdown(); + std::this_thread::sleep_for(500ms); SetDeviceControlName(); StartSnapuserdDaemon(); InitCowDevice(); @@ -574,6 +730,34 @@ void CowSnapuserdTest::SimulateDaemonRestart() { CreateSnapshotDevice(); } +void CowSnapuserdTest::MergeInterruptRandomly(int max_duration) { + std::srand(std::time(nullptr)); + StartMerge(); + + for (int i = 0; i < 20; i++) { + int duration = std::rand() % max_duration; + std::this_thread::sleep_for(std::chrono::milliseconds(duration)); + SimulateDaemonRestart(); + StartMerge(); + } + + SimulateDaemonRestart(); + ASSERT_TRUE(Merge()); +} + +void CowSnapuserdTest::MergeInterruptFixed(int duration) { + StartMerge(); + + for (int i = 0; i < 25; i++) { + std::this_thread::sleep_for(std::chrono::milliseconds(duration)); + SimulateDaemonRestart(); + StartMerge(); + } + + SimulateDaemonRestart(); + ASSERT_TRUE(Merge()); +} + void CowSnapuserdTest::MergeInterrupt() { // Interrupt merge at various intervals StartMerge(); @@ -638,10 +822,9 @@ void CowSnapuserdMetadataTest::ValidatePartialFilledArea() { void* buffer = snapuserd_->GetExceptionBuffer(1); loff_t offset = 0; struct disk_exception* de; - for (int i = 0; i < 12; i++) { + for (int i = 11; i >= 0; i--) { de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, i); - ASSERT_EQ(de->new_chunk, new_chunk); offset += sizeof(struct disk_exception); new_chunk += 1; } @@ -780,71 +963,71 @@ void CowSnapuserdMetadataTest::ValidateMetadata() { de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 100); - ASSERT_EQ(de->new_chunk, 522); + ASSERT_EQ(de->new_chunk, 521); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 105); - ASSERT_EQ(de->new_chunk, 524); + ASSERT_EQ(de->new_chunk, 522); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 110); - ASSERT_EQ(de->new_chunk, 526); + ASSERT_EQ(de->new_chunk, 523); offset += sizeof(struct disk_exception); // The next 4 operations are batch merged as // both old and new chunk are contiguous de = reinterpret_cast((char*)buffer + offset); - ASSERT_EQ(de->old_chunk, 50); - ASSERT_EQ(de->new_chunk, 528); - offset += sizeof(struct disk_exception); - - de = reinterpret_cast((char*)buffer + offset); - ASSERT_EQ(de->old_chunk, 51); - ASSERT_EQ(de->new_chunk, 529); + ASSERT_EQ(de->old_chunk, 53); + ASSERT_EQ(de->new_chunk, 524); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 52); - ASSERT_EQ(de->new_chunk, 530); + ASSERT_EQ(de->new_chunk, 525); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); - ASSERT_EQ(de->old_chunk, 53); - ASSERT_EQ(de->new_chunk, 531); + ASSERT_EQ(de->old_chunk, 51); + ASSERT_EQ(de->new_chunk, 526); + offset += sizeof(struct disk_exception); + + de = reinterpret_cast((char*)buffer + offset); + ASSERT_EQ(de->old_chunk, 50); + ASSERT_EQ(de->new_chunk, 527); offset += sizeof(struct disk_exception); // This is handling overlap operation with // two batch merge operations. de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 18); - ASSERT_EQ(de->new_chunk, 533); + ASSERT_EQ(de->new_chunk, 528); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 19); - ASSERT_EQ(de->new_chunk, 534); + ASSERT_EQ(de->new_chunk, 529); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 20); - ASSERT_EQ(de->new_chunk, 535); + ASSERT_EQ(de->new_chunk, 530); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 21); - ASSERT_EQ(de->new_chunk, 537); + ASSERT_EQ(de->new_chunk, 532); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 22); - ASSERT_EQ(de->new_chunk, 538); + ASSERT_EQ(de->new_chunk, 533); offset += sizeof(struct disk_exception); de = reinterpret_cast((char*)buffer + offset); ASSERT_EQ(de->old_chunk, 23); - ASSERT_EQ(de->new_chunk, 539); + ASSERT_EQ(de->new_chunk, 534); offset += sizeof(struct disk_exception); // End of metadata @@ -909,6 +1092,43 @@ TEST(Snapuserd_Test, Snapshot_COPY_Overlap_Merge_Resume_TEST) { harness.Shutdown(); } +TEST(Snapuserd_Test, ReadDmUserBlockWithoutDaemon) { + CowSnapuserdTest harness; + harness.ReadDmUserBlockWithoutDaemon(); +} + +TEST(Snapuserd_Test, Snapshot_Merge_Crash_Fixed_Ordered) { + CowSnapuserdTest harness; + ASSERT_TRUE(harness.SetupOrderedOps()); + harness.MergeInterruptFixed(300); + harness.ValidateMerge(); + harness.Shutdown(); +} + +TEST(Snapuserd_Test, Snapshot_Merge_Crash_Random_Ordered) { + CowSnapuserdTest harness; + ASSERT_TRUE(harness.SetupOrderedOps()); + harness.MergeInterruptRandomly(500); + harness.ValidateMerge(); + harness.Shutdown(); +} + +TEST(Snapuserd_Test, Snapshot_Merge_Crash_Fixed_Inverted) { + CowSnapuserdTest harness; + ASSERT_TRUE(harness.SetupOrderedOpsInverted()); + harness.MergeInterruptFixed(50); + harness.ValidateMerge(); + harness.Shutdown(); +} + +TEST(Snapuserd_Test, Snapshot_Merge_Crash_Random_Inverted) { + CowSnapuserdTest harness; + ASSERT_TRUE(harness.SetupOrderedOpsInverted()); + harness.MergeInterruptRandomly(50); + harness.ValidateMerge(); + harness.Shutdown(); +} + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapuserd.cpp b/fs_mgr/libsnapshot/snapuserd.cpp index 03c2ef6c7..57a61a791 100644 --- a/fs_mgr/libsnapshot/snapuserd.cpp +++ b/fs_mgr/libsnapshot/snapuserd.cpp @@ -442,8 +442,9 @@ bool Snapuserd::ReadMetadata() { int num_ra_ops_per_iter = ((GetBufferDataSize()) / BLOCK_SZ); std::optional prev_id = {}; - std::map map; + std::vector vec; std::set dest_blocks; + std::set source_blocks; size_t pending_copy_ops = exceptions_per_area_ - num_ops; uint64_t total_copy_ops = reader_->total_copy_ops(); @@ -500,99 +501,45 @@ bool Snapuserd::ReadMetadata() { // scratch space and re-construct it thereby there // is no loss of data. // + // Note that we will follow the same order of COW operations + // as present in the COW file. This will make sure that\ + // the merge of operations are done based on the ops present + // in the file. //=========================================================== - // - // Case 2: - // - // Let's say we have three copy operations written to COW file - // in the following order: - // - // op-1: 15 -> 18 - // op-2: 16 -> 19 - // op-3: 17 -> 20 - // - // As aforementioned, kernel will initiate merge in reverse order. - // Hence, we will read these ops in reverse order so that all these - // ops are exectued in the same order as requested. Thus, we will - // read the metadata in reverse order and for the kernel it will - // look like: - // - // op-3: 17 -> 20 - // op-2: 16 -> 19 - // op-1: 15 -> 18 <-- Merge starts here in the kernel - // - // Now, this is problematic as kernel cannot batch merge them. - // - // Merge sequence will look like: - // - // Merge-1: op-1: 15 -> 18 - // Merge-2: op-2: 16 -> 19 - // Merge-3: op-3: 17 -> 20 - // - // We have three merge operations. - // - // Even though the blocks are contiguous, kernel can batch merge - // them if the blocks are in descending order. Update engine - // addresses this issue partially for overlapping operations as - // we see that op-1 to op-3 and op-4 to op-6 operatiosn are in - // descending order. However, if the copy operations are not - // overlapping, update engine cannot write these blocks - // in descending order. Hence, we will try to address it. - // Thus, we will send these blocks to the kernel and it will - // look like: - // - // op-3: 15 -> 18 - // op-2: 16 -> 19 - // op-1: 17 -> 20 <-- Merge starts here in the kernel - // - // Now with this change, we can batch merge all these three - // operations. Merge sequence will look like: - // - // Merge-1: {op-1: 17 -> 20, op-2: 16 -> 19, op-3: 15 -> 18} - // - // Note that we have changed the ordering of merge; However, this - // is ok as each of these copy operations are independent and there - // is no overlap. - // - //=================================================================== if (prev_id.has_value()) { - chunk_t diff = (cow_op->new_block > prev_id.value()) - ? (cow_op->new_block - prev_id.value()) - : (prev_id.value() - cow_op->new_block); - if (diff != 1) { - break; - } - - if (dest_blocks.count(cow_op->new_block) || map.count(cow_op->source) > 0) { + if (dest_blocks.count(cow_op->new_block) || source_blocks.count(cow_op->source)) { break; } } metadata_found = true; pending_copy_ops -= 1; - map[cow_op->new_block] = cow_op; + vec.push_back(cow_op); dest_blocks.insert(cow_op->source); + source_blocks.insert(cow_op->new_block); prev_id = cow_op->new_block; cowop_riter_->Next(); } while (!cowop_riter_->Done() && pending_copy_ops); data_chunk_id = GetNextAllocatableChunkId(data_chunk_id); - SNAP_LOG(DEBUG) << "Batch Merge copy-ops of size: " << map.size() + SNAP_LOG(DEBUG) << "Batch Merge copy-ops of size: " << vec.size() << " Area: " << vec_.size() << " Area offset: " << offset << " Pending-copy-ops in this area: " << pending_copy_ops; - for (auto it = map.begin(); it != map.end(); it++) { + for (size_t i = 0; i < vec.size(); i++) { struct disk_exception* de = reinterpret_cast((char*)de_ptr.get() + offset); - de->old_chunk = it->first; + const CowOperation* cow_op = vec[i]; + + de->old_chunk = cow_op->new_block; de->new_chunk = data_chunk_id; // Store operation pointer. - chunk_vec_.push_back(std::make_pair(ChunkToSector(data_chunk_id), it->second)); + chunk_vec_.push_back(std::make_pair(ChunkToSector(data_chunk_id), cow_op)); offset += sizeof(struct disk_exception); num_ops += 1; copy_ops++; if (read_ahead_feature_) { - read_ahead_ops_.push_back(it->second); + read_ahead_ops_.push_back(cow_op); } SNAP_LOG(DEBUG) << num_ops << ":" @@ -635,8 +582,9 @@ bool Snapuserd::ReadMetadata() { data_chunk_id = GetNextAllocatableChunkId(data_chunk_id); } } - map.clear(); + vec.clear(); dest_blocks.clear(); + source_blocks.clear(); prev_id.reset(); } From 84b4353790e8087a399a7ac8c3503f0bc2171d95 Mon Sep 17 00:00:00 2001 From: Tri Vo Date: Mon, 2 Aug 2021 14:57:38 -0700 Subject: [PATCH 26/27] trusty: storage: Allow starting without /data mounted Bug: 187105270 Test: m Change-Id: I3735e0752a6e502536000bd3102abda30cbd58fe Merged-In: I3735e0752a6e502536000bd3102abda30cbd58fe --- trusty/storage/proxy/storage.c | 1 - 1 file changed, 1 deletion(-) diff --git a/trusty/storage/proxy/storage.c b/trusty/storage/proxy/storage.c index 5b83e2141..2fde30f84 100644 --- a/trusty/storage/proxy/storage.c +++ b/trusty/storage/proxy/storage.c @@ -477,7 +477,6 @@ int storage_init(const char *dirname) if (ssdir_fd < 0) { ALOGE("failed to open ss root dir \"%s\": %s\n", dirname, strerror(errno)); - return -1; } ssdir_name = dirname; return 0; From 2cf268ab9fd5c4ac6ac4ce7d2ecead212f019fc0 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 13 May 2021 20:18:14 -0700 Subject: [PATCH 27/27] Use std::shared_ptr in Epoll's callback list. Ignore-AOSP-First: Awaiting security triage Bug: 187862380 Bug: 184569329 Test: CtsInitTestCases Change-Id: Ibb34a6b8a5675dbc515b7f8a43d7eecf2084510c (cherry picked from commit aea97815308ab98faf1599c16d6190b787d34941) --- init/Android.bp | 1 + init/epoll.cpp | 12 ++++--- init/epoll.h | 9 ++++-- init/epoll_test.cpp | 76 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 init/epoll_test.cpp diff --git a/init/Android.bp b/init/Android.bp index 827a8293f..4865694f3 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -239,6 +239,7 @@ cc_test { srcs: [ "devices_test.cpp", + "epoll_test.cpp", "firmware_handler_test.cpp", "init_test.cpp", "keychords_test.cpp", diff --git a/init/epoll.cpp b/init/epoll.cpp index 17d63fa5d..74d8aac96 100644 --- a/init/epoll.cpp +++ b/init/epoll.cpp @@ -38,11 +38,12 @@ Result Epoll::Open() { return {}; } -Result Epoll::RegisterHandler(int fd, std::function handler, uint32_t events) { +Result Epoll::RegisterHandler(int fd, Handler handler, uint32_t events) { if (!events) { return Error() << "Must specify events"; } - auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(handler)); + auto sp = std::make_shared(std::move(handler)); + auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(sp)); if (!inserted) { return Error() << "Cannot specify two epoll handlers for a given FD"; } @@ -69,7 +70,7 @@ Result Epoll::UnregisterHandler(int fd) { return {}; } -Result*>> Epoll::Wait( +Result>> Epoll::Wait( std::optional timeout) { int timeout_ms = -1; if (timeout && timeout->count() < INT_MAX) { @@ -81,9 +82,10 @@ Result*>> Epoll::Wait( if (num_events == -1) { return ErrnoError() << "epoll_wait failed"; } - std::vector*> pending_functions; + std::vector> pending_functions; for (int i = 0; i < num_events; ++i) { - pending_functions.emplace_back(reinterpret_cast*>(ev[i].data.ptr)); + auto sp = *reinterpret_cast*>(ev[i].data.ptr); + pending_functions.emplace_back(std::move(sp)); } return pending_functions; diff --git a/init/epoll.h b/init/epoll.h index c32a6614f..0df528935 100644 --- a/init/epoll.h +++ b/init/epoll.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -36,15 +37,17 @@ class Epoll { public: Epoll(); + typedef std::function Handler; + Result Open(); - Result RegisterHandler(int fd, std::function handler, uint32_t events = EPOLLIN); + Result RegisterHandler(int fd, Handler handler, uint32_t events = EPOLLIN); Result UnregisterHandler(int fd); - Result*>> Wait( + Result>> Wait( std::optional timeout); private: android::base::unique_fd epoll_fd_; - std::map> epoll_handlers_; + std::map> epoll_handlers_; }; } // namespace init diff --git a/init/epoll_test.cpp b/init/epoll_test.cpp new file mode 100644 index 000000000..9236cd53e --- /dev/null +++ b/init/epoll_test.cpp @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2021 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 "epoll.h" + +#include + +#include + +#include +#include + +namespace android { +namespace init { + +std::unordered_set sValidObjects; + +class CatchDtor final { + public: + CatchDtor() { sValidObjects.emplace(this); } + CatchDtor(const CatchDtor&) { sValidObjects.emplace(this); } + ~CatchDtor() { + auto iter = sValidObjects.find(this); + if (iter != sValidObjects.end()) { + sValidObjects.erase(iter); + } + } +}; + +TEST(epoll, UnregisterHandler) { + Epoll epoll; + ASSERT_RESULT_OK(epoll.Open()); + + int fds[2]; + ASSERT_EQ(pipe(fds), 0); + + CatchDtor catch_dtor; + bool handler_invoked; + auto handler = [&, catch_dtor]() -> void { + auto result = epoll.UnregisterHandler(fds[0]); + ASSERT_EQ(result.ok(), !handler_invoked); + handler_invoked = true; + ASSERT_NE(sValidObjects.find((void*)&catch_dtor), sValidObjects.end()); + }; + + epoll.RegisterHandler(fds[0], std::move(handler)); + + uint8_t byte = 0xee; + ASSERT_TRUE(android::base::WriteFully(fds[1], &byte, sizeof(byte))); + + auto results = epoll.Wait({}); + ASSERT_RESULT_OK(results); + ASSERT_EQ(results->size(), size_t(1)); + + for (const auto& function : *results) { + (*function)(); + (*function)(); + } + ASSERT_TRUE(handler_invoked); +} + +} // namespace init +} // namespace android