From 7c4beee9f9ea17fb896ebcdd4789c60ccfb17fab Mon Sep 17 00:00:00 2001 From: Michael Hoisie Date: Fri, 12 Jan 2024 19:18:19 +0000 Subject: [PATCH 001/158] Add support for ashmem-host for host Windows Migrate to tmpfile and fileno for temp file operations. These calls are supported on MinGW, and the temp files are automatically cleaned up. A Windows variant of ashmem-host is needed to support CursorWindows on host Windows. In Windows, it is not possible to unlink an open file, so the nlink check in ashmem_validate_stat must be made Unix-only. Test: SQLiteDatabaseTest in Google3 Test: libcutils_test_static on Windows Bug: 317884162 Change-Id: I7fc0f1f49406b01549b7f4d7e138cb3e4d79be72 --- libcutils/Android.bp | 39 +++++++++++---------- libcutils/ashmem-host.cpp | 55 ++++++++++++++++++----------- libcutils/ashmem_base_test.cpp | 63 ++++++++++++++++++++++++++++++++++ libcutils/ashmem_test.cpp | 22 ------------ libcutils/sockets_test.cpp | 2 -- 5 files changed, 119 insertions(+), 62 deletions(-) create mode 100644 libcutils/ashmem_base_test.cpp diff --git a/libcutils/Android.bp b/libcutils/Android.bp index b7752d902..862af841c 100644 --- a/libcutils/Android.bp +++ b/libcutils/Android.bp @@ -156,23 +156,18 @@ cc_library { "fs_config.cpp", ], }, - not_windows: { - srcs: libcutils_nonwindows_sources + [ - "ashmem-host.cpp", - "trace-host.cpp", - ], - }, - windows: { - host_ldlibs: ["-lws2_32"], - + host: { srcs: [ "trace-host.cpp", + "ashmem-host.cpp", ], - + }, + not_windows: { + srcs: libcutils_nonwindows_sources, + }, + windows: { enabled: true, - cflags: [ - "-D_GNU_SOURCE", - ], + host_ldlibs: ["-lws2_32"], }, android: { sanitize: { @@ -241,6 +236,7 @@ cc_library { cc_defaults { name: "libcutils_test_default", srcs: [ + "ashmem_base_test.cpp", "native_handle_test.cpp", "properties_test.cpp", "sockets_test.cpp", @@ -299,20 +295,26 @@ cc_test { cc_defaults { name: "libcutils_test_static_defaults", defaults: ["libcutils_test_default"], - static_libs: [ - "libc", - "libcgrouprc_format", - ] + test_libraries + always_static_test_libraries, stl: "libc++_static", require_root: true, target: { android: { static_executable: true, + static_libs: [ + "libcgrouprc_format", + ] + test_libraries + always_static_test_libraries, + }, + not_windows: { + static_libs: test_libraries + always_static_test_libraries, }, windows: { + static_libs: [ + "libbase", + "libcutils", + "libcutils_sockets", + ], host_ldlibs: ["-lws2_32"], - enabled: true, }, }, @@ -320,6 +322,7 @@ cc_defaults { cc_test { name: "libcutils_test_static", + host_supported: true, test_suites: ["device-tests"], defaults: ["libcutils_test_static_defaults"], } diff --git a/libcutils/ashmem-host.cpp b/libcutils/ashmem-host.cpp index 2ba1eb0c3..9003b76c9 100644 --- a/libcutils/ashmem-host.cpp +++ b/libcutils/ashmem-host.cpp @@ -17,10 +17,13 @@ #include /* - * Implementation of the user-space ashmem API for the simulator, which lacks - * an ashmem-enabled kernel. See ashmem-dev.c for the real ashmem-based version. + * Implementation of the user-space ashmem API for the simulator, which lacks an + * ashmem-enabled kernel. See ashmem-dev.c for the real ashmem-based version. A + * disk-backed temp file is the best option that is consistently supported + * across all host platforms. */ +#include #include #include #include @@ -31,8 +34,10 @@ #include #include #include - #include +#include + +using android::base::unique_fd; static bool ashmem_validate_stat(int fd, struct stat* buf) { int result = fstat(fd, buf); @@ -40,15 +45,20 @@ static bool ashmem_validate_stat(int fd, struct stat* buf) { return false; } - /* - * Check if this is an "ashmem" region. - * TODO: This is very hacky, and can easily break. - * We need some reliable indicator. - */ - if (!(buf->st_nlink == 0 && S_ISREG(buf->st_mode))) { + // Check if this is an ashmem region. Since there's no such thing on the host, + // we can't actually implement that. Check that it's at least a regular file. + if (!S_ISREG(buf->st_mode)) { errno = ENOTTY; return false; } + // In Win32, unlike Unix, the temp file is not unlinked immediately after + // creation. +#if !defined(_WIN32) + if (buf->st_nlink != 0) { + errno = ENOTTY; + return false; + } +#endif return true; } @@ -58,19 +68,24 @@ int ashmem_valid(int fd) { } int ashmem_create_region(const char* /*ignored*/, size_t size) { - char pattern[PATH_MAX]; - snprintf(pattern, sizeof(pattern), "/tmp/android-ashmem-%d-XXXXXXXXX", getpid()); - int fd = mkstemp(pattern); - if (fd == -1) return -1; + // Files returned by tmpfile are automatically removed. + std::unique_ptr tmp(tmpfile(), &fclose); - unlink(pattern); - - if (TEMP_FAILURE_RETRY(ftruncate(fd, size)) == -1) { - close(fd); - return -1; + if (!tmp) { + return -1; } - - return fd; + int fd = fileno(tmp.get()); + if (fd == -1) { + return -1; + } + unique_fd dupfd = unique_fd(dup(fd)); + if (dupfd == -1) { + return -1; + } + if (TEMP_FAILURE_RETRY(ftruncate(dupfd, size)) == -1) { + return -1; + } + return dupfd.release(); } int ashmem_set_prot_region(int /*fd*/, int /*prot*/) { diff --git a/libcutils/ashmem_base_test.cpp b/libcutils/ashmem_base_test.cpp new file mode 100644 index 000000000..c9b14e583 --- /dev/null +++ b/libcutils/ashmem_base_test.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2024 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 + +/* + * Tests in AshmemBaseTest are designed to run on Android as well as host + * platforms (Linux, Mac, Windows). + */ + +#if defined(_WIN32) +static inline size_t getpagesize() { + return 4096; +} +#endif + +using android::base::unique_fd; + +TEST(AshmemBaseTest, BasicTest) { + const size_t size = getpagesize(); + std::vector data(size); + std::generate(data.begin(), data.end(), [n = 0]() mutable { return n++ & 0xFF; }); + + unique_fd fd = unique_fd(ashmem_create_region(nullptr, size)); + ASSERT_TRUE(fd >= 0); + ASSERT_TRUE(ashmem_valid(fd)); + ASSERT_EQ(size, static_cast(ashmem_get_size_region(fd))); + + std::unique_ptr mapped = + android::base::MappedFile::FromFd(fd, 0, size, PROT_READ | PROT_WRITE); + EXPECT_TRUE(mapped.get() != nullptr); + void* region1 = mapped->data(); + EXPECT_TRUE(region1 != nullptr); + + memcpy(region1, data.data(), size); + ASSERT_EQ(0, memcmp(region1, data.data(), size)); + + std::unique_ptr mapped2 = + android::base::MappedFile::FromFd(fd, 0, size, PROT_READ | PROT_WRITE); + EXPECT_TRUE(mapped2.get() != nullptr); + void* region2 = mapped2->data(); + EXPECT_TRUE(region2 != nullptr); + ASSERT_EQ(0, memcmp(region2, data.data(), size)); +} diff --git a/libcutils/ashmem_test.cpp b/libcutils/ashmem_test.cpp index 571b41000..ccbb8c977 100644 --- a/libcutils/ashmem_test.cpp +++ b/libcutils/ashmem_test.cpp @@ -69,28 +69,6 @@ void FillData(std::vector& data) { } } -TEST(AshmemTest, BasicTest) { - const size_t size = getpagesize(); - std::vector data(size); - FillData(data); - - unique_fd fd; - ASSERT_NO_FATAL_FAILURE(TestCreateRegion(size, fd, PROT_READ | PROT_WRITE)); - - void* region1 = nullptr; - ASSERT_NO_FATAL_FAILURE(TestMmap(fd, size, PROT_READ | PROT_WRITE, ®ion1)); - - memcpy(region1, data.data(), size); - ASSERT_EQ(0, memcmp(region1, data.data(), size)); - - EXPECT_EQ(0, munmap(region1, size)); - - void *region2; - ASSERT_NO_FATAL_FAILURE(TestMmap(fd, size, PROT_READ, ®ion2)); - ASSERT_EQ(0, memcmp(region2, data.data(), size)); - EXPECT_EQ(0, munmap(region2, size)); -} - TEST(AshmemTest, ForkTest) { const size_t size = getpagesize(); std::vector data(size); diff --git a/libcutils/sockets_test.cpp b/libcutils/sockets_test.cpp index 1fa40bc6a..f6f9c3628 100644 --- a/libcutils/sockets_test.cpp +++ b/libcutils/sockets_test.cpp @@ -19,8 +19,6 @@ // should be the case for loopback communication, but is not guaranteed. #include -#include -#include #include #include From 3b795a1a7b6b17a3c593ff01bab2ad209552faf1 Mon Sep 17 00:00:00 2001 From: Henri Chataing Date: Thu, 13 Jun 2024 22:47:03 +0000 Subject: [PATCH 002/158] Make libexpresslog apex available Test: TreeHugger Change-Id: I10267c3a57d8a6553f9ef74d09b1cd81c6947a46 --- libstats/expresslog/Android.bp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libstats/expresslog/Android.bp b/libstats/expresslog/Android.bp index 004f8b9bf..96ab59b10 100644 --- a/libstats/expresslog/Android.bp +++ b/libstats/expresslog/Android.bp @@ -47,6 +47,11 @@ cc_library { "libstatssocket", ], export_include_dirs: ["include"], + min_sdk_version: "33", + apex_available: [ + "//apex_available:platform", + "com.android.btservices", + ], } genrule { @@ -75,6 +80,11 @@ cc_library_static { shared_libs: [ "libstatssocket", ], + min_sdk_version: "33", + apex_available: [ + "//apex_available:platform", + "com.android.btservices", + ], } cc_test { From 064ac0bf15e5b8f29481c0b4a795759aefb08d03 Mon Sep 17 00:00:00 2001 From: Nelson Li Date: Mon, 17 Jun 2024 05:02:56 +0000 Subject: [PATCH 003/158] Convert `init_vendor` to Android.bp `init_first_stage` is a dependency of `init_vendor` only when `BOARD_USES_RECOVERY_AS_BOOT` is false. Since `BOARD_USES_RECOVERY_AS_BOOT` is already defined in `build/make/core/android_soong_config_vars.mk` within a soong_namespace, we can use the `soong_config_module_type` to easily convert this to Android.bp. Bug: 347600829 Test: m init_vendor Change-Id: I1ddcd5fb62983b01e51452c9b7367750e03e7f48 --- init/Android.bp | 22 +++++++++++++++++++++- init/Android.mk | 16 ---------------- 2 files changed, 21 insertions(+), 17 deletions(-) delete mode 100644 init/Android.mk diff --git a/init/Android.bp b/init/Android.bp index 57e5a681a..6526a934e 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -162,7 +162,7 @@ libinit_cc_defaults { }, release_write_appcompat_override_system_properties: { cflags: ["-DWRITE_APPCOMPAT_OVERRIDE_SYSTEM_PROPERTIES"], - } + }, }, static_libs: [ "libavb", @@ -663,3 +663,23 @@ sh_binary { src: "extra_free_kbytes.sh", filename_from_src: true, } + +soong_config_module_type { + name: "board_use_recovery_as_boot_phony", + module_type: "phony", + config_namespace: "ANDROID", + bool_variables: ["BOARD_USES_RECOVERY_AS_BOOT"], + properties: ["required"], +} + +board_use_recovery_as_boot_phony { + name: "init_vendor", + soong_config_variables: { + BOARD_USES_RECOVERY_AS_BOOT: { + required: [], + conditions_default: { + required: ["init_first_stage"], + }, + }, + }, +} diff --git a/init/Android.mk b/init/Android.mk deleted file mode 100644 index 4b85c15ea..000000000 --- a/init/Android.mk +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2005 The Android Open Source Project - -LOCAL_PATH:= $(call my-dir) - -include $(CLEAR_VARS) - -LOCAL_MODULE := init_vendor -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_NOTICE_FILE := $(LOCAL_PATH)/NOTICE -ifneq ($(BOARD_USES_RECOVERY_AS_BOOT),true) -LOCAL_REQUIRED_MODULES := \ - init_first_stage \ - -endif # BOARD_USES_RECOVERY_AS_BOOT -include $(BUILD_PHONY_PACKAGE) From 14fbf6d3909dab706568d1c409a941d5fd2a7428 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 17 Jun 2024 19:05:13 +0000 Subject: [PATCH 004/158] Revert "snapuserd: Don't statically link outside of ramdisk." This reverts commit c9fa93f4e892ea06c0c4f8a5270a90161ae4516d. Reason for revert: b/347670914 Bug: 347670914 Change-Id: I9d63a69ccf1f8de98ab7cc23b9fbf400863cddfb --- fs_mgr/libsnapshot/snapuserd/Android.bp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp index f04020549..649309de1 100644 --- a/fs_mgr/libsnapshot/snapuserd/Android.bp +++ b/fs_mgr/libsnapshot/snapuserd/Android.bp @@ -145,6 +145,14 @@ cc_defaults { ], include_dirs: ["bionic/libc/kernel"], + system_shared_libs: [], + + // snapuserd is started during early boot by first-stage init. At that + // point, /system is mounted using the "dm-user" device-mapper kernel + // module. dm-user routes all I/O to userspace to be handled by + // snapuserd, which would lead to deadlock if we had to handle page + // faults for its code pages. + static_executable: true, } cc_binary { @@ -157,10 +165,10 @@ cc_binary { "libsnapuserd_client", ], ramdisk_available: false, - vendor_ramdisk_available: false, + vendor_ramdisk_available: true, } -// This target will install to /system/bin/snapuserd_ramdisk +// This target will install to /system/bin/snapuserd_ramdisk // It will also create a symblink on /system/bin/snapuserd that point to // /system/bin/snapuserd_ramdisk . // This way, init can check if generic ramdisk copy exists. @@ -176,15 +184,6 @@ cc_binary { vendor_ramdisk_available: false, ramdisk: true, symlinks: ["snapuserd"], - - system_shared_libs: [], - - // snapuserd is started during early boot by first-stage init. At that - // point, /system is mounted using the "dm-user" device-mapper kernel - // module. dm-user routes all I/O to userspace to be handled by - // snapuserd, which would lead to deadlock if we had to handle page - // faults for its code pages. - static_executable: true, } cc_defaults { From 00e1b61bb92b7a9905fdb5c00aba9ffd47a36dbf Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Tue, 28 May 2024 18:25:12 -0700 Subject: [PATCH 005/158] reupload: libsnapshot: set thread priority It looks like we can't use as type with specifying size Read merge thread + worker thread priority from build configurations. In the case of low memory devices, a lower priority will reduce CPU utilization post OTA reboot. Test: th Change-Id: Ie895be32e3aea66b46503c5270939bb42f58494a --- .../libsnapshot/snapuserd/user-space-merge/merge_worker.cpp | 4 +++- .../libsnapshot/snapuserd/user-space-merge/read_worker.cpp | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp index bd7eaca74..4afc312e3 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp @@ -577,8 +577,10 @@ bool MergeWorker::Run() { SNAP_LOG(ERROR) << "Merge terminated early..."; return true; } + auto merge_thread_priority = android::base::GetUintProperty( + "ro.virtual_ab.merge_thread_priority", ANDROID_PRIORITY_BACKGROUND); - if (!SetThreadPriority(ANDROID_PRIORITY_BACKGROUND)) { + if (!SetThreadPriority(merge_thread_priority)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp index d40b6d11d..7191f1f74 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp @@ -17,8 +17,10 @@ #include #include +#include "android-base/properties.h" #include "read_worker.h" #include "snapuserd_core.h" +#include "user-space-merge/worker.h" #include "utility.h" namespace android { @@ -259,8 +261,10 @@ bool ReadWorker::Run() { SNAP_LOG(INFO) << "Processing snapshot I/O requests...."; pthread_setname_np(pthread_self(), "ReadWorker"); + auto worker_thread_priority = android::base::GetUintProperty( + "ro.virtual_ab.worker_thread_priority", ANDROID_PRIORITY_NORMAL); - if (!SetThreadPriority(ANDROID_PRIORITY_NORMAL)) { + if (!SetThreadPriority(worker_thread_priority)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } From c2ae951c7d6e51a8fc6db3c3f7be34064e94bd6e Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Mon, 3 Jun 2024 21:49:24 +0800 Subject: [PATCH 006/158] fs_mgr_overlayfs: Don't preemptively change mount propagation type If the overlaid mount have no submount and is not submount of other mount, then we don't need to change its propagation type. This makes the remount process less disruptive as we create less fragmented mount namespaces. The general rule is still to always reboot after initial remount setup to get the most predictable result. Bug: 342967841 Test: adb_remount test Change-Id: I19ca62018149f1fa81186824a9d7f1c32d9d6008 --- fs_mgr/fs_mgr_overlayfs_mount.cpp | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/fs_mgr/fs_mgr_overlayfs_mount.cpp b/fs_mgr/fs_mgr_overlayfs_mount.cpp index a1ec63b69..bd0fcfd98 100644 --- a/fs_mgr/fs_mgr_overlayfs_mount.cpp +++ b/fs_mgr/fs_mgr_overlayfs_mount.cpp @@ -412,6 +412,8 @@ static bool fs_mgr_overlayfs_mount_one(const FstabEntry& fstab_entry) { bool retval = true; bool move_dir_shared = true; bool parent_shared = true; + bool parent_have_parent = false; + bool parent_made_private = false; bool root_shared = true; bool root_made_private = false; @@ -443,6 +445,10 @@ static bool fs_mgr_overlayfs_mount_one(const FstabEntry& fstab_entry) { if (entry.mount_point == "/") { root_shared = entry.shared_flag; } + // Ignore "/" as we don't overlay "/" directly. + if (entry.mount_point != "/") { + parent_have_parent |= android::base::StartsWith(mount_point, entry.mount_point + "/"); + } } // Precondition is that kMoveMountTempDir is MS_PRIVATE, otherwise don't try to move any @@ -453,11 +459,13 @@ static bool fs_mgr_overlayfs_mount_one(const FstabEntry& fstab_entry) { // Need to make the original mountpoint MS_PRIVATE, so that the overlayfs can be MS_MOVE. // This could happen if its parent mount is remounted later. - if (!fs_mgr_overlayfs_set_shared_mount(mount_point, false)) { - // If failed to set "/system" mount type, it might be due to "/system" not being a valid - // mountpoint after switch root. Retry with "/" in this case. - if (errno == EINVAL && mount_point == "/system") { - root_made_private = fs_mgr_overlayfs_set_shared_mount("/", false); + if (parent_have_parent) { + parent_made_private |= fs_mgr_overlayfs_set_shared_mount(mount_point, false); + if (!parent_made_private && errno == EINVAL && mount_point == "/system") { + // If failed to set "/system" mount type, it might be due to "/system" not being a valid + // mountpoint after switch root. Retry with "/" in this case. + parent_made_private |= fs_mgr_overlayfs_set_shared_mount("/", false); + root_made_private |= parent_made_private; } } @@ -496,6 +504,15 @@ static bool fs_mgr_overlayfs_mount_one(const FstabEntry& fstab_entry) { continue; } } + if (!parent_made_private) { + parent_made_private |= fs_mgr_overlayfs_set_shared_mount(mount_point, false); + if (!parent_made_private && errno == EINVAL && mount_point == "/system") { + // If failed to set "/system" mount type, it might be due to "/system" not being a + // valid mountpoint after switch root. Retry with "/" in this case. + parent_made_private |= fs_mgr_overlayfs_set_shared_mount("/", false); + root_made_private |= parent_made_private; + } + } if (new_entry.shared_flag) { new_entry.shared_flag = fs_mgr_overlayfs_set_shared_mount(new_entry.mount_point, false); @@ -524,7 +541,7 @@ static bool fs_mgr_overlayfs_mount_one(const FstabEntry& fstab_entry) { rmdir(entry.dir.c_str()); } // If the original (overridden) mount was MS_SHARED, then set the overlayfs mount to MS_SHARED. - if (parent_shared) { + if (parent_shared && parent_made_private) { fs_mgr_overlayfs_set_shared_mount(mount_point, true); } if (root_shared && root_made_private) { From 28b37f27048679874f1e1185fe9f852d36872e73 Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Fri, 7 Jun 2024 18:34:19 +0000 Subject: [PATCH 007/158] libprocessgroup: Add MaxActivationDepth Cgroup v2 controllers can be enabled in a subtree of the shared hierarchy. That allows users to limit the number of cgroups with a controller enabled to less than the total number of cgroups. [1] There are costs for each cgroup. Kernel memory is used for each cgroup, plus additional memory for each active controller in each cgroup. Some kernel operations scale with the number of cgroups (with a given controller enabled), so it can be desirable to minimize the number of cgroups with that controller enabled. This change allows each v2 controller configuration to specify a maximum activation depth, past which the controller will not be activated deeper in the Android cgroup v2 hierarchy. The hierarchy root is defined as depth 0. MaxActivationDepth is the field name for this purpose for controllers in the Controllers array under Cgroups2 in cgroups.json. Here are two examples: "MaxActivationDepth": 1 This will activate the controller in every per-application cgroup, but not in the per-process cgroups below. /sys/fs/cgroup depth=0 active=true (controller listed in cgroup.subtree_control) /sys/fs/cgroup/uid_0 depth=1 active=true (controller NOT listed in cgroup.subtree_control) /sys/fs/cgroup/uid_0/pid_100 depth=2 active=false (controller NOT listed in cgroup.subtree_control) This can also be used with PRODUCT_CGROUP_V2_SYS_APP_ISOLATION_ENABLED := true. "MaxActivationDepth": 1 This will activate the controller only at the app / system level, but not in per-application cgroups below. This results in a total of only 3 cgroups with the controller enabled (root, apps, system). /sys/fs/cgroup depth=0 active=true (controller listed in cgroup.subtree_control) /sys/fs/cgroup/apps depth=1 active=true (controller NOT listed in cgroup.subtree_control) /sys/fs/cgroup/apps/uid_10000 depth=2 active=false (controller NOT listed in cgroup.subtree_control) /sys/fs/cgroup/apps/uid_10000/pid_100 depth=3 active=false (controller NOT listed in cgroup.subtree_control) [1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#enabling-and-disabling Bug: 346584259 Test: Cuttlefish with memcg v2 Change-Id: I62109ea935261c51fc30b2054c4d28d0360f7985 --- libprocessgroup/Android.bp | 1 + libprocessgroup/cgroup_map.cpp | 9 +- .../cgrouprc/cgroup_controller.cpp | 5 + .../cgrouprc/include/android/cgrouprc.h | 8 ++ libprocessgroup/cgrouprc/libcgrouprc.map.txt | 7 ++ .../cgrouprc_format/cgroup_controller.cpp | 10 +- .../processgroup/format/cgroup_controller.h | 4 +- libprocessgroup/profiles/cgroups.proto | 3 +- libprocessgroup/setup/Android.bp | 1 + libprocessgroup/setup/cgroup_descriptor.h | 3 +- libprocessgroup/setup/cgroup_map_write.cpp | 25 +++- libprocessgroup/util/Android.bp | 47 ++++++++ libprocessgroup/util/OWNERS | 3 + libprocessgroup/util/TEST_MAPPING | 7 ++ .../util/include/processgroup/util.h | 61 ++++++++++ libprocessgroup/util/tests/util.cpp | 109 ++++++++++++++++++ 16 files changed, 290 insertions(+), 13 deletions(-) create mode 100644 libprocessgroup/util/Android.bp create mode 100644 libprocessgroup/util/OWNERS create mode 100644 libprocessgroup/util/TEST_MAPPING create mode 100644 libprocessgroup/util/include/processgroup/util.h create mode 100644 libprocessgroup/util/tests/util.cpp diff --git a/libprocessgroup/Android.bp b/libprocessgroup/Android.bp index d40be9f9e..33e00bc5d 100644 --- a/libprocessgroup/Android.bp +++ b/libprocessgroup/Android.bp @@ -84,6 +84,7 @@ cc_library { header_libs: [ "libcutils_headers", "libprocessgroup_headers", + "libprocessgroup_util", ], export_include_dirs: ["include"], export_header_lib_headers: [ diff --git a/libprocessgroup/cgroup_map.cpp b/libprocessgroup/cgroup_map.cpp index ebc059944..52b5afe5e 100644 --- a/libprocessgroup/cgroup_map.cpp +++ b/libprocessgroup/cgroup_map.cpp @@ -28,6 +28,7 @@ #include #include #include +#include using android::base::StartsWith; using android::base::StringPrintf; @@ -216,7 +217,13 @@ int CgroupMap::ActivateControllers(const std::string& path) const { for (uint32_t i = 0; i < controller_count; ++i) { const ACgroupController* controller = ACgroupFile_getController(i); const uint32_t flags = ACgroupController_getFlags(controller); - if (flags & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION) { + uint32_t max_activation_depth = UINT32_MAX; + if (__builtin_available(android 36, *)) { + max_activation_depth = ACgroupController_getMaxActivationDepth(controller); + } + const int depth = util::GetCgroupDepth(ACgroupController_getPath(controller), path); + + if (flags & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION && depth < max_activation_depth) { std::string str("+"); str.append(ACgroupController_getName(controller)); if (!WriteStringToFile(str, path + "/cgroup.subtree_control")) { diff --git a/libprocessgroup/cgrouprc/cgroup_controller.cpp b/libprocessgroup/cgrouprc/cgroup_controller.cpp index 5a326e55d..889b3becf 100644 --- a/libprocessgroup/cgrouprc/cgroup_controller.cpp +++ b/libprocessgroup/cgrouprc/cgroup_controller.cpp @@ -32,6 +32,11 @@ uint32_t ACgroupController_getFlags(const ACgroupController* controller) { return controller->flags(); } +uint32_t ACgroupController_getMaxActivationDepth(const ACgroupController* controller) { + CHECK(controller != nullptr); + return controller->max_activation_depth(); +} + const char* ACgroupController_getName(const ACgroupController* controller) { CHECK(controller != nullptr); return controller->name(); diff --git a/libprocessgroup/cgrouprc/include/android/cgrouprc.h b/libprocessgroup/cgrouprc/include/android/cgrouprc.h index e704a36aa..3a57df547 100644 --- a/libprocessgroup/cgrouprc/include/android/cgrouprc.h +++ b/libprocessgroup/cgrouprc/include/android/cgrouprc.h @@ -78,6 +78,14 @@ __attribute__((warn_unused_result)) uint32_t ACgroupController_getVersion(const __attribute__((warn_unused_result, weak)) uint32_t ACgroupController_getFlags( const ACgroupController*) __INTRODUCED_IN(30); +/** + * Returns the maximum activation depth of the given controller. + * Only applicable to cgroup v2 controllers. + * Returns UINT32_MAX if no maximum activation depth is set. + */ +__attribute__((warn_unused_result, weak)) uint32_t ACgroupController_getMaxActivationDepth( + const ACgroupController* controller) __INTRODUCED_IN(36); + /** * Returns the name of the given controller. * If the given controller is null, return nullptr. diff --git a/libprocessgroup/cgrouprc/libcgrouprc.map.txt b/libprocessgroup/cgrouprc/libcgrouprc.map.txt index b62b10f3b..30bd25f18 100644 --- a/libprocessgroup/cgrouprc/libcgrouprc.map.txt +++ b/libprocessgroup/cgrouprc/libcgrouprc.map.txt @@ -16,3 +16,10 @@ LIBCGROUPRC_30 { # introduced=30 local: *; }; + +LIBCGROUPRC_36 { # introduced=36 + global: + ACgroupController_getMaxActivationDepth; # llndk=202504 systemapi + local: + *; +}; diff --git a/libprocessgroup/cgrouprc_format/cgroup_controller.cpp b/libprocessgroup/cgrouprc_format/cgroup_controller.cpp index 56e67df59..0dd909a29 100644 --- a/libprocessgroup/cgrouprc_format/cgroup_controller.cpp +++ b/libprocessgroup/cgrouprc_format/cgroup_controller.cpp @@ -21,13 +21,11 @@ namespace cgrouprc { namespace format { CgroupController::CgroupController(uint32_t version, uint32_t flags, const std::string& name, - const std::string& path) -{ + const std::string& path, uint32_t max_activation_depth) + : version_(version), flags_(flags), max_activation_depth_(max_activation_depth) { // strlcpy isn't available on host. Although there is an implementation // in licutils, libcutils itself depends on libcgrouprc_format, causing // a circular dependency. - version_ = version; - flags_ = flags; strncpy(name_, name.c_str(), sizeof(name_) - 1); name_[sizeof(name_) - 1] = '\0'; strncpy(path_, path.c_str(), sizeof(path_) - 1); @@ -42,6 +40,10 @@ uint32_t CgroupController::flags() const { return flags_; } +uint32_t CgroupController::max_activation_depth() const { + return max_activation_depth_; +} + const char* CgroupController::name() const { return name_; } diff --git a/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h b/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h index 9427a1cf9..c0c1f6034 100644 --- a/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h +++ b/libprocessgroup/cgrouprc_format/include/processgroup/format/cgroup_controller.h @@ -29,10 +29,11 @@ struct CgroupController { public: CgroupController() = default; CgroupController(uint32_t version, uint32_t flags, const std::string& name, - const std::string& path); + const std::string& path, uint32_t max_activation_depth); uint32_t version() const; uint32_t flags() const; + uint32_t max_activation_depth() const; const char* name() const; const char* path() const; @@ -44,6 +45,7 @@ struct CgroupController { uint32_t version_ = 0; uint32_t flags_ = 0; + uint32_t max_activation_depth_ = UINT32_MAX; char name_[CGROUP_NAME_BUF_SZ] = {}; char path_[CGROUP_PATH_BUF_SZ] = {}; }; diff --git a/libprocessgroup/profiles/cgroups.proto b/libprocessgroup/profiles/cgroups.proto index f2de3452a..d2fd472d1 100644 --- a/libprocessgroup/profiles/cgroups.proto +++ b/libprocessgroup/profiles/cgroups.proto @@ -24,7 +24,7 @@ message Cgroups { Cgroups2 cgroups2 = 2 [json_name = "Cgroups2"]; } -// Next: 8 +// Next: 9 message Cgroup { string controller = 1 [json_name = "Controller"]; string path = 2 [json_name = "Path"]; @@ -36,6 +36,7 @@ message Cgroup { // https://developers.google.com/protocol-buffers/docs/proto3#default bool needs_activation = 6 [json_name = "NeedsActivation"]; bool is_optional = 7 [json_name = "Optional"]; + uint32 max_activation_depth = 8 [json_name = "MaxActivationDepth"]; } // Next: 6 diff --git a/libprocessgroup/setup/Android.bp b/libprocessgroup/setup/Android.bp index 1e0783ab0..76f0a11f1 100644 --- a/libprocessgroup/setup/Android.bp +++ b/libprocessgroup/setup/Android.bp @@ -37,6 +37,7 @@ cc_library_shared { ], header_libs: [ "libprocessgroup_headers", + "libprocessgroup_util", ], export_header_lib_headers: [ "libprocessgroup_headers", diff --git a/libprocessgroup/setup/cgroup_descriptor.h b/libprocessgroup/setup/cgroup_descriptor.h index 9982bfc73..06ce186fd 100644 --- a/libprocessgroup/setup/cgroup_descriptor.h +++ b/libprocessgroup/setup/cgroup_descriptor.h @@ -30,7 +30,8 @@ namespace cgrouprc { class CgroupDescriptor { public: CgroupDescriptor(uint32_t version, const std::string& name, const std::string& path, - mode_t mode, const std::string& uid, const std::string& gid, uint32_t flags); + mode_t mode, const std::string& uid, const std::string& gid, uint32_t flags, + uint32_t max_activation_depth); const format::CgroupController* controller() const { return &controller_; } mode_t mode() const { return mode_; } diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp index 1b26fbcc6..bd4187475 100644 --- a/libprocessgroup/setup/cgroup_map_write.cpp +++ b/libprocessgroup/setup/cgroup_map_write.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include "../build_flags.h" #include "cgroup_descriptor.h" @@ -173,9 +174,15 @@ static void MergeCgroupToDescriptors(std::map* de controller_flags |= CGROUPRC_CONTROLLER_FLAG_OPTIONAL; } + uint32_t max_activation_depth = UINT32_MAX; + if (cgroup.isMember("MaxActivationDepth")) { + max_activation_depth = cgroup["MaxActivationDepth"].asUInt(); + } + CgroupDescriptor descriptor( cgroups_version, name, path, std::strtoul(cgroup["Mode"].asString().c_str(), 0, 8), - cgroup["UID"].asString(), cgroup["GID"].asString(), controller_flags); + cgroup["UID"].asString(), cgroup["GID"].asString(), controller_flags, + max_activation_depth); auto iter = descriptors->find(name); if (iter == descriptors->end()) { @@ -324,7 +331,8 @@ static bool ActivateV2CgroupController(const CgroupDescriptor& descriptor) { return false; } - if (controller->flags() & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION) { + if (controller->flags() & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION && + controller->max_activation_depth() > 0) { std::string str = "+"; str += controller->name(); std::string path = controller->path(); @@ -433,8 +441,12 @@ static bool WriteRcFile(const std::map& descripto CgroupDescriptor::CgroupDescriptor(uint32_t version, const std::string& name, const std::string& path, mode_t mode, const std::string& uid, - const std::string& gid, uint32_t flags = 0) - : controller_(version, flags, name, path), mode_(mode), uid_(uid), gid_(gid) {} + const std::string& gid, uint32_t flags, + uint32_t max_activation_depth) + : controller_(version, flags, name, path, max_activation_depth), + mode_(mode), + uid_(uid), + gid_(gid) {} void CgroupDescriptor::set_mounted(bool mounted) { uint32_t flags = controller_.flags(); @@ -502,8 +514,11 @@ static bool CreateV2SubHierarchy( for (const auto& [name, descriptor] : descriptors) { const format::CgroupController* controller = descriptor.controller(); std::uint32_t flags = controller->flags(); + std::uint32_t max_activation_depth = controller->max_activation_depth(); + const int depth = util::GetCgroupDepth(controller->path(), path); + if (controller->version() == 2 && name != CGROUPV2_HIERARCHY_NAME && - flags & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION) { + flags & CGROUPRC_CONTROLLER_FLAG_NEEDS_ACTIVATION && depth < max_activation_depth) { std::string str("+"); str += controller->name(); if (!android::base::WriteStringToFile(str, path + "/cgroup.subtree_control")) { diff --git a/libprocessgroup/util/Android.bp b/libprocessgroup/util/Android.bp new file mode 100644 index 000000000..4a940b774 --- /dev/null +++ b/libprocessgroup/util/Android.bp @@ -0,0 +1,47 @@ +// +// Copyright (C) 2019 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. +// + +package { + default_team: "trendy_team_android_kernel", + default_applicable_licenses: ["Android-Apache-2.0"], +} + +cc_library_headers { + name: "libprocessgroup_util", + vendor_available: true, + product_available: true, + ramdisk_available: true, + vendor_ramdisk_available: true, + recovery_available: true, + host_supported: true, + native_bridge_supported: true, + apex_available: [ + "//apex_available:platform", + "//apex_available:anyapex", + ], + min_sdk_version: "30", + export_include_dirs: [ + "include", + ], + defaults: ["libprocessgroup_build_flags_cc"], +} + +cc_test { + name: "libprocessgroup_util_test", + header_libs: ["libprocessgroup_util"], + srcs: ["tests/util.cpp"], + test_suites: ["general-tests"], +} diff --git a/libprocessgroup/util/OWNERS b/libprocessgroup/util/OWNERS new file mode 100644 index 000000000..54ea4001e --- /dev/null +++ b/libprocessgroup/util/OWNERS @@ -0,0 +1,3 @@ +# Bug component: 1293033 +surenb@google.com +tjmercier@google.com diff --git a/libprocessgroup/util/TEST_MAPPING b/libprocessgroup/util/TEST_MAPPING new file mode 100644 index 000000000..6ae2658ce --- /dev/null +++ b/libprocessgroup/util/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "postsubmit": [ + { + "name": "libprocessgroup_util_test" + } + ] +} \ No newline at end of file diff --git a/libprocessgroup/util/include/processgroup/util.h b/libprocessgroup/util/include/processgroup/util.h new file mode 100644 index 000000000..5240744c6 --- /dev/null +++ b/libprocessgroup/util/include/processgroup/util.h @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2024 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 + +namespace util { + +namespace internal { + +const char SEP = '/'; + +std::string DeduplicateAndTrimSeparators(const std::string& path) { + bool lastWasSep = false; + std::string ret; + + std::copy_if(path.begin(), path.end(), std::back_inserter(ret), [&lastWasSep](char c) { + if (lastWasSep) { + if (c == SEP) return false; + lastWasSep = false; + } else if (c == SEP) { + lastWasSep = true; + } + return true; + }); + + if (ret.length() > 1 && ret.back() == SEP) ret.pop_back(); + + return ret; +} + +} // namespace internal + +unsigned int GetCgroupDepth(const std::string& controller_root, const std::string& cgroup_path) { + const std::string deduped_root = internal::DeduplicateAndTrimSeparators(controller_root); + const std::string deduped_path = internal::DeduplicateAndTrimSeparators(cgroup_path); + + if (deduped_root.empty() || deduped_path.empty() || !deduped_path.starts_with(deduped_root)) + return 0; + + return std::count(deduped_path.begin() + deduped_root.size(), deduped_path.end(), + internal::SEP); +} + +} // namespace util diff --git a/libprocessgroup/util/tests/util.cpp b/libprocessgroup/util/tests/util.cpp new file mode 100644 index 000000000..1de7d6f3f --- /dev/null +++ b/libprocessgroup/util/tests/util.cpp @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2024 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 "gtest/gtest.h" + +using util::GetCgroupDepth; + +TEST(EmptyInputs, bothEmpty) { + EXPECT_EQ(GetCgroupDepth({}, {}), 0); +} + +TEST(EmptyInputs, rootEmpty) { + EXPECT_EQ(GetCgroupDepth({}, "foo"), 0); +} + +TEST(EmptyInputs, pathEmpty) { + EXPECT_EQ(GetCgroupDepth("foo", {}), 0); +} + +TEST(InvalidInputs, pathNotInRoot) { + EXPECT_EQ(GetCgroupDepth("foo", "bar"), 0); +} + +TEST(InvalidInputs, rootLargerThanPath) { + EXPECT_EQ(GetCgroupDepth("/a/long/path", "/short"), 0); +} + +TEST(InvalidInputs, pathLargerThanRoot) { + EXPECT_EQ(GetCgroupDepth("/short", "/a/long/path"), 0); +} + +TEST(InvalidInputs, missingSeparator) { + EXPECT_EQ(GetCgroupDepth("/controller/root", "/controller/rootcgroup"), 0); +} + +TEST(ExtraSeparators, root) { + EXPECT_EQ(GetCgroupDepth("///sys/fs/cgroup", "/sys/fs/cgroup/a/b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys///fs/cgroup", "/sys/fs/cgroup/a/b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs///cgroup", "/sys/fs/cgroup/a/b/c"), 3); + + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "///sys/fs/cgroup/a/b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys///fs/cgroup/a/b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs///cgroup/a/b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup///a/b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/a///b/c"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/a/b///c"), 3); +} + +TEST(SeparatorEndings, rootEndsInSeparator) { + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup/", "/sys/fs/cgroup/a/b"), 2); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup///", "/sys/fs/cgroup/a/b"), 2); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup/", "/sys/fs/cgroup/a/b/"), 2); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup///", "/sys/fs/cgroup/a/b/"), 2); +} + +TEST(SeparatorEndings, pathEndsInSeparator) { + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/a/b/"), 2); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/a/b///"), 2); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup/", "/sys/fs/cgroup/a/b/"), 2); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup/", "/sys/fs/cgroup/a/b///"), 2); +} + +TEST(ValidInputs, rootHasZeroDepth) { + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup"), 0); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup/", "/sys/fs/cgroup"), 0); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/"), 0); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup/", "/sys/fs/cgroup/"), 0); +} + +TEST(ValidInputs, atLeastDepth10) { + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/a/b/c/d/e/f/g/h/i/j"), 10); +} + +TEST(ValidInputs, androidCgroupNames) { + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/system/uid_0/pid_1000"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/uid_0/pid_1000"), 2); + + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/apps/uid_100000/pid_1000"), 3); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/uid_100000/pid_1000"), 2); + + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/apps"), 1); + EXPECT_EQ(GetCgroupDepth("/sys/fs/cgroup", "/sys/fs/cgroup/system"), 1); +} + +TEST(ValidInputs, androidCgroupNames_nonDefaultRoot) { + EXPECT_EQ(GetCgroupDepth("/custom/root", "/custom/root/system/uid_0/pid_1000"), 3); + EXPECT_EQ(GetCgroupDepth("/custom/root", "/custom/root/uid_0/pid_1000"), 2); + + EXPECT_EQ(GetCgroupDepth("/custom/root", "/custom/root/apps/uid_100000/pid_1000"), 3); + EXPECT_EQ(GetCgroupDepth("/custom/root", "/custom/root/uid_100000/pid_1000"), 2); + + EXPECT_EQ(GetCgroupDepth("/custom/root", "/custom/root/apps"), 1); + EXPECT_EQ(GetCgroupDepth("/custom/root", "/custom/root/system"), 1); +} From a8f130693dff20e18c74467f68b7c6bcf93340d7 Mon Sep 17 00:00:00 2001 From: Liana Kazanova Date: Tue, 18 Jun 2024 22:20:43 +0000 Subject: [PATCH 008/158] Revert "reupload: libsnapshot: set thread priority" This reverts commit 00e1b61bb92b7a9905fdb5c00aba9ffd47a36dbf. Reason for revert: DroidMonitor: Potential culprit for b/348041418 - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted. Change-Id: Ib7422cc7bfb7815ede814fb5f0ee1d4537fe52d2 --- .../libsnapshot/snapuserd/user-space-merge/merge_worker.cpp | 4 +--- .../libsnapshot/snapuserd/user-space-merge/read_worker.cpp | 6 +----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp index 4afc312e3..bd7eaca74 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp @@ -577,10 +577,8 @@ bool MergeWorker::Run() { SNAP_LOG(ERROR) << "Merge terminated early..."; return true; } - auto merge_thread_priority = android::base::GetUintProperty( - "ro.virtual_ab.merge_thread_priority", ANDROID_PRIORITY_BACKGROUND); - if (!SetThreadPriority(merge_thread_priority)) { + if (!SetThreadPriority(ANDROID_PRIORITY_BACKGROUND)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp index 7191f1f74..d40b6d11d 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp @@ -17,10 +17,8 @@ #include #include -#include "android-base/properties.h" #include "read_worker.h" #include "snapuserd_core.h" -#include "user-space-merge/worker.h" #include "utility.h" namespace android { @@ -261,10 +259,8 @@ bool ReadWorker::Run() { SNAP_LOG(INFO) << "Processing snapshot I/O requests...."; pthread_setname_np(pthread_self(), "ReadWorker"); - auto worker_thread_priority = android::base::GetUintProperty( - "ro.virtual_ab.worker_thread_priority", ANDROID_PRIORITY_NORMAL); - if (!SetThreadPriority(worker_thread_priority)) { + if (!SetThreadPriority(ANDROID_PRIORITY_NORMAL)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } From c2970dd06f7b23179dddf71dd9041024cf743b26 Mon Sep 17 00:00:00 2001 From: Nelson Li Date: Wed, 19 Jun 2024 02:14:23 +0000 Subject: [PATCH 009/158] Rewrite `init_vendor` using select syntax The `select` syntax rewrite makes it more concise and easier to understand. Bug: 347605145 Test: m init_vendor Change-Id: I866bbe9360fdbdf69cac3c6a24bbe37306227755 --- init/Android.bp | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/init/Android.bp b/init/Android.bp index 6526a934e..7b7a856e9 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -664,22 +664,10 @@ sh_binary { filename_from_src: true, } -soong_config_module_type { - name: "board_use_recovery_as_boot_phony", - module_type: "phony", - config_namespace: "ANDROID", - bool_variables: ["BOARD_USES_RECOVERY_AS_BOOT"], - properties: ["required"], -} - -board_use_recovery_as_boot_phony { +phony { name: "init_vendor", - soong_config_variables: { - BOARD_USES_RECOVERY_AS_BOOT: { - required: [], - conditions_default: { - required: ["init_first_stage"], - }, - }, - }, + required: select(soong_config_variable("ANDROID", "BOARD_USES_RECOVERY_AS_BOOT"), { + true: [], + default: ["init_first_stage"], + }), } From 1fd7993f85bdf304bc53ca562b8c79a2142daa08 Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Fri, 21 Jun 2024 22:17:12 +0000 Subject: [PATCH 010/158] libcutils: Add missing dependency on libprocessgroup_headers cutils/sched_policy.h redirects to processgroup/sched_policy.h, but libcutils does not export the libprocessgroup headers. So users have to know about the hidden redirect to libprocessgroup and include a libprocessgroup dependency in their Android.bp files in addition to libcutils. Fix that. Bug: 349105928 Test: m Change-Id: I516bdf2cdeff980c1fcd18883ef8a2f0a9beb629 --- libcutils/Android.bp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libcutils/Android.bp b/libcutils/Android.bp index e2975813c..39cae1b4b 100644 --- a/libcutils/Android.bp +++ b/libcutils/Android.bp @@ -47,6 +47,8 @@ cc_library_headers { defaults: ["libcutils_defaults"], export_include_dirs: ["include"], + header_libs: ["libprocessgroup_headers"], + export_header_lib_headers: ["libprocessgroup_headers"], target: { vendor: { override_export_include_dirs: ["include_outside_system"], From 6f13d115eb0191e240364b70e59186777c8683fd Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Fri, 21 Jun 2024 21:27:32 +0000 Subject: [PATCH 011/158] libutils: Remove unused dependency on libprocessgroup_headers Bug: 349105928 Test: m Change-Id: Ic3e64b073c0ba087c3db19fb428c8c9132c8211f --- libutils/Android.bp | 2 -- libutils/Threads.cpp | 5 ----- 2 files changed, 7 deletions(-) diff --git a/libutils/Android.bp b/libutils/Android.bp index 4877caeee..305cbf0a8 100644 --- a/libutils/Android.bp +++ b/libutils/Android.bp @@ -33,14 +33,12 @@ cc_library_headers { "libbase_headers", "libcutils_headers", "liblog_headers", - "libprocessgroup_headers", "libsystem_headers", ], export_header_lib_headers: [ "libbase_headers", "libcutils_headers", "liblog_headers", - "libprocessgroup_headers", "libsystem_headers", ], export_include_dirs: ["include"], diff --git a/libutils/Threads.cpp b/libutils/Threads.cpp index 90ea29b22..0b96ab0bb 100644 --- a/libutils/Threads.cpp +++ b/libutils/Threads.cpp @@ -36,11 +36,6 @@ #include -#if defined(__ANDROID__) -#include -#include -#endif - #if defined(__ANDROID__) # define __android_unused #else From 9fbe52662210c56041052f4a13bcc70f789ac26e Mon Sep 17 00:00:00 2001 From: Julien Desprez Date: Mon, 24 Jun 2024 20:37:20 +0000 Subject: [PATCH 012/158] Migrate vts_libsnapshot_test and friends to general-tests zip for things that works in VTS, they should be buildable in general-tests zip rather than device-tests. Which is cheaper in term of build resources. Change-Id: If12442bf88cb5dd2b503e8023f925015890a39e2 Test: presubmit Bug: None --- fs_mgr/libsnapshot/Android.bp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index 39b5b7653..f0983186f 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -294,7 +294,7 @@ cc_test { ], test_suites: [ "vts", - "device-tests", + "general-tests", ], test_options: { min_shipping_api_level: 30, @@ -311,7 +311,7 @@ cc_test { "-DLIBSNAPSHOT_TEST_VAB_LEGACY", ], test_suites: [ - "device-tests", + "general-tests", ], test_options: { // Legacy VAB launched in Android R. @@ -414,7 +414,7 @@ cc_test { "libstorage_literals_headers", ], test_suites: [ - "device-tests", + "general-tests", ], test_options: { min_shipping_api_level: 30, From 7b55fbc6d4044a5059c63dea4678adb248e59f39 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Tue, 28 May 2024 18:25:12 -0700 Subject: [PATCH 013/158] reupload^2: libsnapshot: set thread priority It looks like the previous error was due to a missing include that somehow my LSP didn't catch. The build failure isn't reproducible on aosp-main but is on git_main. Confirmed this new patch with the #include builds correctly on git_main. Read merge thread + worker thread priority from build configurations. In the case of low memory devices, a lower priority will reduce CPU utilization post OTA reboot. Test: th Change-Id: I7fd398c4688cf78abe738e7320b9f27f5f7d1e13 --- .../snapuserd/user-space-merge/merge_worker.cpp | 10 +++++++--- .../snapuserd/user-space-merge/read_worker.cpp | 7 ++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp index bd7eaca74..e6a9a2989 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp @@ -17,6 +17,8 @@ #include #include +#include + #include "merge_worker.h" #include "snapuserd_core.h" #include "utility.h" @@ -179,8 +181,8 @@ bool MergeWorker::MergeReplaceZeroOps() { bufsink_.ResetBufferOffset(); if (snapuserd_->IsIOTerminated()) { - SNAP_LOG(ERROR) - << "MergeReplaceZeroOps: MergeWorker threads terminated - shutting down merge"; + SNAP_LOG(ERROR) << "MergeReplaceZeroOps: MergeWorker threads terminated - shutting " + "down merge"; return false; } } @@ -577,8 +579,10 @@ bool MergeWorker::Run() { SNAP_LOG(ERROR) << "Merge terminated early..."; return true; } + auto merge_thread_priority = android::base::GetUintProperty( + "ro.virtual_ab.merge_thread_priority", ANDROID_PRIORITY_BACKGROUND); - if (!SetThreadPriority(ANDROID_PRIORITY_BACKGROUND)) { + if (!SetThreadPriority(merge_thread_priority)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp index d40b6d11d..ef311d475 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp @@ -14,11 +14,14 @@ * limitations under the License. */ +#include + #include #include #include "read_worker.h" #include "snapuserd_core.h" +#include "user-space-merge/worker.h" #include "utility.h" namespace android { @@ -259,8 +262,10 @@ bool ReadWorker::Run() { SNAP_LOG(INFO) << "Processing snapshot I/O requests...."; pthread_setname_np(pthread_self(), "ReadWorker"); + auto worker_thread_priority = android::base::GetUintProperty( + "ro.virtual_ab.worker_thread_priority", ANDROID_PRIORITY_NORMAL); - if (!SetThreadPriority(ANDROID_PRIORITY_NORMAL)) { + if (!SetThreadPriority(worker_thread_priority)) { SNAP_PLOG(ERROR) << "Failed to set thread priority"; } From 8abaa8442f88800e8b653916828dd89f14aaa155 Mon Sep 17 00:00:00 2001 From: Priyanka Advani Date: Tue, 25 Jun 2024 18:11:53 +0000 Subject: [PATCH 014/158] Revert "Migrate vts_libsnapshot_test and friends to general-tests zip" This reverts commit 9fbe52662210c56041052f4a13bcc70f789ac26e. Reason for revert: Droidmonitor created revert due to b/349278999. Will be verifying through ABTD before submission. Change-Id: I9eae3b5ce729e0efdc092c814cbe225bcd43d32f --- fs_mgr/libsnapshot/Android.bp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index f0983186f..39b5b7653 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -294,7 +294,7 @@ cc_test { ], test_suites: [ "vts", - "general-tests", + "device-tests", ], test_options: { min_shipping_api_level: 30, @@ -311,7 +311,7 @@ cc_test { "-DLIBSNAPSHOT_TEST_VAB_LEGACY", ], test_suites: [ - "general-tests", + "device-tests", ], test_options: { // Legacy VAB launched in Android R. @@ -414,7 +414,7 @@ cc_test { "libstorage_literals_headers", ], test_suites: [ - "general-tests", + "device-tests", ], test_options: { min_shipping_api_level: 30, From 47fde793b111aecdb34edba598b5697ba9c4701c Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Wed, 26 Jun 2024 17:50:14 +0000 Subject: [PATCH 015/158] libutils: CallStackTest log_stack check more log Check last 10,000 instead of last 1,000 log entries. Fixes: 348793356 Test: presubmit Change-Id: I08c660ddd7e49466a4b55d96e46a402ee8912a24 --- libutils/CallStack_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libutils/CallStack_test.cpp b/libutils/CallStack_test.cpp index 7afc2c333..bfe6b8798 100644 --- a/libutils/CallStack_test.cpp +++ b/libutils/CallStack_test.cpp @@ -72,7 +72,8 @@ TEST(CallStackTest, thread_backtrace) { TEST(CallStackTest, log_stack) { android::CallStack::logStack("callstack_test"); auto logger_list = android_logger_list_open(android_name_to_log_id("main"), - ANDROID_LOG_NONBLOCK, 1000, getpid()); + ANDROID_LOG_NONBLOCK, + 10000 /* tail */, getpid()); ASSERT_NE(nullptr, logger_list); std::string log; while (true) { From 791663022e9b8aac8d6576d658f5bad802eda41c Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 26 Jun 2024 20:22:37 +0000 Subject: [PATCH 016/158] fs_mgr_remount.cpp: pedantic grammar fixes. In my defense, I genuinely thought it was telling me to try again. Change-Id: I613cd076a13de440bc98a8385022dd9f4b74311c --- fs_mgr/fs_mgr_remount.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp index 79c0b6df0..f91d2327c 100644 --- a/fs_mgr/fs_mgr_remount.cpp +++ b/fs_mgr/fs_mgr_remount.cpp @@ -171,11 +171,12 @@ bool VerifyCheckpointing() { } if (show_help) { show_help = false; - std::cerr << "WARNING: Userdata checkpoint is in progress. To force end checkpointing, " - "call 'vdc checkpoint commitChanges'. This can lead to data corruption if " - "rolled back." + std::cerr << "WARNING: Userdata checkpoint is in progress. " + "To forcibly end checkpointing, " + "call 'vdc checkpoint commitChanges'. " + "This can lead to data corruption if rolled back." << std::endl; - LOG(INFO) << "Waiting for checkpoint to complete and then continue remount."; + LOG(INFO) << "Waiting for checkpoint to complete before remounting..."; } std::this_thread::sleep_for(4s); } From 5c8a0f6752ad56234182fde9627620afa28c5211 Mon Sep 17 00:00:00 2001 From: jahinimtiaz Date: Wed, 26 Jun 2024 21:00:25 +0000 Subject: [PATCH 017/158] Opt out of gtest error if no binary found Bug: 348446766 Test: Presubmit Change-Id: If1aaa9726529453fc612fe4dec31f334460436d4 --- fs_mgr/libsnapshot/Android.bp | 6 ++++++ fs_mgr/libsnapshot/snapuserd/Android.bp | 10 +++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index 39b5b7653..cc6db358b 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -316,6 +316,12 @@ cc_test { test_options: { // Legacy VAB launched in Android R. min_shipping_api_level: 30, + test_runner_options: [ + { + name: "force-no-test-error", + value: "false", + }, + ], }, } diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp index 649309de1..d83524a18 100644 --- a/fs_mgr/libsnapshot/snapuserd/Android.bp +++ b/fs_mgr/libsnapshot/snapuserd/Android.bp @@ -168,7 +168,7 @@ cc_binary { vendor_ramdisk_available: true, } -// This target will install to /system/bin/snapuserd_ramdisk +// This target will install to /system/bin/snapuserd_ramdisk // It will also create a symblink on /system/bin/snapuserd that point to // /system/bin/snapuserd_ramdisk . // This way, init can check if generic ramdisk copy exists. @@ -249,6 +249,14 @@ cc_test { test_suites: [ "device-tests", ], + test_options: { + test_runner_options: [ + { + name: "force-no-test-error", + value: "false", + }, + ], + }, } // vts tests cannot be host_supported. From 0509c71cc44523af4c44b20316be96f781d52e02 Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Fri, 9 Feb 2024 19:18:32 +0000 Subject: [PATCH 018/158] Move StagedRollbackTest from postsubmit to presubmit Bug: 324596460 Change-Id: Iad7bfd7cb56f0c16366a9aa61b9466545ee4c661 --- libprocessgroup/TEST_MAPPING | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libprocessgroup/TEST_MAPPING b/libprocessgroup/TEST_MAPPING index 29a9ff050..b713f6b5e 100644 --- a/libprocessgroup/TEST_MAPPING +++ b/libprocessgroup/TEST_MAPPING @@ -1,5 +1,5 @@ { - "postsubmit": [ + "presubmit": [ { "name": "StagedRollbackTest" } From 104d3cc4b9e8f8b8ce3d94ba607c6074c1ec81ff Mon Sep 17 00:00:00 2001 From: Pechetty Sravani Date: Thu, 27 Jun 2024 01:49:59 +0000 Subject: [PATCH 019/158] Revert "Move StagedRollbackTest from postsubmit to presubmit" This reverts commit 0509c71cc44523af4c44b20316be96f781d52e02. Reason for revert: Droidmonitor triggered revert due to Test breakage in b/349693967. Will be verifying through ABTD before submission. Change-Id: I66a706b8b4f9bf802f7ea1a6b50bb4847b915c92 --- libprocessgroup/TEST_MAPPING | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libprocessgroup/TEST_MAPPING b/libprocessgroup/TEST_MAPPING index b713f6b5e..29a9ff050 100644 --- a/libprocessgroup/TEST_MAPPING +++ b/libprocessgroup/TEST_MAPPING @@ -1,5 +1,5 @@ { - "presubmit": [ + "postsubmit": [ { "name": "StagedRollbackTest" } From 36e2a44710f1eab8d2f125106f400fc5b5cb194a Mon Sep 17 00:00:00 2001 From: "T.J. Mercier" Date: Thu, 27 Jun 2024 01:58:24 +0000 Subject: [PATCH 020/158] Revert^2 "Move StagedRollbackTest from postsubmit to presubmit" This reverts commit 104d3cc4b9e8f8b8ce3d94ba607c6074c1ec81ff. Reason for revert: change from presubmit to presubmit-large Bug: 349693967 Change-Id: I47ea41162aea0140bc460521d0638ce9ca8a96a3 --- libprocessgroup/TEST_MAPPING | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libprocessgroup/TEST_MAPPING b/libprocessgroup/TEST_MAPPING index 29a9ff050..0f670ef41 100644 --- a/libprocessgroup/TEST_MAPPING +++ b/libprocessgroup/TEST_MAPPING @@ -1,5 +1,5 @@ { - "postsubmit": [ + "presubmit-large": [ { "name": "StagedRollbackTest" } From a7b4a9d4f94113f9bde6f287ed6de1c38b2c2986 Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Thu, 27 Jun 2024 10:27:21 -0700 Subject: [PATCH 021/158] Call sync() to guarantee blocks were released by unlink Otherwise, the cached inode does not release the blocks which causes next allocation for pinning fails. Bug: 349270107 Change-Id: I4c2886683ea1949c1999e551a80b56dc37459dba Signed-off-by: Jaegeuk Kim --- fs_mgr/libfiemap/fiemap_writer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/fs_mgr/libfiemap/fiemap_writer.cpp b/fs_mgr/libfiemap/fiemap_writer.cpp index 06e210eca..d61e0071b 100644 --- a/fs_mgr/libfiemap/fiemap_writer.cpp +++ b/fs_mgr/libfiemap/fiemap_writer.cpp @@ -60,6 +60,7 @@ static_assert(sizeof(off_t) == sizeof(uint64_t)); static inline void cleanup(const std::string& file_path, bool created) { if (created) { unlink(file_path.c_str()); + sync(); } } From 8d9b33d64917a05e3342a9b7faef2156afdf8604 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Thu, 27 Jun 2024 21:51:49 -0700 Subject: [PATCH 022/158] fs_mgr: avoid vector A container of const T uses std::allocator, which was an undocumented libc++ extension that has been removed. See https://github.com/llvm/llvm-project/pull/96319. Bug: 349681543 Test: m libfs_mgr Change-Id: Ic7f50453f05293b7684be22393d6e5871e493983 --- fs_mgr/fs_mgr_overlayfs_mount.cpp | 2 +- fs_mgr/fs_mgr_overlayfs_mount.h | 2 +- fs_mgr/fs_mgr_vendor_overlay.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs_mgr/fs_mgr_overlayfs_mount.cpp b/fs_mgr/fs_mgr_overlayfs_mount.cpp index bd0fcfd98..b63b9e7aa 100644 --- a/fs_mgr/fs_mgr_overlayfs_mount.cpp +++ b/fs_mgr/fs_mgr_overlayfs_mount.cpp @@ -74,7 +74,7 @@ bool fs_mgr_is_dsu_running() { return android::gsi::IsGsiRunning(); } -std::vector OverlayMountPoints() { +std::vector OverlayMountPoints() { // Never fallback to legacy cache mount point if within a DSU system, // because running a DSU system implies the device supports dynamic // partitions, which means legacy cache mustn't be used. diff --git a/fs_mgr/fs_mgr_overlayfs_mount.h b/fs_mgr/fs_mgr_overlayfs_mount.h index 98b9007b5..f941ab1d6 100644 --- a/fs_mgr/fs_mgr_overlayfs_mount.h +++ b/fs_mgr/fs_mgr_overlayfs_mount.h @@ -54,7 +54,7 @@ const std::string fs_mgr_mount_point(const std::string& mount_point); bool OverlayfsSetupAllowed(bool verbose = false); bool MountScratch(const std::string& device_path, bool readonly = false); bool fs_mgr_overlayfs_umount_scratch(); -std::vector OverlayMountPoints(); +std::vector OverlayMountPoints(); bool fs_mgr_overlayfs_already_mounted(const std::string& mount_point, bool overlay_only = true); bool fs_mgr_wants_overlayfs(android::fs_mgr::FstabEntry* entry); android::fs_mgr::Fstab fs_mgr_overlayfs_candidate_list(const android::fs_mgr::Fstab& fstab); diff --git a/fs_mgr/fs_mgr_vendor_overlay.cpp b/fs_mgr/fs_mgr_vendor_overlay.cpp index 6b742e55e..668728384 100644 --- a/fs_mgr/fs_mgr_vendor_overlay.cpp +++ b/fs_mgr/fs_mgr_vendor_overlay.cpp @@ -36,7 +36,7 @@ namespace { // The order of the list means the priority to show the files in the directory. // The last one has the highest priority. -const std::vector kVendorOverlaySourceDirs = { +const std::vector kVendorOverlaySourceDirs = { "/system/vendor_overlay/", "/product/vendor_overlay/", }; From d8f5fd4edb0efe61c1b23e451564d2563aa9ec69 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Thu, 27 Jun 2024 23:08:13 -0700 Subject: [PATCH 023/158] bootstat: avoid vector A container of const T uses std::allocator, which was an undocumented libc++ extension that has been removed. See https://github.com/llvm/llvm-project/pull/96319. Bug: 349681543 Test: m bootstat Change-Id: Id338f439aa4caf8c9f3c6fc15faef19b1edc4368 --- bootstat/bootstat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index d402bf17b..d476d36a9 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -499,7 +499,7 @@ int32_t BootReasonStrToEnum(const std::string& boot_reason) { } // Canonical list of supported primary reboot reasons. -const std::vector knownReasons = { +const std::vector knownReasons = { // clang-format off // kernel "watchdog", From dfd7b88fb47e92b611a6b654dafad44b8f065c34 Mon Sep 17 00:00:00 2001 From: Ashok Mutyala Date: Tue, 28 May 2024 09:19:59 +0000 Subject: [PATCH 024/158] Add length parameter to format /data With metadata encryption, if partition is wiped (or) cryptfs failed , it will do format without length which make partition size is grown large to occupy remaining space instead of restricting the Android defined partition size Bug: 343159184 Test: Add length flag in fstab && fastboot erase userdata && fastboot reboot && df -h /data Signed-off-by: Ashok Mutyala Change-Id: Ifce4b78df5edb899c84a032004895802766cd5cf --- fs_mgr/fs_mgr.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 7f41cea83..5156754a8 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1556,6 +1556,7 @@ MountAllResult fs_mgr_mount_all(Fstab* fstab, int mount_mode) { attempted_entry.mount_point, wiped ? "true" : "false", attempted_entry.fs_type, attempted_entry.fs_mgr_flags.is_zoned ? "true" : "false", + std::to_string(attempted_entry.length), android::base::Join(attempted_entry.user_devices, ' ')}, nullptr)) { LERROR << "Encryption failed"; @@ -1601,6 +1602,7 @@ MountAllResult fs_mgr_mount_all(Fstab* fstab, int mount_mode) { current_entry.mount_point, "true" /* shouldFormat */, current_entry.fs_type, current_entry.fs_mgr_flags.is_zoned ? "true" : "false", + std::to_string(current_entry.length), android::base::Join(current_entry.user_devices, ' ')}, nullptr)) { LERROR << "Encryption failed"; From d41c0f4a9e6e8178f5b8f46fe3b9a4530efc113a Mon Sep 17 00:00:00 2001 From: Tomasz Wasilczyk Date: Tue, 25 Jun 2024 10:34:23 -0700 Subject: [PATCH 025/158] Migrate pthread to std::thread Bug: 341997808 Test: atest --host libutils_test Change-Id: I4f435d5d85efcbcc351b7620811c172badc2276d --- libutils/Looper.cpp | 33 ++----------------- .../arm64/source-based/libutils.so.lsdump | 26 --------------- .../arm_arm64/source-based/libutils.so.lsdump | 26 --------------- libutils/include/utils/Looper.h | 2 -- 4 files changed, 3 insertions(+), 84 deletions(-) diff --git a/libutils/Looper.cpp b/libutils/Looper.cpp index 7700c9030..e11d19731 100644 --- a/libutils/Looper.cpp +++ b/libutils/Looper.cpp @@ -70,8 +70,7 @@ int SimpleLooperCallback::handleEvent(int fd, int events, void* data) { // Maximum number of file descriptors for which to retrieve poll events each iteration. static const int EPOLL_MAX_EVENTS = 16; -static pthread_once_t gTLSOnce = PTHREAD_ONCE_INIT; -static pthread_key_t gTLSKey = 0; +thread_local static sp gThreadLocalLooper; Looper::Looper(bool allowNonCallbacks) : mAllowNonCallbacks(allowNonCallbacks), @@ -91,38 +90,12 @@ Looper::Looper(bool allowNonCallbacks) Looper::~Looper() { } -void Looper::initTLSKey() { - int error = pthread_key_create(&gTLSKey, threadDestructor); - LOG_ALWAYS_FATAL_IF(error != 0, "Could not allocate TLS key: %s", strerror(error)); -} - -void Looper::threadDestructor(void *st) { - Looper* const self = static_cast(st); - if (self != nullptr) { - self->decStrong((void*)threadDestructor); - } -} - void Looper::setForThread(const sp& looper) { - sp old = getForThread(); // also has side-effect of initializing TLS - - if (looper != nullptr) { - looper->incStrong((void*)threadDestructor); - } - - pthread_setspecific(gTLSKey, looper.get()); - - if (old != nullptr) { - old->decStrong((void*)threadDestructor); - } + gThreadLocalLooper = looper; } sp Looper::getForThread() { - int result = pthread_once(& gTLSOnce, initTLSKey); - LOG_ALWAYS_FATAL_IF(result != 0, "pthread_once failed"); - - Looper* looper = (Looper*)pthread_getspecific(gTLSKey); - return sp::fromExisting(looper); + return gThreadLocalLooper; } sp Looper::prepare(int opts) { diff --git a/libutils/abi-dumps/arm64/source-based/libutils.so.lsdump b/libutils/abi-dumps/arm64/source-based/libutils.so.lsdump index 82ddca8f5..caea1a96c 100644 --- a/libutils/abi-dumps/arm64/source-based/libutils.so.lsdump +++ b/libutils/abi-dumps/arm64/source-based/libutils.so.lsdump @@ -509,9 +509,6 @@ { "name" : "_ZN7android47LightRefBase_reportIncStrongRequireStrongFailedEPKv" }, - { - "name" : "_ZN7android6Looper10initTLSKeyEv" - }, { "name" : "_ZN7android6Looper11sendMessageERKNS_2spINS_14MessageHandlerEEERKNS_7MessageE" }, @@ -527,9 +524,6 @@ { "name" : "_ZN7android6Looper14removeMessagesERKNS_2spINS_14MessageHandlerEEEi" }, - { - "name" : "_ZN7android6Looper16threadDestructorEPv" - }, { "name" : "_ZN7android6Looper17sendMessageAtTimeElRKNS_2spINS_14MessageHandlerEEERKNS_7MessageE" }, @@ -6909,13 +6903,6 @@ "return_type" : "_ZTIv", "source_file" : "system/core/libutils/include/utils/LightRefBase.h" }, - { - "access" : "private", - "function_name" : "android::Looper::initTLSKey", - "linker_set_key" : "_ZN7android6Looper10initTLSKeyEv", - "return_type" : "_ZTIv", - "source_file" : "system/core/libutils/include/utils/Looper.h" - }, { "function_name" : "android::Looper::sendMessage", "linker_set_key" : "_ZN7android6Looper11sendMessageERKNS_2spINS_14MessageHandlerEEERKNS_7MessageE", @@ -6988,19 +6975,6 @@ "return_type" : "_ZTIv", "source_file" : "system/core/libutils/include/utils/Looper.h" }, - { - "access" : "private", - "function_name" : "android::Looper::threadDestructor", - "linker_set_key" : "_ZN7android6Looper16threadDestructorEPv", - "parameters" : - [ - { - "referenced_type" : "_ZTIPv" - } - ], - "return_type" : "_ZTIv", - "source_file" : "system/core/libutils/include/utils/Looper.h" - }, { "function_name" : "android::Looper::sendMessageAtTime", "linker_set_key" : "_ZN7android6Looper17sendMessageAtTimeElRKNS_2spINS_14MessageHandlerEEERKNS_7MessageE", diff --git a/libutils/abi-dumps/arm_arm64/source-based/libutils.so.lsdump b/libutils/abi-dumps/arm_arm64/source-based/libutils.so.lsdump index dfc1ab581..b9c4c521f 100644 --- a/libutils/abi-dumps/arm_arm64/source-based/libutils.so.lsdump +++ b/libutils/abi-dumps/arm_arm64/source-based/libutils.so.lsdump @@ -513,9 +513,6 @@ { "name" : "_ZN7android47LightRefBase_reportIncStrongRequireStrongFailedEPKv" }, - { - "name" : "_ZN7android6Looper10initTLSKeyEv" - }, { "name" : "_ZN7android6Looper11sendMessageERKNS_2spINS_14MessageHandlerEEERKNS_7MessageE" }, @@ -531,9 +528,6 @@ { "name" : "_ZN7android6Looper14removeMessagesERKNS_2spINS_14MessageHandlerEEEi" }, - { - "name" : "_ZN7android6Looper16threadDestructorEPv" - }, { "name" : "_ZN7android6Looper17sendMessageAtTimeExRKNS_2spINS_14MessageHandlerEEERKNS_7MessageE" }, @@ -6949,13 +6943,6 @@ "return_type" : "_ZTIv", "source_file" : "system/core/libutils/include/utils/LightRefBase.h" }, - { - "access" : "private", - "function_name" : "android::Looper::initTLSKey", - "linker_set_key" : "_ZN7android6Looper10initTLSKeyEv", - "return_type" : "_ZTIv", - "source_file" : "system/core/libutils/include/utils/Looper.h" - }, { "function_name" : "android::Looper::sendMessage", "linker_set_key" : "_ZN7android6Looper11sendMessageERKNS_2spINS_14MessageHandlerEEERKNS_7MessageE", @@ -7028,19 +7015,6 @@ "return_type" : "_ZTIv", "source_file" : "system/core/libutils/include/utils/Looper.h" }, - { - "access" : "private", - "function_name" : "android::Looper::threadDestructor", - "linker_set_key" : "_ZN7android6Looper16threadDestructorEPv", - "parameters" : - [ - { - "referenced_type" : "_ZTIPv" - } - ], - "return_type" : "_ZTIv", - "source_file" : "system/core/libutils/include/utils/Looper.h" - }, { "function_name" : "android::Looper::sendMessageAtTime", "linker_set_key" : "_ZN7android6Looper17sendMessageAtTimeExRKNS_2spINS_14MessageHandlerEEERKNS_7MessageE", diff --git a/libutils/include/utils/Looper.h b/libutils/include/utils/Looper.h index 41c5536f4..a02280b13 100644 --- a/libutils/include/utils/Looper.h +++ b/libutils/include/utils/Looper.h @@ -499,8 +499,6 @@ private: void rebuildEpollLocked(); void scheduleEpollRebuildLocked(); - static void initTLSKey(); - static void threadDestructor(void *st); static void initEpollEvent(struct epoll_event* eventItem); }; From 1bbf8f042f7ffb4a38657ccf6dca0798931fecd5 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Fri, 21 Jun 2024 10:53:53 -0700 Subject: [PATCH 026/158] libsnapshot: Check if the vendor is updated from Android S for GRF In a GRF config, if Vendor partition is updated from Android 12; post OTA reboot, first stage init will communicate to daemon to check if the daemon can support socket handoff. If that succeeds, then it is a signal that the vendor has been updated from Android 12. Use a marker in /metadata to signal that the vendor was updated. If the marker is present, then post OTA reboot, userspace snapshot will be used. Bug: 333854394 Test: OTA Android U (system) + S (vendor) -> Android V (system) + V (Vendor) Change-Id: Ie38c4379010789a84e5b44529b407f9f82135271 Signed-off-by: Akilesh Kailash --- .../include/libsnapshot/snapshot.h | 4 +++ fs_mgr/libsnapshot/snapshot.cpp | 32 ++++++++++++++++++- init/first_stage_mount.cpp | 7 +--- init/snapuserd_transition.cpp | 4 +++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 6d422c614..1ec863444 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -335,6 +335,9 @@ class SnapshotManager final : public ISnapshotManager { // after loading selinux policy. bool PrepareSnapuserdArgsForSelinux(std::vector* snapuserd_argv); + // If snapuserd from first stage init was started from system partition. + bool MarkSnapuserdFromSystem(); + // Detach dm-user devices from the first stage snapuserd. Load // new dm-user tables after loading selinux policy. bool DetachFirstStageSnapuserdForSelinux(); @@ -670,6 +673,7 @@ class SnapshotManager final : public ISnapshotManager { std::string GetForwardMergeIndicatorPath(); std::string GetOldPartitionMetadataPath(); std::string GetBootSnapshotsWithoutSlotSwitchPath(); + std::string GetSnapuserdFromSystemPath(); const LpMetadata* ReadOldPartitionMetadata(LockedFile* lock); diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index c01360e0b..3a474d710 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -88,6 +88,7 @@ static constexpr char kBootSnapshotsWithoutSlotSwitch[] = "/metadata/ota/snapshot-boot-without-slot-switch"; static constexpr char kBootIndicatorPath[] = "/metadata/ota/snapshot-boot"; static constexpr char kRollbackIndicatorPath[] = "/metadata/ota/rollback-indicator"; +static constexpr char kSnapuserdFromSystem[] = "/metadata/ota/snapuserd-from-system"; static constexpr auto kUpdateStateCheckInterval = 2s; /* * The readahead size is set to 32kb so that @@ -318,7 +319,7 @@ bool SnapshotManager::RemoveAllUpdateState(LockedFile* lock, const std::function std::vector files = { GetSnapshotBootIndicatorPath(), GetRollbackIndicatorPath(), GetForwardMergeIndicatorPath(), GetOldPartitionMetadataPath(), - GetBootSnapshotsWithoutSlotSwitchPath(), + GetBootSnapshotsWithoutSlotSwitchPath(), GetSnapuserdFromSystemPath(), }; for (const auto& file : files) { RemoveFileIfExists(file); @@ -1457,6 +1458,10 @@ std::string SnapshotManager::GetRollbackIndicatorPath() { return metadata_dir_ + "/" + android::base::Basename(kRollbackIndicatorPath); } +std::string SnapshotManager::GetSnapuserdFromSystemPath() { + return metadata_dir_ + "/" + android::base::Basename(kSnapuserdFromSystem); +} + std::string SnapshotManager::GetForwardMergeIndicatorPath() { return metadata_dir_ + "/allow-forward-merge"; } @@ -2122,6 +2127,16 @@ bool SnapshotManager::UpdateUsesODirect(LockedFile* lock) { return update_status.o_direct(); } +bool SnapshotManager::MarkSnapuserdFromSystem() { + auto path = GetSnapuserdFromSystemPath(); + + if (!android::base::WriteStringToFile("1", path)) { + PLOG(ERROR) << "Unable to write to vendor update path: " << path; + return false; + } + return true; +} + /* * Please see b/304829384 for more details. * @@ -2158,11 +2173,26 @@ bool SnapshotManager::UpdateUsesODirect(LockedFile* lock) { * iii: If both (i) and (ii) are true, then use the dm-snapshot based * approach. * + * 3: Post OTA reboot, if the vendor partition was updated from Android 12 to + * any other release post Android 12, then snapuserd binary will be "system" + * partition as post Android 12, init_boot will contain a copy of snapuserd + * binary. Thus, during first stage init, if init is able to communicate to + * daemon, that gives us a signal that the binary is from "system" copy. Hence, + * there is no need to fallback to legacy dm-snapshot. Thus, init will use a + * marker in /metadata to signal that the snapuserd binary from first stage init + * can handle userspace snapshots. + * */ bool SnapshotManager::IsLegacySnapuserdPostReboot() { if (is_legacy_snapuserd_.has_value() && is_legacy_snapuserd_.value() == true) { auto slot = GetCurrentSlot(); if (slot == Slot::Target) { + // If this marker is present, then daemon can handle userspace + // snapshots; also, it indicates that the vendor partition was + // updated from Android 12. + if (access(GetSnapuserdFromSystemPath().c_str(), F_OK) == 0) { + return false; + } return true; } } diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 5d3a27354..55cce6eaa 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -395,12 +395,7 @@ bool FirstStageMountVBootV2::CreateSnapshotPartitions(SnapshotManager* sm) { use_snapuserd_ = sm->IsSnapuserdRequired(); if (use_snapuserd_) { - if (sm->UpdateUsesUserSnapshots()) { - LaunchFirstStageSnapuserd(); - } else { - LOG(FATAL) << "legacy virtual-ab is no longer supported"; - return false; - } + LaunchFirstStageSnapuserd(); } sm->SetUeventRegenCallback([this](const std::string& device) -> bool { diff --git a/init/snapuserd_transition.cpp b/init/snapuserd_transition.cpp index 9e3ff4175..2370bc205 100644 --- a/init/snapuserd_transition.cpp +++ b/init/snapuserd_transition.cpp @@ -100,6 +100,10 @@ void LaunchFirstStageSnapuserd() { } if (client->SupportsSecondStageSocketHandoff()) { setenv(kSnapuserdFirstStageInfoVar, "socket", 1); + auto sm = SnapshotManager::NewForFirstStageMount(); + if (!sm->MarkSnapuserdFromSystem()) { + LOG(ERROR) << "Failed to update MarkSnapuserdFromSystem"; + } } setenv(kSnapuserdFirstStagePidVar, std::to_string(pid).c_str(), 1); From ab772110c74ec77e59c593fe7e3b3836fbdd164d Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 1 Jul 2024 13:21:36 +0000 Subject: [PATCH 027/158] Insulate against log spam. We may as well check the entire log. The previous bump seems to have made this test less flaky, so why not go all the way? Change-Id: I26b7524731ec755b724b9363fc9151f6a6d9116d --- libutils/CallStack_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libutils/CallStack_test.cpp b/libutils/CallStack_test.cpp index bfe6b8798..5ed3c27e8 100644 --- a/libutils/CallStack_test.cpp +++ b/libutils/CallStack_test.cpp @@ -73,7 +73,7 @@ TEST(CallStackTest, log_stack) { android::CallStack::logStack("callstack_test"); auto logger_list = android_logger_list_open(android_name_to_log_id("main"), ANDROID_LOG_NONBLOCK, - 10000 /* tail */, getpid()); + INT_MAX /* tail */, getpid()); ASSERT_NE(nullptr, logger_list); std::string log; while (true) { From ef0f5bbcf117f7abf5b2c671568fb23b7200f41f Mon Sep 17 00:00:00 2001 From: Ted Bauer Date: Tue, 2 Jul 2024 14:42:44 +0000 Subject: [PATCH 028/158] Skip test that unmounts/remounts metadata partition Aconfig flagging is migrating to a new storage backend that makes heavy use of the /metadata partition. It is causing failures in tests like vts_libsnapshot_test, which unmount and remount this partition. The failed unmounts are due to the partition being busy. The test should be reworked to not unmount and remount the partition; linked bug in GSKIP_TEST for followup. Bug: 349807180 Bug: 350715463 Test: m vts_libsnapshot_test Change-Id: Ie4f8cadcad189b13b9eba1eb15c5251002e8138a --- fs_mgr/libsnapshot/snapshot_test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 3299ec5c9..1435b12bf 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -1978,6 +1978,8 @@ TEST_F(MetadataMountedTest, Android) { } TEST_F(MetadataMountedTest, Recovery) { + GTEST_SKIP() << "b/350715463"; + test_device->set_recovery(true); metadata_dir_ = test_device->GetMetadataDir(); From dcc81d427a9cfcb7560b4ba590ace3348b314860 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 2 Jul 2024 10:57:23 -0700 Subject: [PATCH 029/158] Make snapuserd recovery_available. Bug: 349287459 Test: adb reboot recovery adb root adb shell ls -l /system/bin/snapuserd Change-Id: I69a3f8d2fd2d7dc157d14a0f743650881eec473d --- fs_mgr/libsnapshot/snapuserd/Android.bp | 1 + 1 file changed, 1 insertion(+) diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp index d83524a18..b3a7e8cf1 100644 --- a/fs_mgr/libsnapshot/snapuserd/Android.bp +++ b/fs_mgr/libsnapshot/snapuserd/Android.bp @@ -166,6 +166,7 @@ cc_binary { ], ramdisk_available: false, vendor_ramdisk_available: true, + recovery_available: true, } // This target will install to /system/bin/snapuserd_ramdisk From 0bdd68bb065cc168a262aae3b701ecb7335c6c52 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Wed, 3 Jul 2024 18:41:05 +0900 Subject: [PATCH 030/158] Use no_full_install: true instead of installable: false This is a follow-up on I37380c19232f2c497bdf492a83cdc16616f0ae8d. Bug: 338160898 Bug: 345110999 Test: Microdroid boots even with BOARD_USES_RECOVERY_AS_BOOT Change-Id: I41c1e40aeaffd5499fb6bd25e80b5be83470bc6b --- init/Android.bp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/init/Android.bp b/init/Android.bp index 7b7a856e9..c70a5deaa 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -338,7 +338,7 @@ soong_config_module_type { module_type: "cc_defaults", config_namespace: "ANDROID", bool_variables: ["BOARD_USES_RECOVERY_AS_BOOT"], - properties: ["installable"], + properties: ["no_full_install"], } // Do not install init_first_stage even with mma if we're system-as-root. @@ -347,7 +347,7 @@ init_first_stage_cc_defaults { name: "init_first_stage_defaults", soong_config_variables: { BOARD_USES_RECOVERY_AS_BOOT: { - installable: false, + no_full_install: true, }, }, From d9d7f7a8c3fe8e20bf32419fa3939c0cbfce283b Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Wed, 3 Jul 2024 17:33:08 -0700 Subject: [PATCH 031/158] Make timeout messages distinct ... so that we can confirm that it's actually the poll() call that's timing out. Bug: 332593241 Test: Treehugger Change-Id: I529be76a268d7ba1f7f26a953eb84945f3ac4924 --- debuggerd/client/debuggerd_client.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/debuggerd/client/debuggerd_client.cpp b/debuggerd/client/debuggerd_client.cpp index af1bb8181..632f6f1c9 100644 --- a/debuggerd/client/debuggerd_client.cpp +++ b/debuggerd/client/debuggerd_client.cpp @@ -138,7 +138,7 @@ bool debuggerd_trigger_dump(pid_t tid, DebuggerdDumpType dump_type, unsigned int auto remaining = end - std::chrono::steady_clock::now(); if (remaining < decltype(remaining)::zero()) { - log_error(output_fd, 0, "timeout expired"); + log_error(output_fd, 0, "timeout expired (update_timeout)"); return false; } @@ -254,7 +254,7 @@ bool debuggerd_trigger_dump(pid_t tid, DebuggerdDumpType dump_type, unsigned int if (timeout_ms <= 0) { remaining_ms = -1; } else if (remaining_ms < 0) { - log_error(output_fd, 0, "timeout expired"); + log_error(output_fd, 0, "timeout expired before poll"); return false; } @@ -271,7 +271,7 @@ bool debuggerd_trigger_dump(pid_t tid, DebuggerdDumpType dump_type, unsigned int return false; } } else if (rc == 0) { - log_error(output_fd, 0, "timeout expired"); + log_error(output_fd, 0, "poll timeout expired"); return false; } From 8896d29c5f9292681fd9f068ccfe3df76ae99c0e Mon Sep 17 00:00:00 2001 From: Owner Cleanup Bot Date: Tue, 9 Jul 2024 21:12:29 +0000 Subject: [PATCH 032/158] Remove alanstokes@google.com from mini_keyctl/OWNERS This suggested change is automatically generated based on group memberships and affiliations. If this change is unnecessary or in error, vote the lowest CR value (i.e. reject the CL) and the bot will abandon it. Vote the highest CR to approve this change. You may also abandon this change. See the owner's recent activity for context: https://android-review.googlesource.com/q/alanstokes@google.com To report an issue, file a bug in the Infra>Codereview component. Change-Id: I1b8637979a8206e6f8d1e56a7a9a4c84b7896739 --- mini_keyctl/OWNERS | 1 - 1 file changed, 1 deletion(-) diff --git a/mini_keyctl/OWNERS b/mini_keyctl/OWNERS index f9e7b25ea..1f2485a0c 100644 --- a/mini_keyctl/OWNERS +++ b/mini_keyctl/OWNERS @@ -1,4 +1,3 @@ -alanstokes@google.com ebiggers@google.com jeffv@google.com jiyong@google.com From 453665b5f567b0042aa47fb658142693a26130c6 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Wed, 10 Jul 2024 00:28:03 -0700 Subject: [PATCH 033/158] snapuserd_test: Create dup of fd before passing it to cow writer Bug: 352085551 Test: snapuserd_test - atest 10 iterations passed Change-Id: I25549e546cb3ce234fcf92533effb124793f1953 Signed-off-by: Akilesh Kailash --- .../user-space-merge/snapuserd_test.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp index 56f7b5911..6d0ae3d09 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp @@ -79,6 +79,8 @@ class SnapuserdTestBase : public ::testing::TestWithParam { std::unique_ptr CreateCowDeviceInternal(); std::unique_ptr CreateV3Cow(); + unique_fd GetCowFd() { return unique_fd{dup(cow_system_->fd)}; } + std::unique_ptr harness_; size_t size_ = 10_MiB; int total_base_size_ = 0; @@ -101,7 +103,9 @@ void SnapuserdTestBase::SetUp() { #endif } -void SnapuserdTestBase::TearDown() {} +void SnapuserdTestBase::TearDown() { + cow_system_ = nullptr; +} void SnapuserdTestBase::CreateBaseDevice() { total_base_size_ = (size_ * 5); @@ -132,10 +136,7 @@ std::unique_ptr SnapuserdTestBase::CreateCowDeviceInternal() { CowOptions options; options.compression = "gz"; - unique_fd fd(cow_system_->fd); - cow_system_->fd = -1; - - return CreateCowWriter(2, options, std::move(fd)); + return CreateCowWriter(2, options, GetCowFd()); } std::unique_ptr SnapuserdTestBase::CreateV3Cow() { @@ -151,10 +152,7 @@ std::unique_ptr SnapuserdTestBase::CreateV3Cow() { std::string path = android::base::GetExecutableDirectory(); cow_system_ = std::make_unique(path); - unique_fd fd(cow_system_->fd); - cow_system_->fd = -1; - - return CreateCowWriter(3, options, std::move(fd)); + return CreateCowWriter(3, options, GetCowFd()); } void SnapuserdTestBase::CreateCowDevice() { From 0fb39f6e698d99157b2ce640a4bacd4479c0cf11 Mon Sep 17 00:00:00 2001 From: Tiffany Yang Date: Tue, 21 May 2024 16:43:35 -0700 Subject: [PATCH 034/158] init: Support for initializing virtio-console devices This change allows init to ensure that a specified virtio-console device file (`/dev/hvc*`) is available before `ueventd` coldboot. Times out if device path is not encountered within 10 seconds. Bug: 325538592 Test: build bertha_x86_64 and bertha_arm64 Change-Id: Ia1512e69ea607bf4d235595caa53668e2dac500c --- init/block_dev_initializer.cpp | 4 ++++ init/block_dev_initializer.h | 1 + 2 files changed, 5 insertions(+) diff --git a/init/block_dev_initializer.cpp b/init/block_dev_initializer.cpp index a686d0513..8f5215856 100644 --- a/init/block_dev_initializer.cpp +++ b/init/block_dev_initializer.cpp @@ -139,6 +139,10 @@ bool BlockDevInitializer::InitPlatformDevice(const std::string& dev_name) { return InitDevice("/sys/devices/platform", dev_name); } +bool BlockDevInitializer::InitHvcDevice(const std::string& dev_name) { + return InitDevice("/sys/devices/virtual/tty", dev_name); +} + bool BlockDevInitializer::InitDevice(const std::string& syspath, const std::string& device_name) { bool found = false; diff --git a/init/block_dev_initializer.h b/init/block_dev_initializer.h index d5b1f6006..cb1d36555 100644 --- a/init/block_dev_initializer.h +++ b/init/block_dev_initializer.h @@ -34,6 +34,7 @@ class BlockDevInitializer final { bool InitDevices(std::set devices); bool InitDmDevice(const std::string& device); bool InitPlatformDevice(const std::string& device); + bool InitHvcDevice(const std::string& device); private: ListenerAction HandleUevent(const Uevent& uevent, std::set* devices); From 4d1894abc4e2b33032b0a74c5946f9db6393c24d Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Thu, 11 Jul 2024 09:53:23 -0700 Subject: [PATCH 035/158] Add debug logs to understand update start delay Test: th Bug: 352332753 Change-Id: I7c8f11409dc2a1312813b29a4c523b3fe17833c4 --- fs_mgr/libsnapshot/snapshot.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 3a474d710..4d5531535 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -2876,6 +2876,7 @@ bool SnapshotManager::UnmapAllSnapshots(LockedFile* lock) { return false; } } + LOG(INFO) << "Unmapped " << snapshots.size() << " partitions with snapshots"; // Terminate the daemon and release the snapuserd_client_ object. // If we need to re-connect with the daemon, EnsureSnapuserdConnected() From a3a6c8e0525ffdeeb1b773f0d388bbb84eb2f0a2 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Thu, 11 Jul 2024 23:54:48 +0000 Subject: [PATCH 036/158] Remove usage of ATOMIC_VAR_INIT. The use of the macro ATOMIC_VAR_INIT is not necessary and is causing warnings about it being deprecated. So remove it. Test: Compiles without any warnings. Change-Id: I137ffd0a7cf9a24c2c7ddea5c30f310722f57b98 --- libcutils/trace-dev.inc | 6 +++--- libcutils/trace-host.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libcutils/trace-dev.inc b/libcutils/trace-dev.inc index 94945ec7d..3bc6dc343 100644 --- a/libcutils/trace-dev.inc +++ b/libcutils/trace-dev.inc @@ -49,17 +49,17 @@ constexpr uint32_t kSeqNoNotInit = static_cast(-1); -atomic_bool atrace_is_ready = ATOMIC_VAR_INIT(false); +atomic_bool atrace_is_ready = false; int atrace_marker_fd = -1; uint64_t atrace_enabled_tags = ATRACE_TAG_NOT_READY; -static atomic_bool atrace_is_enabled = ATOMIC_VAR_INIT(true); +static atomic_bool atrace_is_enabled = true; static pthread_mutex_t atrace_tags_mutex = PTHREAD_MUTEX_INITIALIZER; /** * Sequence number of debug.atrace.tags.enableflags the last time the enabled * tags were reloaded. **/ -static _Atomic(uint32_t) last_sequence_number = ATOMIC_VAR_INIT(kSeqNoNotInit); +static _Atomic(uint32_t) last_sequence_number = kSeqNoNotInit; #if defined(__BIONIC__) // All zero prop_info that has a sequence number of 0. This is easier than diff --git a/libcutils/trace-host.cpp b/libcutils/trace-host.cpp index 2bf57ebbe..c45d0675c 100644 --- a/libcutils/trace-host.cpp +++ b/libcutils/trace-host.cpp @@ -16,7 +16,7 @@ #include -atomic_bool atrace_is_ready = ATOMIC_VAR_INIT(true); +atomic_bool atrace_is_ready = true; int atrace_marker_fd = -1; uint64_t atrace_enabled_tags = 0; From 22a683c7aa8648fdc66503ae68c8e3e22d7d26aa Mon Sep 17 00:00:00 2001 From: Ahmad Chaudhry Date: Fri, 5 Jul 2024 05:12:21 +0000 Subject: [PATCH 037/158] fs_mgr: libfstab: allow recovery.fstab with suffix Some platforms may need different fstab configs. Accordingly, allow recovery init to select a recovery.fstab with a suffix provided through bootconfigs or cmdline during runtime, to make a single recovery.img work on different environments. Change-Id: I0c2f61ffe9ddeb03afc54d8acc43b4917d65b846 --- fs_mgr/libfstab/fstab.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libfstab/fstab.cpp b/fs_mgr/libfstab/fstab.cpp index f00e0dc26..21d2e2ef3 100644 --- a/fs_mgr/libfstab/fstab.cpp +++ b/fs_mgr/libfstab/fstab.cpp @@ -520,6 +520,24 @@ std::vector GetEntriesByPred(FstabPtr fstab, const Pred& pre } // namespace +// Return the path to the recovery fstab file. There may be multiple fstab files; +// the one that is returned will be the first that exists of recovery.fstab., +// recovery.fstab., and recovery.fstab.. +std::string GetRecoveryFstabPath() { + for (const char* prop : {"fstab_suffix", "hardware", "hardware.platform"}) { + std::string suffix; + + if (!fs_mgr_get_boot_config(prop, &suffix)) continue; + + std::string fstab_path = "/etc/recovery.fstab." + suffix; + if (access(fstab_path.c_str(), F_OK) == 0) { + return fstab_path; + } + } + + return "/etc/recovery.fstab"; +} + // Return the path to the fstab file. There may be multiple fstab files; the // one that is returned will be the first that exists of fstab., // fstab., and fstab.. The fstab is searched for @@ -529,7 +547,7 @@ std::vector GetEntriesByPred(FstabPtr fstab, const Pred& pre // the system/etc directory is supported too and is the preferred location. std::string GetFstabPath() { if (InRecovery()) { - return "/etc/recovery.fstab"; + return GetRecoveryFstabPath(); } for (const char* prop : {"fstab_suffix", "hardware", "hardware.platform"}) { std::string suffix; From 2c2f3294b7f98c2715c56eb8c21c6b586bddbb09 Mon Sep 17 00:00:00 2001 From: Nelson Li Date: Thu, 11 Jul 2024 14:13:02 +0800 Subject: [PATCH 038/158] Convert *-developer-gsi.avbpubkey to Android.bp Use `soong_config_module_type` to install `*-developer-gsi.avbpubkey` to either `ramdisk` or `vendor_ramdisk` based on the value of `BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT` in each device. In build/soong/android/paths.go's modulePartition() function, there is already logic to construct `vendor_ramdisk` and `ramdisk` partitions based on the different values of `BOARD_MOVE_RECOVERY_RESOURCES_TO_VENDOR_BOOT` and `BOARD_USES_RECOVERY_AS_BOOT`. And the logic is identical to the original Android.mk. Therefore, this change only needs to determine whether the avb public keys should be placed in `vendor_ramdisk` or `ramdisk` based on the value of `BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT`. The rest of the judgment logic can be ignored. Bug: 347636127 Test: 1) lunch aosp_cf_x86_64_phone && m q-developer-gsi.avbpubkey 2) check it installed to vendor_ramdisk/first_stage_ramdisk/avb/ 3) lunch yukawa && m q-developer-gsi.avbpubkey 4) check it installed to ramdisk/avb/q-developer-gsi.avbpubkey Change-Id: I6de1a038261f2feeae4504d8097c7392b166848d --- rootdir/avb/Android.bp | 71 ++++++++++++++++++++++++++++++++++++++++++ rootdir/avb/Android.mk | 56 --------------------------------- 2 files changed, 71 insertions(+), 56 deletions(-) create mode 100644 rootdir/avb/Android.bp delete mode 100644 rootdir/avb/Android.mk diff --git a/rootdir/avb/Android.bp b/rootdir/avb/Android.bp new file mode 100644 index 000000000..a584e3e59 --- /dev/null +++ b/rootdir/avb/Android.bp @@ -0,0 +1,71 @@ +// Copyright (C) 2024 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. + +soong_config_module_type { + name: "avb_keys_prebuilt_avb", + module_type: "prebuilt_avb", + config_namespace: "ANDROID", + bool_variables: [ + "BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT", + ], + properties: [ + "ramdisk", + "vendor_ramdisk", + ], +} + +avb_keys_prebuilt_avb { + name: "q-developer-gsi.avbpubkey", + src: "q-developer-gsi.avbpubkey", + soong_config_variables: { + BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT: { + ramdisk: false, + vendor_ramdisk: true, + conditions_default: { + ramdisk: true, + vendor_ramdisk: false, + }, + }, + }, +} + +avb_keys_prebuilt_avb { + name: "r-developer-gsi.avbpubkey", + src: "r-developer-gsi.avbpubkey", + soong_config_variables: { + BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT: { + ramdisk: false, + vendor_ramdisk: true, + conditions_default: { + ramdisk: true, + vendor_ramdisk: false, + }, + }, + }, +} + +avb_keys_prebuilt_avb { + name: "s-developer-gsi.avbpubkey", + src: "s-developer-gsi.avbpubkey", + soong_config_variables: { + BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT: { + ramdisk: false, + vendor_ramdisk: true, + conditions_default: { + ramdisk: true, + vendor_ramdisk: false, + }, + }, + }, +} diff --git a/rootdir/avb/Android.mk b/rootdir/avb/Android.mk deleted file mode 100644 index 8cf317231..000000000 --- a/rootdir/avb/Android.mk +++ /dev/null @@ -1,56 +0,0 @@ -LOCAL_PATH:= $(call my-dir) - -ifeq ($(BOARD_MOVE_GSI_AVB_KEYS_TO_VENDOR_BOOT),true) # AVB keys are installed to vendor ramdisk - ifeq ($(BOARD_MOVE_RECOVERY_RESOURCES_TO_VENDOR_BOOT),true) # no dedicated recovery partition - my_gsi_avb_keys_path := $(TARGET_VENDOR_RAMDISK_OUT)/first_stage_ramdisk/avb - else # device has a dedicated recovery partition - my_gsi_avb_keys_path := $(TARGET_VENDOR_RAMDISK_OUT)/avb - endif -else - ifeq ($(BOARD_USES_RECOVERY_AS_BOOT),true) # no dedicated recovery partition - my_gsi_avb_keys_path := $(TARGET_RECOVERY_ROOT_OUT)/first_stage_ramdisk/avb - else # device has a dedicated recovery partition - my_gsi_avb_keys_path := $(TARGET_RAMDISK_OUT)/avb - endif -endif - -####################################### -# q-developer-gsi.avbpubkey -include $(CLEAR_VARS) - -LOCAL_MODULE := q-developer-gsi.avbpubkey -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_MODULE_CLASS := ETC -LOCAL_SRC_FILES := $(LOCAL_MODULE) -LOCAL_MODULE_PATH := $(my_gsi_avb_keys_path) - -include $(BUILD_PREBUILT) - -####################################### -# r-developer-gsi.avbpubkey -include $(CLEAR_VARS) - -LOCAL_MODULE := r-developer-gsi.avbpubkey -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_MODULE_CLASS := ETC -LOCAL_SRC_FILES := $(LOCAL_MODULE) -LOCAL_MODULE_PATH := $(my_gsi_avb_keys_path) - -include $(BUILD_PREBUILT) - -####################################### -# s-developer-gsi.avbpubkey -include $(CLEAR_VARS) - -LOCAL_MODULE := s-developer-gsi.avbpubkey -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_MODULE_CLASS := ETC -LOCAL_SRC_FILES := $(LOCAL_MODULE) -LOCAL_MODULE_PATH := $(my_gsi_avb_keys_path) - -include $(BUILD_PREBUILT) - -my_gsi_avb_keys_path := From b335f4d4bee8607e177dd664af5835e837b0ea53 Mon Sep 17 00:00:00 2001 From: Nelson Li Date: Wed, 17 Jul 2024 10:16:39 +0800 Subject: [PATCH 039/158] Convert `asan.options` and `asan_extract` to Android.bp In the original Android.mk, `asan.option` is only enabled when "address" is present in SANITIZE_TARGET. `asan_extract` is only enabled when `SANITIZE_TARGET_SYSTEM` is `true` and `address` is present in SANITIZE_TARGET. However, in the normal build system design, a module should `not` decide when it should be enabled. Therefore, the `ifeq` condition should be directly removed during conversion. The correct approach is: Wait until converting `init.environ.rc`, then use `select` or similar methods to determine whether it should use `asan.option` or `asan_extract`. Bug: 353164536 Test: SANITIZE_TARGET=address m init.environ.rc SANITIZE_TARGET_SYSTEM=true SANITIZE_TARGET=address m init.environ.rc Change-Id: I69af3199536a3ba9f54dcfa198295826303e2d22 --- rootdir/Android.bp | 15 ++++++++++++++- rootdir/Android.mk | 27 --------------------------- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/rootdir/Android.bp b/rootdir/Android.bp index e8f7627f9..7105ed52a 100644 --- a/rootdir/Android.bp +++ b/rootdir/Android.bp @@ -114,10 +114,23 @@ prebuilt_etc { sub_dir: "init", } +prebuilt_etc { + name: "asan.options", + src: "asan.options", +} + +sh_binary { + name: "asan_extract", + src: "asan_extract.sh", + init_rc: ["asan_extract.rc"], + // We need bzip2 on device for extraction. + required: ["bzip2"], +} + llndk_libraries_txt { name: "llndk.libraries.txt", } sanitizer_libraries_txt { name: "sanitizer.libraries.txt", -} \ No newline at end of file +} diff --git a/rootdir/Android.mk b/rootdir/Android.mk index 4c1f2e4f8..e6ccda788 100644 --- a/rootdir/Android.mk +++ b/rootdir/Android.mk @@ -3,39 +3,12 @@ LOCAL_PATH:= $(call my-dir) $(eval $(call declare-1p-copy-files,system/core/rootdir,)) ####################################### -# asan.options ifneq ($(filter address,$(SANITIZE_TARGET)),) - -include $(CLEAR_VARS) - -LOCAL_MODULE := asan.options -LOCAL_LICENSE_KINDS := SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS := notice -LOCAL_MODULE_CLASS := ETC -LOCAL_SRC_FILES := $(LOCAL_MODULE) -LOCAL_MODULE_PATH := $(TARGET_OUT) - -include $(BUILD_PREBUILT) - -# ASAN extration. ASAN_EXTRACT_FILES := ifeq ($(SANITIZE_TARGET_SYSTEM),true) -include $(CLEAR_VARS) -LOCAL_MODULE:= asan_extract -LOCAL_LICENSE_KINDS:= SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS:= notice -LOCAL_MODULE_TAGS := optional -LOCAL_MODULE_CLASS := EXECUTABLES -LOCAL_SRC_FILES := asan_extract.sh -LOCAL_INIT_RC := asan_extract.rc -# We need bzip2 on device for extraction. -LOCAL_REQUIRED_MODULES := bzip2 -include $(BUILD_PREBUILT) ASAN_EXTRACT_FILES := asan_extract endif - endif - ####################################### # init.environ.rc From 1315ca7b06b76ac50538c7444e7b2acb0b02f807 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Wed, 17 Jul 2024 14:48:57 -0700 Subject: [PATCH 040/158] Log lock time if it takes too long Test: th Bug: 352332753 Change-Id: Ie7397f7027a597cebb33206a6df845918bc3dab5 --- fs_mgr/libsnapshot/snapshot.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 4d5531535..32f6d8913 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -2892,6 +2893,7 @@ bool SnapshotManager::UnmapAllSnapshots(LockedFile* lock) { auto SnapshotManager::OpenFile(const std::string& file, int lock_flags) -> std::unique_ptr { + const auto start = std::chrono::system_clock::now(); unique_fd fd(open(file.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW)); if (fd < 0) { PLOG(ERROR) << "Open failed: " << file; @@ -2904,6 +2906,11 @@ auto SnapshotManager::OpenFile(const std::string& file, // For simplicity, we want to CHECK that lock_mode == LOCK_EX, in some // calls, so strip extra flags. int lock_mode = lock_flags & (LOCK_EX | LOCK_SH); + const auto end = std::chrono::system_clock::now(); + const auto duration_ms = std::chrono::duration_cast(end - start); + if (duration_ms >= 1000ms) { + LOG(INFO) << "Taking lock on " << file << " took " << duration_ms.count() << "ms"; + } return std::make_unique(file, std::move(fd), lock_mode); } From 9c77e66d56fe20c8763d007bd0722147816636b6 Mon Sep 17 00:00:00 2001 From: Mike McTernan Date: Fri, 12 Jul 2024 20:40:53 +0100 Subject: [PATCH 041/158] storageproxy: set a property when secure storage becomes rw Bug: 350362101 Test: ABTD Change-Id: I877a62e5c6337f31ffe63a4fd3cdeb54b69a8ae6 --- trusty/storage/proxy/storage.c | 38 +++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/trusty/storage/proxy/storage.c b/trusty/storage/proxy/storage.c index 6d0c61671..ca39f6aab 100644 --- a/trusty/storage/proxy/storage.c +++ b/trusty/storage/proxy/storage.c @@ -55,7 +55,7 @@ static const char *ssdir_name; static struct storage_mapping_node* storage_mapping_head; /* - * Property set to 1 after we have opened a file under ssdir_name. The backing + * Properties set to 1 after we have opened a file under ssdir_name. The backing * files for both TD and TDP are currently located under /data/vendor/ss and can * only be opened once userdata is mounted. This storageproxyd service is * restarted when userdata is available, which causes the Trusty storage service @@ -64,11 +64,16 @@ static struct storage_mapping_node* storage_mapping_head; * ports will be available (although they may block if still being initialized), * and connections will not be reset after this point (assuming the * storageproxyd service stays running). + * + * fs_ready - secure storage is read-only (due to checkpointing after upgrade) + * fs_ready_rw - secure storage is readable and writable */ #define FS_READY_PROPERTY "ro.vendor.trusty.storage.fs_ready" +#define FS_READY_RW_PROPERTY "ro.vendor.trusty.storage.fs_ready_rw" /* has FS_READY_PROPERTY been set? */ -static bool fs_ready_initialized = false; +static bool fs_ready_set = false; +static bool fs_ready_rw_set = false; static enum sync_state fs_state; static enum sync_state fd_state[FD_TBL_SIZE]; @@ -80,6 +85,17 @@ static struct { uint8_t data[MAX_READ_SIZE]; } read_rsp; +static bool property_set_helper(const char* prop) { + int rc = property_set(prop, "1"); + if (rc == 0) { + ALOGI("Set property %s\n", prop); + } else { + ALOGE("Could not set property %s, rc: %d\n", prop, rc); + } + + return rc == 0; +} + static uint32_t insert_fd(int open_flags, int fd, struct storage_mapping_node* node) { uint32_t handle = fd; @@ -520,12 +536,20 @@ int storage_file_open(struct storage_msg* msg, const void* r, size_t req_len, path = NULL; /* a backing file has been opened, notify any waiting init steps */ - if (!fs_ready_initialized) { - rc = property_set(FS_READY_PROPERTY, "1"); - if (rc == 0) { - fs_ready_initialized = true; + if (!fs_ready_set || !fs_ready_rw_set) { + bool is_checkpoint_active = false; + + rc = is_data_checkpoint_active(&is_checkpoint_active); + if (rc != 0) { + ALOGE("is_data_checkpoint_active() failed (%d)\n", rc); } else { - ALOGE("Could not set property %s, rc: %d\n", FS_READY_PROPERTY, rc); + if (!fs_ready_rw_set && !is_checkpoint_active) { + fs_ready_rw_set = property_set_helper(FS_READY_RW_PROPERTY); + } + + if (!fs_ready_set) { + fs_ready_set = property_set_helper(FS_READY_PROPERTY); + } } } From 47a95af84f59acf66d27fbe61341d32f4c324bc5 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Thu, 18 Jul 2024 09:10:56 -0700 Subject: [PATCH 042/158] Add debug logs to understand slow update start issue Test: th Bug: 352332753 Change-Id: I2d8f99217722479dcae2e2ea0f74bbf112725ce2 --- fs_mgr/libsnapshot/snapshot.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 32f6d8913..66743789a 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -2866,10 +2866,12 @@ bool SnapshotManager::UnmapAllSnapshots() { } bool SnapshotManager::UnmapAllSnapshots(LockedFile* lock) { + LOG(INFO) << "Lock acquired for " << __FUNCTION__; std::vector snapshots; if (!ListSnapshots(lock, &snapshots)) { return false; } + LOG(INFO) << "Found " << snapshots.size() << " partitions with snapshots"; for (const auto& snapshot : snapshots) { if (!UnmapPartitionWithSnapshot(lock, snapshot)) { From c6dfdbb761da5709b0cb9379ea2a9908775dd2fd Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Thu, 11 Jul 2024 10:56:44 -0700 Subject: [PATCH 043/158] Set block device as RO/RW before mount umount is faster if an RO mount is on top of an RO block device. Test: th Bug: 349507086 Change-Id: I621cafd5b15c2c4e104ae7678a1bcf2588fe29b6 --- fs_mgr/fs_mgr.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 5156754a8..ea2e3a8c7 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -826,6 +826,9 @@ static int __mount(const std::string& source, const std::string& target, const F if (read_only) { mountflags |= MS_RDONLY; } + if (!fs_mgr_set_blk_ro(source, read_only)) { + PLOG(ERROR) << "Failed to set " << source << " as " << (read_only ? "RO" : "RW"); + } int ret = 0; int save_errno = 0; int gc_allowance = 0; @@ -880,9 +883,6 @@ static int __mount(const std::string& source, const std::string& target, const F } PINFO << __FUNCTION__ << "(source=" << source << source_missing << ",target=" << target << target_missing << ",type=" << entry.fs_type << ")=" << ret; - if ((ret == 0) && (mountflags & MS_RDONLY) != 0) { - fs_mgr_set_blk_ro(source); - } if (ret == 0) { android::base::SetProperty("ro.boottime.init.mount." + Basename(target), std::to_string(t.duration().count())); From b885e4ad533ea4d12e18a0147d298be0cd8cec72 Mon Sep 17 00:00:00 2001 From: Tiffany Yang Date: Wed, 22 May 2024 21:04:38 -0700 Subject: [PATCH 044/158] init: Wait for /dev/hvc1 during ARCVM first-stage mount This commit introduces a function to allow ARC-specific logic during first-stage mount by checking for the existence of an indicator file at the path "/is_arcvm". ARC uses the virtio-console device `hvc1` to pass byte data to Android before second-stage init. Ensure that /dev/hvc1 is initialized before ARCVM continues booting, but allow other devices to bypass this device initialization. Bug: 325538592 Test: boot ARC, ensure /dev/hvc1 can be read during PropertyInit Change-Id: Ic258b7b004b59da462f4990131a5c11fc94eca62 --- init/first_stage_mount.cpp | 5 +++++ init/selinux.cpp | 2 ++ init/util.h | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 55cce6eaa..927b45f22 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -305,6 +305,11 @@ bool FirstStageMountVBootV2::InitDevices() { return false; } } + + if (IsArcvm() && !block_dev_init_.InitHvcDevice("hvc1")) { + return false; + } + return true; } diff --git a/init/selinux.cpp b/init/selinux.cpp index c2d9b8d28..01af2b64d 100644 --- a/init/selinux.cpp +++ b/init/selinux.cpp @@ -474,6 +474,8 @@ void SelinuxRestoreContext() { RestoreconIfExists(SnapshotManager::GetGlobalRollbackIndicatorPath().c_str(), 0); RestoreconIfExists("/metadata/gsi", SELINUX_ANDROID_RESTORECON_RECURSE | SELINUX_ANDROID_RESTORECON_SKIP_SEHASH); + + RestoreconIfExists("/dev/hvc1", 0); } int SelinuxKlogCallback(int type, const char* fmt, ...) { diff --git a/init/util.h b/init/util.h index aa24123df..056539181 100644 --- a/init/util.h +++ b/init/util.h @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -108,6 +109,10 @@ inline constexpr bool IsMicrodroid() { #endif } +inline bool IsArcvm() { + return !access("/is_arcvm", F_OK); +} + bool Has32BitAbi(); std::string GetApexNameFromFileName(const std::string& path); From b525463558d4776c12f2e5328987f0131ad1b737 Mon Sep 17 00:00:00 2001 From: Mike McTernan Date: Thu, 18 Jul 2024 14:42:28 +0100 Subject: [PATCH 045/158] init: set a new trigger when a vold checkpoint is committed Add post-fs-data-checkpointed trigger when vold completes checkpointing after an OTA update. Bug: 350362101 Test: ABTD Change-Id: I647a73a942174015b46c5f40bd8f8d3347977ecd --- init/README.md | 8 +++++--- rootdir/init.rc | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/init/README.md b/init/README.md index 11c4e1cf3..0bb26e891 100644 --- a/init/README.md +++ b/init/README.md @@ -499,9 +499,11 @@ have been omitted. 4. `late-fs` - Mount partitions marked as latemounted. 5. `post-fs-data` - Mount and configure `/data`; set up encryption. `/metadata` is reformatted here if it couldn't mount in first-stage init. - 6. `zygote-start` - Start the zygote. - 7. `early-boot` - After zygote has started. - 8. `boot` - After `early-boot` actions have completed. + 6. `post-fs-data-checkpointed` - Triggered when vold has completed committing a checkpoint + after an OTA update. Not triggered if checkpointing is not needed or supported. + 7. `zygote-start` - Start the zygote. + 8. `early-boot` - After zygote has started. + 9. `boot` - After `early-boot` actions have completed. Commands -------- diff --git a/rootdir/init.rc b/rootdir/init.rc index 2443b7cf6..b804c1bde 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -1087,6 +1087,9 @@ on post-fs-data # Update dm-verity state and set partition.*.verified properties. verity_update_state +on property:vold.checkpoint_committed=1 + trigger post-fs-data-checkpointed + # It is recommended to put unnecessary data/ initialization from post-fs-data # to start-zygote in device's init.rc to unblock zygote start. on zygote-start From d959fc72c13bb0154bfcaf52cbd33aefeba61a34 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 22 Jul 2024 11:24:03 +0900 Subject: [PATCH 046/158] Update visibility according to the change AVF directories layout Bug: 352458998 Test: m nothing Change-Id: Ibdae7e76a66104cc644b7f82059a2dc5f08659d0 --- init/Android.bp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/init/Android.bp b/init/Android.bp index c70a5deaa..ffb638063 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -330,7 +330,7 @@ cc_binary { static_libs: ["libinit.microdroid"], cflags: ["-DMICRODROID=1"], no_full_install: true, - visibility: ["//packages/modules/Virtualization/microdroid"], + visibility: ["//packages/modules/Virtualization/build/microdroid"], } soong_config_module_type { From 4363e8c3a781df3569ed02983e74257cfd3b910b Mon Sep 17 00:00:00 2001 From: Jack Wu Date: Wed, 10 Jul 2024 14:47:14 +0800 Subject: [PATCH 047/158] charger: fix show qustion mark when start to draw UI skip drawing UI if battery status is not ready. Bug: 343093322 Test: confirm offmode charge UI behavior Change-Id: Ic5ed4577502bea22f693b7ec0b577dc25ba24712 Signed-off-by: Jack Wu --- healthd/healthd_mode_charger.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/healthd/healthd_mode_charger.cpp b/healthd/healthd_mode_charger.cpp index 26af13b31..54b469be3 100644 --- a/healthd/healthd_mode_charger.cpp +++ b/healthd/healthd_mode_charger.cpp @@ -306,8 +306,8 @@ void Charger::UpdateScreenState(int64_t now) { if (!batt_anim_.run || now < next_screen_transition_) return; - // If battery level is not ready, keep checking in the defined time - if (health_info_.battery_level == 0 && health_info_.battery_status == BatteryStatus::UNKNOWN) { + // If battery status is not ready, keep checking in the defined time + if (health_info_.battery_status == BatteryStatus::UNKNOWN) { if (wait_batt_level_timestamp_ == 0) { // Set max delay time and skip drawing screen wait_batt_level_timestamp_ = now + MAX_BATT_LEVEL_WAIT_TIME; @@ -317,7 +317,7 @@ void Charger::UpdateScreenState(int64_t now) { // Do nothing, keep waiting return; } - // If timeout and battery level is still not ready, draw unknown battery + // If timeout and battery status is still not ready, draw unknown battery } if (healthd_draw_ == nullptr) return; From 1b76cb48ef83bceb2a5f85ec0e18f69dde9ba0ae Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Mon, 1 Jul 2024 10:07:43 -0700 Subject: [PATCH 048/158] libsnapshot: Address GRF config when updating from Android S config Bug: 333854394 Test: 1)S+U > V+V , FULL OTA pass 2)S+U > S+V, FULL OTA pass Change-Id: Ia8e6db89c3395930856ace8940424e60cae92375 Signed-off-by: Akilesh Kailash --- fs_mgr/libsnapshot/snapshot.cpp | 45 ++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 66743789a..11aa3f934 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -91,6 +92,8 @@ static constexpr char kBootIndicatorPath[] = "/metadata/ota/snapshot-boot"; static constexpr char kRollbackIndicatorPath[] = "/metadata/ota/rollback-indicator"; static constexpr char kSnapuserdFromSystem[] = "/metadata/ota/snapuserd-from-system"; static constexpr auto kUpdateStateCheckInterval = 2s; +static constexpr char kOtaFileContext[] = "u:object_r:ota_metadata_file:s0"; + /* * The readahead size is set to 32kb so that * there is no significant memory pressure (/proc/pressure/memory) during boot. @@ -2135,6 +2138,24 @@ bool SnapshotManager::MarkSnapuserdFromSystem() { PLOG(ERROR) << "Unable to write to vendor update path: " << path; return false; } + + unique_fd fd(open(path.c_str(), O_PATH)); + if (fd < 0) { + PLOG(ERROR) << "Failed to open file: " << path; + return false; + } + + /* + * This function is invoked by first stage init and hence we need to + * explicitly set the correct selinux label for this file as update_engine + * will try to remove this file later on once the snapshot merge is + * complete. + */ + if (fsetxattr(fd.get(), XATTR_NAME_SELINUX, kOtaFileContext, strlen(kOtaFileContext) + 1, 0) < + 0) { + PLOG(ERROR) << "fsetxattr for the path: " << path << " failed"; + } + return true; } @@ -2185,18 +2206,24 @@ bool SnapshotManager::MarkSnapuserdFromSystem() { * */ bool SnapshotManager::IsLegacySnapuserdPostReboot() { - if (is_legacy_snapuserd_.has_value() && is_legacy_snapuserd_.value() == true) { - auto slot = GetCurrentSlot(); - if (slot == Slot::Target) { - // If this marker is present, then daemon can handle userspace - // snapshots; also, it indicates that the vendor partition was - // updated from Android 12. - if (access(GetSnapuserdFromSystemPath().c_str(), F_OK) == 0) { - return false; - } + auto slot = GetCurrentSlot(); + if (slot == Slot::Target) { + /* + If this marker is present, the daemon can handle userspace snapshots. + During post-OTA reboot, this implies that the vendor partition is + Android 13 or higher. If the snapshots were created on an + Android 12 vendor, this means the vendor partition has been updated. + */ + if (access(GetSnapuserdFromSystemPath().c_str(), F_OK) == 0) { + is_snapshot_userspace_ = true; + return false; + } + // If the marker isn't present and if the vendor is still in Android 12 + if (is_legacy_snapuserd_.has_value() && is_legacy_snapuserd_.value() == true) { return true; } } + return false; } From 3360d387d205ce3f89b311c20f25adbcec1bebbc Mon Sep 17 00:00:00 2001 From: Betty Zhou Date: Fri, 19 Jul 2024 13:40:53 -0700 Subject: [PATCH 049/158] Add adb-remount-sh to presubmit & kernel-presubmit group. Test: test-mapping test Bug: 231996550 Change-Id: Ic03dcf8c316e64bc5d6b106b598c0af590d24802 --- fs_mgr/TEST_MAPPING | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs_mgr/TEST_MAPPING b/fs_mgr/TEST_MAPPING index 32e8b88e1..192232d6c 100644 --- a/fs_mgr/TEST_MAPPING +++ b/fs_mgr/TEST_MAPPING @@ -33,6 +33,9 @@ } ], "kernel-presubmit": [ + { + "name": "adb-remount-sh" + }, { "name": "libdm_test" }, From 6906249312be46b64b6b6221ba33bad196bf6cd6 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Thu, 11 Jul 2024 10:06:47 -0700 Subject: [PATCH 050/158] Skip tests if vendor partition is on Android S Bug: 349278999 Test: vts_libsnapshot_test on GSI config Change-Id: I6826b287565e8a78bea21b4830ad30f1c30a01bf Signed-off-by: Akilesh Kailash --- .../include_test/libsnapshot/test_helpers.h | 18 ++++++++++++++++++ fs_mgr/libsnapshot/snapshot_test.cpp | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h index 90813fe79..0afd8bd22 100644 --- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h @@ -30,6 +30,8 @@ #include #include +#include "utility.h" + namespace android { namespace snapshot { @@ -234,5 +236,21 @@ bool IsVirtualAbEnabled(); #define RETURN_IF_NON_VIRTUAL_AB() RETURN_IF_NON_VIRTUAL_AB_MSG("") +#define SKIP_IF_VENDOR_ON_ANDROID_S() \ + do { \ + if (IsVendorFromAndroid12()) \ + GTEST_SKIP() << "Skip test as Vendor partition is on Android S"; \ + } while (0) + +#define RETURN_IF_VENDOR_ON_ANDROID_S_MSG(msg) \ + do { \ + if (IsVendorFromAndroid12()) { \ + std::cerr << (msg); \ + return; \ + } \ + } while (0) + +#define RETURN_IF_VENDOR_ON_ANDROID_S() RETURN_IF_VENDOR_ON_ANDROID_S_MSG("") + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 1435b12bf..07f13014d 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -122,6 +122,7 @@ class SnapshotTest : public ::testing::Test { LOG(INFO) << "Starting test: " << test_name_; SKIP_IF_NON_VIRTUAL_AB(); + SKIP_IF_VENDOR_ON_ANDROID_S(); SetupProperties(); if (!DeviceSupportsMode()) { @@ -168,6 +169,7 @@ class SnapshotTest : public ::testing::Test { void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); LOG(INFO) << "Tearing down SnapshotTest test: " << test_name_; @@ -1015,6 +1017,7 @@ class SnapshotUpdateTest : public SnapshotTest { public: void SetUp() override { SKIP_IF_NON_VIRTUAL_AB(); + SKIP_IF_VENDOR_ON_ANDROID_S(); SnapshotTest::SetUp(); if (!image_manager_) { @@ -1097,6 +1100,7 @@ class SnapshotUpdateTest : public SnapshotTest { } void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); LOG(INFO) << "Tearing down SnapshotUpdateTest test: " << test_name_; @@ -2833,6 +2837,7 @@ void SnapshotTestEnvironment::SetUp() { // that is fixed, don't call GTEST_SKIP here, but instead call GTEST_SKIP in individual test // suites. RETURN_IF_NON_VIRTUAL_AB_MSG("Virtual A/B is not enabled, skipping global setup.\n"); + RETURN_IF_VENDOR_ON_ANDROID_S_MSG("Test not enabled for Vendor on Android S.\n"); std::vector paths = { // clang-format off @@ -2887,6 +2892,8 @@ void SnapshotTestEnvironment::SetUp() { void SnapshotTestEnvironment::TearDown() { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); + if (super_images_ != nullptr) { DeleteBackingImage(super_images_.get(), "fake-super"); } From 5c1378a5ffaebee585cce4894e62993e5c407eeb Mon Sep 17 00:00:00 2001 From: Tommy Chiu Date: Wed, 15 May 2024 09:50:31 +0000 Subject: [PATCH 051/158] Add trusty_rkp_set_uds_cert for UdsCert provisioning Bug: 330791928 Test: trusty_rkp_set_uds_cert /data/rkp_uds_cert_test.xml Change-Id: I368be346197099ff6d3fe7a09d666791faada040 --- trusty/keymaster/Android.bp | 37 ++- .../trusty_keymaster/ipc/keymaster_ipc.h | 2 + .../set_uds_certs/rkp_uds_cert_test.xml | 42 +++ .../set_uds_certs/set_uds_certificates.cpp | 279 ++++++++++++++++++ 4 files changed, 358 insertions(+), 2 deletions(-) create mode 100644 trusty/keymaster/set_uds_certs/rkp_uds_cert_test.xml create mode 100644 trusty/keymaster/set_uds_certs/set_uds_certificates.cpp diff --git a/trusty/keymaster/Android.bp b/trusty/keymaster/Android.bp index b249013ba..aca59b6c6 100644 --- a/trusty/keymaster/Android.bp +++ b/trusty/keymaster/Android.bp @@ -44,7 +44,7 @@ cc_binary { "libtrusty", "libkeymaster_messages", "libkeymaster3device", - "android.hardware.keymaster@3.0" + "android.hardware.keymaster@3.0", ], } @@ -74,7 +74,7 @@ cc_binary { "libtrusty", "libkeymaster_messages", "libkeymaster4", - "android.hardware.keymaster@4.0" + "android.hardware.keymaster@4.0", ], vintf_fragments: ["4.0/android.hardware.keymaster@4.0-service.trusty.xml"], @@ -208,3 +208,36 @@ cc_binary { "-Werror", ], } + +prebuilt_etc { + name: "rkp_uds_cert_test.xml", + vendor: true, + src: "set_uds_certs/rkp_uds_cert_test.xml", +} + +cc_binary { + name: "trusty_rkp_set_uds_cert", + vendor: true, + + srcs: [ + "set_uds_certs/set_uds_certificates.cpp", + "ipc/trusty_keymaster_ipc.cpp", + ], + + local_include_dirs: ["include"], + + shared_libs: [ + "libc", + "libcrypto", + "liblog", + "libtrusty", + "libhardware", + "libkeymaster_messages", + "libutils", + "libxml2", + ], + cflags: [ + "-Wall", + "-Werror", + ], +} diff --git a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h index 09f696bb5..822e93334 100644 --- a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h +++ b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h @@ -78,6 +78,8 @@ enum keymaster_command : uint32_t { KM_SET_ATTESTATION_IDS = (0xc000 << KEYMASTER_REQ_SHIFT), KM_SET_ATTESTATION_IDS_KM3 = (0xc001 << KEYMASTER_REQ_SHIFT), KM_CONFIGURE_BOOT_PATCHLEVEL = (0xd000 << KEYMASTER_REQ_SHIFT), + KM_APPEND_UDS_CERT_CHAIN = (0xe0000 << KEYMASTER_REQ_SHIFT), + KM_CLEAR_UDS_CERT_CHAIN = (0xe0001 << KEYMASTER_REQ_SHIFT), }; #ifdef __ANDROID__ diff --git a/trusty/keymaster/set_uds_certs/rkp_uds_cert_test.xml b/trusty/keymaster/set_uds_certs/rkp_uds_cert_test.xml new file mode 100644 index 000000000..73b7b4c30 --- /dev/null +++ b/trusty/keymaster/set_uds_certs/rkp_uds_cert_test.xml @@ -0,0 +1,42 @@ + + + + 3 + +-----BEGIN CERTIFICATE----- +MIIBWTCB3qADAgECAgUA3q2+7zAMBggqhkjOPQQDAwUAMBIxEDAOBgNVBAMMB0dT +TUkxLjAwIhgPMjAyNDAxMDEwMDAwMDBaGA85OTk5MTIzMTIzNTk1OVowEjEQMA4G +A1UEAwwHR1NNSTEuMDB2MBAGByqGSM49AgEGBSuBBAAiA2IABFsNPdsPmx2NKNiT +7oReF36HTXjftRfGffYgPf/vEhrT7XLJ7dThkcb+OFYWYlOckdk3DRgqTNWQXsot +UsZhwRveU8huEjOBO5+dwDiWPs+s9jSRAn5inlTqJ0YGAmdHRzAMBggqhkjOPQQD +AwUAA2gAMGUCMQCuiJwmRWOgfWbqdSlnXfhCbphjdWc6sHelLkkM21vxQ3RZkhC2 +ERh90RA1rxB+XTgCMHZrYG3leS0PEoz5hUviv27LbBHBU6GeOzrS2e0VH0BMSNXN +iP9Wnit+mJw58niEGw== +-----END CERTIFICATE----- + + +-----BEGIN CERTIFICATE----- +MIIBWTCB3qADAgECAgUA3q2+7zAMBggqhkjOPQQDAwUAMBIxEDAOBgNVBAMMB0dT +TUkxLjAwIhgPMjAyNDAxMDEwMDAwMDBaGA85OTk5MTIzMTIzNTk1OVowEjEQMA4G +A1UEAwwHR1NNSTEuMDB2MBAGByqGSM49AgEGBSuBBAAiA2IABFsNPdsPmx2NKNiT +7oReF36HTXjftRfGffYgPf/vEhrT7XLJ7dThkcb+OFYWYlOckdk3DRgqTNWQXsot +UsZhwRveU8huEjOBO5+dwDiWPs+s9jSRAn5inlTqJ0YGAmdHRzAMBggqhkjOPQQD +AwUAA2gAMGUCMQCuiJwmRWOgfWbqdSlnXfhCbphjdWc6sHelLkkM21vxQ3RZkhC2 +ERh90RA1rxB+XTgCMHZrYG3leS0PEoz5hUviv27LbBHBU6GeOzrS2e0VH0BMSNXN +iP9Wnit+mJw58niEGw== +-----END CERTIFICATE----- + + +-----BEGIN CERTIFICATE----- +MIIBWTCB3qADAgECAgUA3q2+7zAMBggqhkjOPQQDAwUAMBIxEDAOBgNVBAMMB0dT +TUkxLjAwIhgPMjAyNDAxMDEwMDAwMDBaGA85OTk5MTIzMTIzNTk1OVowEjEQMA4G +A1UEAwwHR1NNSTEuMDB2MBAGByqGSM49AgEGBSuBBAAiA2IABFsNPdsPmx2NKNiT +7oReF36HTXjftRfGffYgPf/vEhrT7XLJ7dThkcb+OFYWYlOckdk3DRgqTNWQXsot +UsZhwRveU8huEjOBO5+dwDiWPs+s9jSRAn5inlTqJ0YGAmdHRzAMBggqhkjOPQQD +AwUAA2gAMGUCMQCuiJwmRWOgfWbqdSlnXfhCbphjdWc6sHelLkkM21vxQ3RZkhC2 +ERh90RA1rxB+XTgCMHZrYG3leS0PEoz5hUviv27LbBHBU6GeOzrS2e0VH0BMSNXN +iP9Wnit+mJw58niEGw== +-----END CERTIFICATE----- + + + diff --git a/trusty/keymaster/set_uds_certs/set_uds_certificates.cpp b/trusty/keymaster/set_uds_certs/set_uds_certificates.cpp new file mode 100644 index 000000000..13356a911 --- /dev/null +++ b/trusty/keymaster/set_uds_certs/set_uds_certificates.cpp @@ -0,0 +1,279 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +static const char* _sopts = "h"; +static const struct option _lopts[] = { + {"help", no_argument, 0, 'h'}, + {0, 0, 0, 0}, +}; + +static const char* usage = + "Usage: %s [options] xml-file\n" + "\n" + "options:\n" + " -h, --help prints this message and exit\n" + "\n"; + +static void print_usage_and_exit(const char* prog, int code) { + fprintf(stderr, usage, prog); + exit(code); +} + +static void parse_options(int argc, char** argv) { + int c; + int oidx = 0; + + while (1) { + c = getopt_long(argc, argv, _sopts, _lopts, &oidx); + if (c == -1) { + break; /* done */ + } + + switch (c) { + case 'h': + print_usage_and_exit(argv[0], EXIT_SUCCESS); + break; + + default: + print_usage_and_exit(argv[0], EXIT_FAILURE); + } + } +} + +struct AppendUdsCertificateRequest : public keymaster::KeymasterMessage { + explicit AppendUdsCertificateRequest(int32_t ver = keymaster::kDefaultMessageVersion) + : KeymasterMessage(ver) {} + + size_t SerializedSize() const override { return cert_data.SerializedSize(); } + uint8_t* Serialize(uint8_t* buf, const uint8_t* end) const override { + return cert_data.Serialize(buf, end); + } + bool Deserialize(const uint8_t** buf_ptr, const uint8_t* end) override { + return cert_data.Deserialize(buf_ptr, end); + } + + keymaster::Buffer cert_data; +}; + +struct ClearUdsCertificateRequest : public keymaster::KeymasterMessage { + explicit ClearUdsCertificateRequest(int32_t ver = keymaster::kDefaultMessageVersion) + : KeymasterMessage(ver) {} + + size_t SerializedSize() const override { return 0; } + uint8_t* Serialize(uint8_t* buf, const uint8_t*) const override { return buf; } + bool Deserialize(const uint8_t**, const uint8_t*) override { return true; }; +}; + +struct KeymasterNoResponse : public keymaster::KeymasterResponse{ + explicit KeymasterNoResponse(int32_t ver = keymaster::kDefaultMessageVersion) + : keymaster::KeymasterResponse(ver) {} + + size_t NonErrorSerializedSize() const override { return 0; } + uint8_t* NonErrorSerialize(uint8_t* buf, const uint8_t*) const override { return buf; } + bool NonErrorDeserialize(const uint8_t**, const uint8_t*) override { return true; } +}; + +struct AppendUdsCertificateResponse : public KeymasterNoResponse {}; +struct ClearUdsCertificateResponse : public KeymasterNoResponse {}; + +static int set_uds_cert_bin(uint32_t cmd, const void* cert_data, size_t cert_data_size) { + int ret; + + AppendUdsCertificateRequest req; + req.cert_data.Reinitialize(cert_data, cert_data_size); + AppendUdsCertificateResponse rsp; + + ret = trusty_keymaster_send(cmd, req, &rsp); + if (ret) { + fprintf(stderr, "trusty_keymaster_send cmd 0x%x failed %d\n", cmd, ret); + return ret; + } + + return 0; +} + +static int set_uds_cert_pem(uint32_t cmd, const xmlChar* pemkey) { + int ret; + int sslret; + + /* Convert from pem to binary */ + BIO* bio = BIO_new_mem_buf(pemkey, xmlStrlen(pemkey)); + if (!bio) { + fprintf(stderr, "BIO_new_mem_buf failed\n"); + ERR_print_errors_fp(stderr); + return -1; + } + + char* key_name; + char* key_header; + uint8_t* key; + long keylen; + sslret = PEM_read_bio(bio, &key_name, &key_header, &key, &keylen); + BIO_free(bio); + + if (!sslret) { + fprintf(stderr, "PEM_read_bio failed\n"); + ERR_print_errors_fp(stderr); + return -1; + } + + /* Send key in binary format to trusty */ + ret = set_uds_cert_bin(cmd, key, keylen); + + OPENSSL_free(key_name); + OPENSSL_free(key_header); + OPENSSL_free(key); + + return ret; +} + +static int set_uds_cert(uint32_t cmd, const xmlChar* format, const xmlChar* str) { + int ret; + + if (xmlStrEqual(format, BAD_CAST "pem")) { + ret = set_uds_cert_pem(cmd, str); + } else { + printf("unsupported key/cert format: %s\n", format); + return -1; + } + return ret; +} + +// TODO: Guard by Production Mode +static int clear_cert_chain() { + int ret; + ClearUdsCertificateRequest req; + ClearUdsCertificateResponse rsp; + + ret = trusty_keymaster_send(KM_CLEAR_UDS_CERT_CHAIN, req, &rsp); + if (ret) { + fprintf(stderr, "%s: trusty_keymaster_send failed %d\n", __func__, ret); + return ret; + } + return 0; +} + +static int process_xml(xmlTextReaderPtr xml) { + int ret; + const xmlChar* element = NULL; + const xmlChar* element_format = NULL; + bool isPixelUdsCert = false; + + while ((ret = xmlTextReaderRead(xml)) == 1) { + int nodetype = xmlTextReaderNodeType(xml); + const xmlChar *name, *value; + name = xmlTextReaderConstName(xml); + switch (nodetype) { + case XML_READER_TYPE_ELEMENT: + element = name; + element_format = xmlTextReaderGetAttribute(xml, BAD_CAST "format"); + if (isPixelUdsCert || xmlStrEqual(name, BAD_CAST "PixelUdsCertificates")) { + // The first element name must be "PixelUdsCertificates" + isPixelUdsCert = true; + } else { + fprintf(stderr, "Not a PixelUdsCertificates: \"%s\"\n", name); + return -1; + } + if (xmlStrEqual(name, BAD_CAST "CertificateChain")) { + ret = clear_cert_chain(); + if (ret) { + fprintf(stderr, "%s: Clear cert chain cmd failed, %d\n", element, ret); + return ret; + } + printf("%s: Clear cert chain cmd done\n", element); + } + break; + case XML_READER_TYPE_TEXT: + value = xmlTextReaderConstValue(xml); + uint32_t cmd; + if (xmlStrEqual(element, BAD_CAST "Certificate")) { + cmd = KM_APPEND_UDS_CERT_CHAIN; + } else { + break; + } + + ret = set_uds_cert(cmd, element_format, value); + if (ret) { + fprintf(stderr, "%s, format %s: Cmd 0x%x failed, %d\n", element, element_format, + cmd, ret); + return ret; + } + printf("%s, format %s: Cmd 0x%x done\n", element, element_format, cmd); + break; + case XML_READER_TYPE_END_ELEMENT: + element = NULL; + break; + } + } + return ret; +} + +static int parse_and_provision_xml_file(const char* filename) { + int ret; + xmlTextReaderPtr xml = xmlReaderForFile(filename, NULL, 0); + if (!xml) { + fprintf(stderr, "failed to open %s\n", filename); + return -1; + } + + ret = process_xml(xml); + + xmlFreeTextReader(xml); + if (ret != 0) { + fprintf(stderr, "Failed to parse or process %s\n", filename); + return -1; + } + + return 0; +} + +int main(int argc, char** argv) { + int ret = 0; + + parse_options(argc, argv); + if (optind + 1 != argc) { + print_usage_and_exit(argv[0], EXIT_FAILURE); + } + + ret = trusty_keymaster_connect(); + if (ret) { + fprintf(stderr, "trusty_keymaster_connect failed %d\n", ret); + return EXIT_FAILURE; + } + + ret = parse_and_provision_xml_file(argv[optind]); + if (ret) { + fprintf(stderr, "parse_and_provision_xml_file failed %d\n", ret); + trusty_keymaster_disconnect(); + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; +} From 9088d1bb12b1a97f49055abef609e0fc48a10234 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 9 Jun 2023 09:52:49 +0900 Subject: [PATCH 052/158] init: non-crashing service can restart immediately This CL allows restart_period to be set to a value shorter than 5s. Previously this was prohibited to rate limit crashing services. That behavior is considered to be a bit too conservative because some services don't crash, but exit deliverately. adbd is the motivating example. When adb root or adb unroot is requested, it changes its mode of operation (via sysprop), exits itself, and restarts (by init) to enter into the mode. However, due to the 5s delay, the mode change can complete no earlier than 5 seconds after adbd was started last time. This can slow the mode change when it is requested right after the boot. With this CL, restart_period can be set to a value smaller than 5. And services like adbd can make use of it. However, in ordef to rate limit crashing service, the default is enforced if the service was crashed last time. In addition, such intended restart is not counted as crashes when monitoring successive crashes during booting. Bug: 286061817 Bug: 339844204 Test: /packages/modules/Virtualization/vm/vm_shell.sh start-microdroid \ --auto-connect -- --protected * with this change: within 2s * without this change: over 6s (cherry picked from https://android-review.googlesource.com/q/commit:0d277d777f290729fd9708b88f7ed57f0e0e49eb) Merged-In: I1b3f0c92d349e8c8760821cf50fb69997b67b242 Change-Id: I1b3f0c92d349e8c8760821cf50fb69997b67b242 --- init/README.md | 13 ++++++++----- init/service.cpp | 4 +++- init/service.h | 17 +++++++++++++++-- init/service_parser.cpp | 4 ++-- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/init/README.md b/init/README.md index aaafe7887..734e14eee 100644 --- a/init/README.md +++ b/init/README.md @@ -316,11 +316,14 @@ runs the service. intended to be used with the `exec_start` builtin for any must-have checks during boot. `restart_period ` -> If a non-oneshot service exits, it will be restarted at its start time plus - this period. It defaults to 5s to rate limit crashing services. - This can be increased for services that are meant to run periodically. For - example, it may be set to 3600 to indicate that the service should run every hour - or 86400 to indicate that the service should run every day. +> If a non-oneshot service exits, it will be restarted at its previous start time plus this period. + The default value is 5s. This can be used to implement periodic services together with the + `timeout_period` command below. For example, it may be set to 3600 to indicate that the service + should run every hour or 86400 to indicate that the service should run every day. This can be set + to a value shorter than 5s for example 0, but the minimum 5s delay is enforced if the restart was + due to a crash. This is to rate limit persistentally crashing services. In other words, + `` smaller than 5 is respected only when the service exits deliverately and successfully + (i.e. by calling exit(0)). `rlimit ` > This applies the given rlimit to the service. rlimits are inherited by child diff --git a/init/service.cpp b/init/service.cpp index bd704cf8e..7d83d60e4 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -298,6 +298,7 @@ void Service::Reap(const siginfo_t& siginfo) { pid_ = 0; flags_ &= (~SVC_RUNNING); start_order_ = 0; + was_last_exit_ok_ = siginfo.si_code == CLD_EXITED && siginfo.si_status == 0; // Oneshot processes go into the disabled state on exit, // except when manually restarted. @@ -321,7 +322,8 @@ void Service::Reap(const siginfo_t& siginfo) { // If we crash > 4 times in 'fatal_crash_window_' minutes or before boot_completed, // reboot into bootloader or set crashing property boot_clock::time_point now = boot_clock::now(); - if (((flags_ & SVC_CRITICAL) || is_process_updatable) && !(flags_ & SVC_RESTART)) { + if (((flags_ & SVC_CRITICAL) || is_process_updatable) && !(flags_ & SVC_RESTART) && + !was_last_exit_ok_) { bool boot_completed = GetBoolProperty("sys.boot_completed", false); if (now < time_crashed_ + fatal_crash_window_ || !boot_completed) { if (++crash_count_ > 4) { diff --git a/init/service.h b/init/service.h index c314aa1c6..f5f361901 100644 --- a/init/service.h +++ b/init/service.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -113,6 +114,7 @@ class Service { pid_t pid() const { return pid_; } android::base::boot_clock::time_point time_started() const { return time_started_; } int crash_count() const { return crash_count_; } + int was_last_exit_ok() const { return was_last_exit_ok_; } uid_t uid() const { return proc_attr_.uid; } gid_t gid() const { return proc_attr_.gid; } int namespace_flags() const { return namespaces_.flags; } @@ -128,7 +130,15 @@ class Service { bool process_cgroup_empty() const { return process_cgroup_empty_; } unsigned long start_order() const { return start_order_; } void set_sigstop(bool value) { sigstop_ = value; } - std::chrono::seconds restart_period() const { return restart_period_; } + std::chrono::seconds restart_period() const { + // If the service exited abnormally or due to timeout, late limit the restart even if + // restart_period is set to a very short value. + // If not, i.e. restart after a deliberate and successful exit, respect the period. + if (!was_last_exit_ok_) { + return std::max(restart_period_, default_restart_period_); + } + return restart_period_; + } std::optional timeout_period() const { return timeout_period_; } const std::vector& args() const { return args_; } bool is_updatable() const { return updatable_; } @@ -171,6 +181,8 @@ class Service { int crash_count_; // number of times crashed within window std::chrono::minutes fatal_crash_window_ = 4min; // fatal() when more than 4 crashes in it std::optional fatal_reboot_target_; // reboot target of fatal handler + bool was_last_exit_ok_ = + true; // true if the service never exited, or exited with status code 0 std::optional capabilities_; ProcessAttributes proc_attr_; @@ -211,7 +223,8 @@ class Service { bool sigstop_ = false; - std::chrono::seconds restart_period_ = 5s; + const std::chrono::seconds default_restart_period_ = 5s; + std::chrono::seconds restart_period_ = default_restart_period_; std::optional timeout_period_; bool updatable_ = false; diff --git a/init/service_parser.cpp b/init/service_parser.cpp index 1ee309d98..cc39565de 100644 --- a/init/service_parser.cpp +++ b/init/service_parser.cpp @@ -364,8 +364,8 @@ Result ServiceParser::ParseRebootOnFailure(std::vector&& args Result ServiceParser::ParseRestartPeriod(std::vector&& args) { int period; - if (!ParseInt(args[1], &period, 5)) { - return Error() << "restart_period value must be an integer >= 5"; + if (!ParseInt(args[1], &period, 0)) { + return Error() << "restart_period value must be an integer >= 0"; } service_->restart_period_ = std::chrono::seconds(period); return {}; From 555d2393b6ba28de4cc84bc23e0e87957e086756 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 9 Jun 2023 09:52:49 +0900 Subject: [PATCH 053/158] init: non-crashing service can restart immediately This CL allows restart_period to be set to a value shorter than 5s. Previously this was prohibited to rate limit crashing services. That behavior is considered to be a bit too conservative because some services don't crash, but exit deliverately. adbd is the motivating example. When adb root or adb unroot is requested, it changes its mode of operation (via sysprop), exits itself, and restarts (by init) to enter into the mode. However, due to the 5s delay, the mode change can complete no earlier than 5 seconds after adbd was started last time. This can slow the mode change when it is requested right after the boot. With this CL, restart_period can be set to a value smaller than 5. And services like adbd can make use of it. However, in ordef to rate limit crashing service, the default is enforced if the service was crashed last time. In addition, such intended restart is not counted as crashes when monitoring successive crashes during booting. Bug: 286061817 Bug: 339844204 Test: /packages/modules/Virtualization/vm/vm_shell.sh start-microdroid \ --auto-connect -- --protected * with this change: within 2s * without this change: over 6s (cherry picked from https://android-review.googlesource.com/q/commit:0d277d777f290729fd9708b88f7ed57f0e0e49eb) Merged-In: I1b3f0c92d349e8c8760821cf50fb69997b67b242 Change-Id: I1b3f0c92d349e8c8760821cf50fb69997b67b242 --- init/README.md | 13 ++++++++----- init/service.cpp | 4 +++- init/service.h | 17 +++++++++++++++-- init/service_parser.cpp | 4 ++-- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/init/README.md b/init/README.md index 4a262c920..e1f506ce6 100644 --- a/init/README.md +++ b/init/README.md @@ -279,11 +279,14 @@ runs the service. intended to be used with the `exec_start` builtin for any must-have checks during boot. `restart_period ` -> If a non-oneshot service exits, it will be restarted at its start time plus - this period. It defaults to 5s to rate limit crashing services. - This can be increased for services that are meant to run periodically. For - example, it may be set to 3600 to indicate that the service should run every hour - or 86400 to indicate that the service should run every day. +> If a non-oneshot service exits, it will be restarted at its previous start time plus this period. + The default value is 5s. This can be used to implement periodic services together with the + `timeout_period` command below. For example, it may be set to 3600 to indicate that the service + should run every hour or 86400 to indicate that the service should run every day. This can be set + to a value shorter than 5s for example 0, but the minimum 5s delay is enforced if the restart was + due to a crash. This is to rate limit persistentally crashing services. In other words, + `` smaller than 5 is respected only when the service exits deliverately and successfully + (i.e. by calling exit(0)). `rlimit ` > This applies the given rlimit to the service. rlimits are inherited by child diff --git a/init/service.cpp b/init/service.cpp index c3069f5b2..a423656ec 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -289,6 +289,7 @@ void Service::Reap(const siginfo_t& siginfo) { pid_ = 0; flags_ &= (~SVC_RUNNING); start_order_ = 0; + was_last_exit_ok_ = siginfo.si_code == CLD_EXITED && siginfo.si_status == 0; // Oneshot processes go into the disabled state on exit, // except when manually restarted. @@ -312,7 +313,8 @@ void Service::Reap(const siginfo_t& siginfo) { // If we crash > 4 times in 'fatal_crash_window_' minutes or before boot_completed, // reboot into bootloader or set crashing property boot_clock::time_point now = boot_clock::now(); - if (((flags_ & SVC_CRITICAL) || is_process_updatable) && !(flags_ & SVC_RESTART)) { + if (((flags_ & SVC_CRITICAL) || is_process_updatable) && !(flags_ & SVC_RESTART) && + !was_last_exit_ok_) { bool boot_completed = GetBoolProperty("sys.boot_completed", false); if (now < time_crashed_ + fatal_crash_window_ || !boot_completed) { if (++crash_count_ > 4) { diff --git a/init/service.h b/init/service.h index 043555fa4..41d929956 100644 --- a/init/service.h +++ b/init/service.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -109,6 +110,7 @@ class Service { pid_t pid() const { return pid_; } android::base::boot_clock::time_point time_started() const { return time_started_; } int crash_count() const { return crash_count_; } + int was_last_exit_ok() const { return was_last_exit_ok_; } uid_t uid() const { return proc_attr_.uid; } gid_t gid() const { return proc_attr_.gid; } int namespace_flags() const { return namespaces_.flags; } @@ -124,7 +126,15 @@ class Service { bool process_cgroup_empty() const { return process_cgroup_empty_; } unsigned long start_order() const { return start_order_; } void set_sigstop(bool value) { sigstop_ = value; } - std::chrono::seconds restart_period() const { return restart_period_; } + std::chrono::seconds restart_period() const { + // If the service exited abnormally or due to timeout, late limit the restart even if + // restart_period is set to a very short value. + // If not, i.e. restart after a deliberate and successful exit, respect the period. + if (!was_last_exit_ok_) { + return std::max(restart_period_, default_restart_period_); + } + return restart_period_; + } std::optional timeout_period() const { return timeout_period_; } const std::vector& args() const { return args_; } bool is_updatable() const { return updatable_; } @@ -158,6 +168,8 @@ class Service { int crash_count_; // number of times crashed within window std::chrono::minutes fatal_crash_window_ = 4min; // fatal() when more than 4 crashes in it std::optional fatal_reboot_target_; // reboot target of fatal handler + bool was_last_exit_ok_ = + true; // true if the service never exited, or exited with status code 0 std::optional capabilities_; ProcessAttributes proc_attr_; @@ -198,7 +210,8 @@ class Service { bool sigstop_ = false; - std::chrono::seconds restart_period_ = 5s; + const std::chrono::seconds default_restart_period_ = 5s; + std::chrono::seconds restart_period_ = default_restart_period_; std::optional timeout_period_; bool updatable_ = false; diff --git a/init/service_parser.cpp b/init/service_parser.cpp index 57c311a52..75e93e5ff 100644 --- a/init/service_parser.cpp +++ b/init/service_parser.cpp @@ -363,8 +363,8 @@ Result ServiceParser::ParseRebootOnFailure(std::vector&& args Result ServiceParser::ParseRestartPeriod(std::vector&& args) { int period; - if (!ParseInt(args[1], &period, 5)) { - return Error() << "restart_period value must be an integer >= 5"; + if (!ParseInt(args[1], &period, 0)) { + return Error() << "restart_period value must be an integer >= 0"; } service_->restart_period_ = std::chrono::seconds(period); return {}; From 1fbba3ed0d8e9566d28733907bfa0285bbbf3a11 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Tue, 28 May 2024 16:36:52 -0700 Subject: [PATCH 054/158] libsnapshot: cap merge op count Set op processing window during snapshot merge from the build. The lower the merge count, the lower the memory strain during the merge process Bug: 332255580 Test: verify merge_size propogates to snapuserd daemon Change-Id: Ic7770115bca963d923a7a276897c5deac30f9faf --- fs_mgr/libsnapshot/android/snapshot/snapshot.proto | 4 ++++ fs_mgr/libsnapshot/include/libsnapshot/snapshot.h | 2 ++ fs_mgr/libsnapshot/snapshot.cpp | 13 +++++++++++++ fs_mgr/libsnapshot/snapuserd/snapuserd_daemon.cpp | 6 +++--- .../snapuserd/user-space-merge/extractor.cpp | 2 +- .../snapuserd/user-space-merge/handler_manager.cpp | 4 ++-- .../snapuserd/user-space-merge/handler_manager.h | 14 ++++++-------- .../snapuserd/user-space-merge/merge_worker.cpp | 10 ++++++++-- .../snapuserd/user-space-merge/merge_worker.h | 4 +++- .../snapuserd/user-space-merge/snapuserd_core.cpp | 9 +++++---- .../snapuserd/user-space-merge/snapuserd_core.h | 5 +++-- .../user-space-merge/snapuserd_readahead.cpp | 10 ++++++++-- .../user-space-merge/snapuserd_readahead.h | 4 +++- .../user-space-merge/snapuserd_server.cpp | 6 ++++-- .../snapuserd/user-space-merge/snapuserd_server.h | 3 ++- .../snapuserd/user-space-merge/snapuserd_test.cpp | 14 ++++++++------ 16 files changed, 75 insertions(+), 35 deletions(-) diff --git a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto index 2e948dda7..62f99013e 100644 --- a/fs_mgr/libsnapshot/android/snapshot/snapshot.proto +++ b/fs_mgr/libsnapshot/android/snapshot/snapshot.proto @@ -229,6 +229,10 @@ message SnapshotUpdateStatus { // Enable direct reads from source device bool o_direct = 12; + + // Number of cow operations to be merged at once + uint32 cow_op_merge_size = 13; + } // Next: 10 diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index 1ec863444..4a3ec1d3a 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -831,6 +831,8 @@ class SnapshotManager final : public ISnapshotManager { // Check if direct reads are enabled for the source image bool UpdateUsesODirect(LockedFile* lock); + // Get value of maximum cow op merge size + uint32_t GetUpdateCowOpMergeSize(LockedFile* lock); // Wrapper around libdm, with diagnostics. bool DeleteDeviceIfExists(const std::string& name, const std::chrono::milliseconds& timeout_ms = {}); diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 11aa3f934..265445b60 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1709,6 +1709,10 @@ bool SnapshotManager::PerformInitTransition(InitTransition transition, if (UpdateUsesODirect(lock.get())) { snapuserd_argv->emplace_back("-o_direct"); } + uint cow_op_merge_size = GetUpdateCowOpMergeSize(lock.get()); + if (cow_op_merge_size != 0) { + snapuserd_argv->emplace_back("-cow_op_merge_size=" + std::to_string(cow_op_merge_size)); + } } size_t num_cows = 0; @@ -2131,6 +2135,11 @@ bool SnapshotManager::UpdateUsesODirect(LockedFile* lock) { return update_status.o_direct(); } +uint32_t SnapshotManager::GetUpdateCowOpMergeSize(LockedFile* lock) { + SnapshotUpdateStatus update_status = ReadSnapshotUpdateStatus(lock); + return update_status.cow_op_merge_size(); +} + bool SnapshotManager::MarkSnapuserdFromSystem() { auto path = GetSnapuserdFromSystemPath(); @@ -3092,6 +3101,7 @@ bool SnapshotManager::WriteUpdateState(LockedFile* lock, UpdateState state, status.set_io_uring_enabled(old_status.io_uring_enabled()); status.set_legacy_snapuserd(old_status.legacy_snapuserd()); status.set_o_direct(old_status.o_direct()); + status.set_cow_op_merge_size(old_status.cow_op_merge_size()); } return WriteSnapshotUpdateStatus(lock, status); } @@ -3474,6 +3484,8 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife status.set_legacy_snapuserd(true); LOG(INFO) << "Setting legacy_snapuserd to true"; } + status.set_cow_op_merge_size( + android::base::GetUintProperty("ro.virtual_ab.cow_op_merge_size", 0)); } else if (legacy_compression) { LOG(INFO) << "Virtual A/B using legacy snapuserd"; } else { @@ -3909,6 +3921,7 @@ bool SnapshotManager::Dump(std::ostream& os) { ss << "Using userspace snapshots: " << update_status.userspace_snapshots() << std::endl; ss << "Using io_uring: " << update_status.io_uring_enabled() << std::endl; ss << "Using o_direct: " << update_status.o_direct() << std::endl; + ss << "Cow op merge size (0 for uncapped): " << update_status.cow_op_merge_size() << std::endl; ss << "Using XOR compression: " << GetXorCompressionEnabledProperty() << std::endl; ss << "Current slot: " << device_->GetSlotSuffix() << std::endl; ss << "Boot indicator: booting from " << GetCurrentSlot() << " slot" << std::endl; diff --git a/fs_mgr/libsnapshot/snapuserd/snapuserd_daemon.cpp b/fs_mgr/libsnapshot/snapuserd/snapuserd_daemon.cpp index 67e9e52ea..dd2dd5659 100644 --- a/fs_mgr/libsnapshot/snapuserd/snapuserd_daemon.cpp +++ b/fs_mgr/libsnapshot/snapuserd/snapuserd_daemon.cpp @@ -30,6 +30,7 @@ DEFINE_bool(socket_handoff, false, DEFINE_bool(user_snapshot, false, "If true, user-space snapshots are used"); DEFINE_bool(io_uring, false, "If true, io_uring feature is enabled"); DEFINE_bool(o_direct, false, "If true, enable direct reads on source device"); +DEFINE_int32(cow_op_merge_size, 0, "number of operations to be processed at once"); namespace android { namespace snapshot { @@ -106,7 +107,6 @@ bool Daemon::StartServerForUserspaceSnapshots(int arg_start, int argc, char** ar } return user_server_.Run(); } - for (int i = arg_start; i < argc; i++) { auto parts = android::base::Split(argv[i], ","); @@ -114,8 +114,8 @@ bool Daemon::StartServerForUserspaceSnapshots(int arg_start, int argc, char** ar LOG(ERROR) << "Malformed message, expected at least four sub-arguments."; return false; } - auto handler = - user_server_.AddHandler(parts[0], parts[1], parts[2], parts[3], FLAGS_o_direct); + auto handler = user_server_.AddHandler(parts[0], parts[1], parts[2], parts[3], + FLAGS_o_direct, FLAGS_cow_op_merge_size); if (!handler || !user_server_.StartHandler(parts[0])) { return false; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp index c85331b34..ef4ba93fe 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/extractor.cpp @@ -41,7 +41,7 @@ Extractor::Extractor(const std::string& base_path, const std::string& cow_path) bool Extractor::Init() { auto opener = factory_.CreateTestOpener(control_name_); handler_ = std::make_shared(control_name_, cow_path_, base_path_, base_path_, - opener, 1, false, false, false); + opener, 1, false, false, false, 0); if (!handler_->InitCowDevice()) { return false; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp index ea11f0e6e..fdd9cce0c 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.cpp @@ -53,10 +53,10 @@ std::shared_ptr SnapshotHandlerManager::AddHandler( const std::string& misc_name, const std::string& cow_device_path, const std::string& backing_device, const std::string& base_path_merge, std::shared_ptr opener, int num_worker_threads, bool use_iouring, - bool o_direct) { + bool o_direct, uint32_t cow_op_merge_size) { auto snapuserd = std::make_shared( misc_name, cow_device_path, backing_device, base_path_merge, opener, num_worker_threads, - use_iouring, perform_verification_, o_direct); + use_iouring, perform_verification_, o_direct, cow_op_merge_size); if (!snapuserd->InitCowDevice()) { LOG(ERROR) << "Failed to initialize Snapuserd"; return nullptr; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h index f23f07eb5..ecf5d5c38 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/handler_manager.h @@ -52,13 +52,11 @@ class ISnapshotHandlerManager { virtual ~ISnapshotHandlerManager() {} // Add a new snapshot handler but do not start serving requests yet. - virtual std::shared_ptr AddHandler(const std::string& misc_name, - const std::string& cow_device_path, - const std::string& backing_device, - const std::string& base_path_merge, - std::shared_ptr opener, - int num_worker_threads, bool use_iouring, - bool o_direct) = 0; + virtual std::shared_ptr AddHandler( + const std::string& misc_name, const std::string& cow_device_path, + const std::string& backing_device, const std::string& base_path_merge, + std::shared_ptr opener, int num_worker_threads, bool use_iouring, + bool o_direct, uint32_t cow_op_merge_size) = 0; // Start serving requests on a snapshot handler. virtual bool StartHandler(const std::string& misc_name) = 0; @@ -98,7 +96,7 @@ class SnapshotHandlerManager final : public ISnapshotHandlerManager { const std::string& base_path_merge, std::shared_ptr opener, int num_worker_threads, bool use_iouring, - bool o_direct) override; + bool o_direct, uint32_t cow_op_merge_size) override; bool StartHandler(const std::string& misc_name) override; bool DeleteHandler(const std::string& misc_name) override; bool InitiateMerge(const std::string& misc_name) override; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp index e6a9a2989..e2c58741a 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.cpp @@ -32,12 +32,18 @@ using android::base::unique_fd; MergeWorker::MergeWorker(const std::string& cow_device, const std::string& misc_name, const std::string& base_path_merge, - std::shared_ptr snapuserd) - : Worker(cow_device, misc_name, base_path_merge, snapuserd) {} + std::shared_ptr snapuserd, uint32_t cow_op_merge_size) + : Worker(cow_device, misc_name, base_path_merge, snapuserd), + cow_op_merge_size_(cow_op_merge_size) {} int MergeWorker::PrepareMerge(uint64_t* source_offset, int* pending_ops, std::vector* replace_zero_vec) { int num_ops = *pending_ops; + // 0 indicates ro.virtual_ab.cow_op_merge_size was not set in the build + if (cow_op_merge_size_ != 0) { + num_ops = std::min(cow_op_merge_size_, static_cast(*pending_ops)); + } + int nr_consecutive = 0; bool checkOrderedOp = (replace_zero_vec == nullptr); size_t num_blocks = 1; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.h index 478d4c8a2..a19352dad 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/merge_worker.h @@ -23,7 +23,8 @@ namespace snapshot { class MergeWorker : public Worker { public: MergeWorker(const std::string& cow_device, const std::string& misc_name, - const std::string& base_path_merge, std::shared_ptr snapuserd); + const std::string& base_path_merge, std::shared_ptr snapuserd, + uint32_t cow_op_merge_size); bool Run(); private: @@ -53,6 +54,7 @@ class MergeWorker : public Worker { // syscalls and fallback to synchronous I/O, we // don't want huge queue depth int queue_depth_ = 8; + uint32_t cow_op_merge_size_ = 0; }; } // namespace snapshot diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp index 05ba047b5..7c9a64ee4 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.cpp @@ -36,7 +36,8 @@ using android::base::unique_fd; SnapshotHandler::SnapshotHandler(std::string misc_name, std::string cow_device, std::string backing_device, std::string base_path_merge, std::shared_ptr opener, int num_worker_threads, - bool use_iouring, bool perform_verification, bool o_direct) { + bool use_iouring, bool perform_verification, bool o_direct, + uint32_t cow_op_merge_size) { misc_name_ = std::move(misc_name); cow_device_ = std::move(cow_device); backing_store_device_ = std::move(backing_device); @@ -46,6 +47,7 @@ SnapshotHandler::SnapshotHandler(std::string misc_name, std::string cow_device, is_io_uring_enabled_ = use_iouring; perform_verification_ = perform_verification; o_direct_ = o_direct; + cow_op_merge_size_ = cow_op_merge_size; } bool SnapshotHandler::InitializeWorkers() { @@ -60,12 +62,11 @@ bool SnapshotHandler::InitializeWorkers() { worker_threads_.push_back(std::move(wt)); } - merge_thread_ = std::make_unique(cow_device_, misc_name_, base_path_merge_, - GetSharedPtr()); + GetSharedPtr(), cow_op_merge_size_); read_ahead_thread_ = std::make_unique(cow_device_, backing_store_device_, misc_name_, - GetSharedPtr()); + GetSharedPtr(), cow_op_merge_size_); update_verify_ = std::make_unique(misc_name_); diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h index 9b7238a5d..c7de9951f 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_core.h @@ -104,7 +104,8 @@ class SnapshotHandler : public std::enable_shared_from_this { public: SnapshotHandler(std::string misc_name, std::string cow_device, std::string backing_device, std::string base_path_merge, std::shared_ptr opener, - int num_workers, bool use_iouring, bool perform_verification, bool o_direct); + int num_workers, bool use_iouring, bool perform_verification, bool o_direct, + uint32_t cow_op_merge_size); bool InitCowDevice(); bool Start(); @@ -247,7 +248,7 @@ class SnapshotHandler : public std::enable_shared_from_this { bool resume_merge_ = false; bool merge_complete_ = false; bool o_direct_ = false; - + uint32_t cow_op_merge_size_ = 0; std::unique_ptr update_verify_; std::shared_ptr block_server_opener_; }; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp index 2baf20ddd..6b1ed0cd7 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.cpp @@ -18,6 +18,7 @@ #include +#include "android-base/properties.h" #include "snapuserd_core.h" #include "utility.h" @@ -29,11 +30,13 @@ using namespace android::dm; using android::base::unique_fd; ReadAhead::ReadAhead(const std::string& cow_device, const std::string& backing_device, - const std::string& misc_name, std::shared_ptr snapuserd) { + const std::string& misc_name, std::shared_ptr snapuserd, + uint32_t cow_op_merge_size) { cow_device_ = cow_device; backing_store_device_ = backing_device; misc_name_ = misc_name; snapuserd_ = snapuserd; + cow_op_merge_size_ = cow_op_merge_size; } void ReadAhead::CheckOverlap(const CowOperation* cow_op) { @@ -62,8 +65,11 @@ int ReadAhead::PrepareNextReadAhead(uint64_t* source_offset, int* pending_ops, std::vector& blocks, std::vector& xor_op_vec) { int num_ops = *pending_ops; - int nr_consecutive = 0; + if (cow_op_merge_size_ != 0) { + num_ops = std::min(static_cast(cow_op_merge_size_), *pending_ops); + } + int nr_consecutive = 0; bool is_ops_present = (!RAIterDone() && num_ops); if (!is_ops_present) { diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.h index d3ba1265b..4885c9608 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_readahead.h @@ -35,7 +35,8 @@ class SnapshotHandler; class ReadAhead { public: ReadAhead(const std::string& cow_device, const std::string& backing_device, - const std::string& misc_name, std::shared_ptr snapuserd); + const std::string& misc_name, std::shared_ptr snapuserd, + uint32_t cow_op_merge_size); bool RunThread(); private: @@ -106,6 +107,7 @@ class ReadAhead { // syscalls and fallback to synchronous I/O, we // don't want huge queue depth int queue_depth_ = 8; + uint32_t cow_op_merge_size_; std::unique_ptr ring_; }; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp index 0b881b683..c0af5c5b7 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.cpp @@ -347,7 +347,8 @@ std::shared_ptr UserSnapshotServer::AddHandler(const std::string& const std::string& cow_device_path, const std::string& backing_device, const std::string& base_path_merge, - const bool o_direct) { + const bool o_direct, + uint32_t cow_op_merge_size) { // We will need multiple worker threads only during // device boot after OTA. For all other purposes, // one thread is sufficient. We don't want to consume @@ -369,7 +370,8 @@ std::shared_ptr UserSnapshotServer::AddHandler(const std::string& auto opener = block_server_factory_->CreateOpener(misc_name); return handlers_->AddHandler(misc_name, cow_device_path, backing_device, base_path_merge, - opener, num_worker_threads, io_uring_enabled_, o_direct); + opener, num_worker_threads, io_uring_enabled_, o_direct, + cow_op_merge_size); } bool UserSnapshotServer::WaitForSocket() { diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h index d9cf97f9d..ceea36ae4 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_server.h @@ -87,7 +87,8 @@ class UserSnapshotServer { const std::string& cow_device_path, const std::string& backing_device, const std::string& base_path_merge, - bool o_direct = false); + bool o_direct = false, + uint32_t cow_op_merge_size = 0); bool StartHandler(const std::string& misc_name); void SetTerminating() { terminating_ = true; } diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp index 6d0ae3d09..9042f2bb8 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp @@ -67,6 +67,7 @@ struct TestParam { std::string compression; int block_size; int num_threads; + uint32_t cow_op_merge_size; }; class SnapuserdTestBase : public ::testing::TestWithParam { @@ -708,9 +709,9 @@ void SnapuserdTest::InitCowDevice() { auto opener = factory->CreateOpener(system_device_ctrl_name_); handlers_->DisableVerification(); const TestParam params = GetParam(); - auto handler = handlers_->AddHandler(system_device_ctrl_name_, cow_system_->path, - base_dev_->GetPath(), base_dev_->GetPath(), opener, 1, - params.io_uring, params.o_direct); + auto handler = handlers_->AddHandler( + system_device_ctrl_name_, cow_system_->path, base_dev_->GetPath(), base_dev_->GetPath(), + opener, 1, params.io_uring, params.o_direct, params.cow_op_merge_size); ASSERT_NE(handler, nullptr); ASSERT_NE(handler->snapuserd(), nullptr); #ifdef __ANDROID__ @@ -1227,9 +1228,9 @@ void HandlerTest::InitializeDevice() { ASSERT_NE(opener_, nullptr); const TestParam params = GetParam(); - handler_ = std::make_shared(system_device_ctrl_name_, cow_system_->path, - base_dev_->GetPath(), base_dev_->GetPath(), - opener_, 1, false, false, params.o_direct); + handler_ = std::make_shared( + system_device_ctrl_name_, cow_system_->path, base_dev_->GetPath(), base_dev_->GetPath(), + opener_, 1, false, false, params.o_direct, params.cow_op_merge_size); ASSERT_TRUE(handler_->InitCowDevice()); ASSERT_TRUE(handler_->InitializeWorkers()); @@ -1507,6 +1508,7 @@ std::vector GetVariableBlockTestConfigs() { param.num_threads = thread; param.io_uring = io_uring; param.o_direct = false; + param.cow_op_merge_size = 0; testParams.push_back(std::move(param)); } } From 39fe6e1d384b0730161d6b545199dfb10110de58 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Wed, 24 Jul 2024 14:50:01 -0700 Subject: [PATCH 055/158] libsnapshot: add test for merge_size param Add test to make sure that setting the protobuf successfully propogates the gflag argument to snapuserd_handler. Bug: 332255580 Test: th Change-Id: Ic7a008bee7dd3ca29faa0a5409f0513f1551bf37 --- fs_mgr/libsnapshot/snapshot_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 07f13014d..b2e36d426 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -2665,6 +2665,7 @@ TEST_F(SnapshotTest, FlagCheck) { status.set_o_direct(true); status.set_io_uring_enabled(true); status.set_userspace_snapshots(true); + status.set_cow_op_merge_size(16); sm->WriteSnapshotUpdateStatus(lock_.get(), status); // Ensure a connection to the second-stage daemon, but use the first-stage @@ -2686,6 +2687,8 @@ TEST_F(SnapshotTest, FlagCheck) { snapuserd_argv.end()); ASSERT_TRUE(std::find(snapuserd_argv.begin(), snapuserd_argv.end(), "-user_snapshot") != snapuserd_argv.end()); + ASSERT_TRUE(std::find(snapuserd_argv.begin(), snapuserd_argv.end(), "-cow_op_merge_size=16") != + snapuserd_argv.end()); } class FlashAfterUpdateTest : public SnapshotUpdateTest, From 77d388f2858357f4d5229878c3e8f9ce0a359984 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Thu, 11 Jul 2024 10:06:47 -0700 Subject: [PATCH 056/158] Skip tests if vendor partition is on Android S Bug: 349278999 Test: vts_libsnapshot_test on GSI config Signed-off-by: Akilesh Kailash (cherry picked from https://android-review.googlesource.com/q/commit:6906249312be46b64b6b6221ba33bad196bf6cd6) Merged-In: I6826b287565e8a78bea21b4830ad30f1c30a01bf Change-Id: I6826b287565e8a78bea21b4830ad30f1c30a01bf --- .../include_test/libsnapshot/test_helpers.h | 18 ++++++++++++++++++ fs_mgr/libsnapshot/snapshot_test.cpp | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h index 90813fe79..0afd8bd22 100644 --- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h @@ -30,6 +30,8 @@ #include #include +#include "utility.h" + namespace android { namespace snapshot { @@ -234,5 +236,21 @@ bool IsVirtualAbEnabled(); #define RETURN_IF_NON_VIRTUAL_AB() RETURN_IF_NON_VIRTUAL_AB_MSG("") +#define SKIP_IF_VENDOR_ON_ANDROID_S() \ + do { \ + if (IsVendorFromAndroid12()) \ + GTEST_SKIP() << "Skip test as Vendor partition is on Android S"; \ + } while (0) + +#define RETURN_IF_VENDOR_ON_ANDROID_S_MSG(msg) \ + do { \ + if (IsVendorFromAndroid12()) { \ + std::cerr << (msg); \ + return; \ + } \ + } while (0) + +#define RETURN_IF_VENDOR_ON_ANDROID_S() RETURN_IF_VENDOR_ON_ANDROID_S_MSG("") + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 1435b12bf..07f13014d 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -122,6 +122,7 @@ class SnapshotTest : public ::testing::Test { LOG(INFO) << "Starting test: " << test_name_; SKIP_IF_NON_VIRTUAL_AB(); + SKIP_IF_VENDOR_ON_ANDROID_S(); SetupProperties(); if (!DeviceSupportsMode()) { @@ -168,6 +169,7 @@ class SnapshotTest : public ::testing::Test { void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); LOG(INFO) << "Tearing down SnapshotTest test: " << test_name_; @@ -1015,6 +1017,7 @@ class SnapshotUpdateTest : public SnapshotTest { public: void SetUp() override { SKIP_IF_NON_VIRTUAL_AB(); + SKIP_IF_VENDOR_ON_ANDROID_S(); SnapshotTest::SetUp(); if (!image_manager_) { @@ -1097,6 +1100,7 @@ class SnapshotUpdateTest : public SnapshotTest { } void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); LOG(INFO) << "Tearing down SnapshotUpdateTest test: " << test_name_; @@ -2833,6 +2837,7 @@ void SnapshotTestEnvironment::SetUp() { // that is fixed, don't call GTEST_SKIP here, but instead call GTEST_SKIP in individual test // suites. RETURN_IF_NON_VIRTUAL_AB_MSG("Virtual A/B is not enabled, skipping global setup.\n"); + RETURN_IF_VENDOR_ON_ANDROID_S_MSG("Test not enabled for Vendor on Android S.\n"); std::vector paths = { // clang-format off @@ -2887,6 +2892,8 @@ void SnapshotTestEnvironment::SetUp() { void SnapshotTestEnvironment::TearDown() { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); + if (super_images_ != nullptr) { DeleteBackingImage(super_images_.get(), "fake-super"); } From adaf620dde1cffba59bacf8cb475cabdfcb1eff6 Mon Sep 17 00:00:00 2001 From: Antonio Kantek Date: Thu, 25 Jul 2024 16:07:41 -0700 Subject: [PATCH 057/158] Enable secondary_user_on_secondary_display for CtsFsMgrTestCases secondary_user_on_secondary_display is for background users that have access to UI on assigned displays (a.k.a. visible background users) on devices that have config_multiuserVisibleBackgroundUsers enabled. The main use case is Automotive's multi-display Whole Cabin experience where passengers (modeled as visible background users) can interact with the display in front of them concurrently with the driver (modeled as the the current user) interacting with driver's display. Bug: 349714805 Flag: EXEMPT test fix Test: atest CtsFsMgrTestCases Test: atest --user-type secondary_user_on_secondary_display CtsFsMgrTestCases Change-Id: I0e4498fd431f5c70135d8704f1d2fa5476620ef1 --- fs_mgr/tests/AndroidTest.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/fs_mgr/tests/AndroidTest.xml b/fs_mgr/tests/AndroidTest.xml index de835b3ad..1c06ebd33 100644 --- a/fs_mgr/tests/AndroidTest.xml +++ b/fs_mgr/tests/AndroidTest.xml @@ -16,6 +16,7 @@