From d192d5471c19c0f9eca7c6e1adc5cdb20d5aeee4 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Thu, 7 May 2020 15:52:48 +0100 Subject: [PATCH 01/43] Create /metadata/staged-install dir to store staged install failure reasons Bug: 146343545 Test: presubmit Change-Id: I513d403f67643929bb8f90212c1054fb3024f12a --- rootdir/init.rc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rootdir/init.rc b/rootdir/init.rc index 38ba137e7..4b64715ac 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -514,6 +514,8 @@ on post-fs mkdir /metadata/apex 0700 root system mkdir /metadata/apex/sessions 0700 root system + + mkdir /metadata/staged-install 0770 root system on late-fs # Ensure that tracefs has the correct permissions. # This does not work correctly if it is called in post-fs. From 4048e49956a2dfd49af3adf0f78881bf15f3550f Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 28 May 2020 00:29:08 +0000 Subject: [PATCH 02/43] String16::remove - avoid overflow Bug: 156999009 Test: libutils_test (cases added) Change-Id: Iad46d95d9848928ba81000090b2fe9aec1e5eaac Merged-In: Iad46d95d9848928ba81000090b2fe9aec1e5eaac (cherry picked from commit f251c1c581f2d1b9940e60e756315c5e15443990) (cherry picked from commit 9a9c8910e9296c9dc8d79c37f589895f5a2a836c) --- libutils/String16.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libutils/String16.cpp b/libutils/String16.cpp index e8f1c5184..7055fc6c3 100644 --- a/libutils/String16.cpp +++ b/libutils/String16.cpp @@ -402,7 +402,7 @@ status_t String16::remove(size_t len, size_t begin) mString = getEmptyString(); return NO_ERROR; } - if ((begin+len) > N) len = N-begin; + if (len > N || len > N - begin) len = N - begin; if (begin == 0 && len == N) { return NO_ERROR; } From 8bafa9e28e6bbe9ea1c82cdcf22eb4292ca0ad9d Mon Sep 17 00:00:00 2001 From: Steve Muckle Date: Fri, 29 May 2020 16:30:33 -0700 Subject: [PATCH 03/43] add libmodprobe api to query the number of modules loaded In the short term this will be used to help implement support for multiple kernel module directories but it may be useful in other contexts as well. Bug: 157645635 Change-Id: I15a252b6e9394292c8f3557ed65112c935830441 Merged-In: I15a252b6e9394292c8f3557ed65112c935830441 --- libmodprobe/include/modprobe/modprobe.h | 3 +++ libmodprobe/libmodprobe_ext.cpp | 1 + libmodprobe/libmodprobe_ext_test.cpp | 1 + libmodprobe/libmodprobe_test.cpp | 1 + 4 files changed, 6 insertions(+) diff --git a/libmodprobe/include/modprobe/modprobe.h b/libmodprobe/include/modprobe/modprobe.h index 297036e41..a7687af16 100644 --- a/libmodprobe/include/modprobe/modprobe.h +++ b/libmodprobe/include/modprobe/modprobe.h @@ -34,6 +34,8 @@ class Modprobe { bool GetAllDependencies(const std::string& module, std::vector* pre_dependencies, std::vector* dependencies, std::vector* post_dependencies); + void ResetModuleCount() { module_count_ = 0; } + int GetModuleCount() { return module_count_; } void EnableBlacklist(bool enable); void EnableVerbose(bool enable); @@ -65,5 +67,6 @@ class Modprobe { std::unordered_map module_options_; std::set module_blacklist_; std::unordered_set module_loaded_; + int module_count_ = 0; bool blacklist_enabled = false; }; diff --git a/libmodprobe/libmodprobe_ext.cpp b/libmodprobe/libmodprobe_ext.cpp index 99472c14d..658970874 100644 --- a/libmodprobe/libmodprobe_ext.cpp +++ b/libmodprobe/libmodprobe_ext.cpp @@ -63,6 +63,7 @@ bool Modprobe::Insmod(const std::string& path_name, const std::string& parameter LOG(INFO) << "Loaded kernel module " << path_name; module_loaded_.emplace(canonical_name); + module_count_++; return true; } diff --git a/libmodprobe/libmodprobe_ext_test.cpp b/libmodprobe/libmodprobe_ext_test.cpp index 057dea3a0..9ee5ba7ab 100644 --- a/libmodprobe/libmodprobe_ext_test.cpp +++ b/libmodprobe/libmodprobe_ext_test.cpp @@ -56,6 +56,7 @@ bool Modprobe::Insmod(const std::string& path_name, const std::string& parameter } modules_loaded.emplace_back(path_name + options); + module_count_++; return true; } diff --git a/libmodprobe/libmodprobe_test.cpp b/libmodprobe/libmodprobe_test.cpp index 879c7f2e9..eea0abd52 100644 --- a/libmodprobe/libmodprobe_test.cpp +++ b/libmodprobe/libmodprobe_test.cpp @@ -161,6 +161,7 @@ TEST(libmodprobe, Test) { EXPECT_TRUE(modules_loaded == expected_modules_loaded); + EXPECT_TRUE(m.GetModuleCount() == 15); EXPECT_TRUE(m.Remove("test4")); GTEST_LOG_(INFO) << "Expected modules loaded after removing test4 (in order):"; From e35d217958bb070a10dcdda8a646f736cdd00f74 Mon Sep 17 00:00:00 2001 From: Steve Muckle Date: Fri, 29 May 2020 16:31:19 -0700 Subject: [PATCH 04/43] first_stage_init: support kernel module directories Kernel modules may be located within directories in /lib/modules. Attempt to load kernel modules from each directory that has a name starting with the major and minor version of the currently running kernel. If a single kernel module is successfully loaded from a directory, that directory is treated as the correct kernel module directory for the system. No other kernel module directories are searched and any kernel module load errors in that directory are fatal. If the attempt to load the first kernel module from a directory fails, or if there are no kernel modules in a directory, then the search proceeds to the next directory. If no kernel module is successfully loaded from any directory as above, an attempt is made to load kernel modules from the top level at /lib/modules/. Bug: 157645635 Change-Id: I92eadd8617f03a645da460ccb776bc04fa541f00 Merged-In: I92eadd8617f03a645da460ccb776bc04fa541f00 --- init/first_stage_init.cpp | 85 ++++++++++++++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 11 deletions(-) diff --git a/init/first_stage_init.cpp b/init/first_stage_init.cpp index 1a608f65b..021557697 100644 --- a/init/first_stage_init.cpp +++ b/init/first_stage_init.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -99,6 +100,77 @@ bool ForceNormalBoot(const std::string& cmdline) { } // namespace +std::string GetModuleLoadList(bool recovery, const std::string& dir_path) { + auto module_load_file = "modules.load"; + if (recovery) { + struct stat fileStat; + std::string recovery_load_path = dir_path + "/modules.load.recovery"; + if (!stat(recovery_load_path.c_str(), &fileStat)) { + module_load_file = "modules.load.recovery"; + } + } + + return module_load_file; +} + +#define MODULE_BASE_DIR "/lib/modules" +bool LoadKernelModules(bool recovery, bool want_console) { + struct utsname uts; + if (uname(&uts)) { + LOG(FATAL) << "Failed to get kernel version."; + } + int major, minor; + if (sscanf(uts.release, "%d.%d", &major, &minor) != 2) { + LOG(FATAL) << "Failed to parse kernel version " << uts.release; + } + + std::unique_ptr base_dir(opendir(MODULE_BASE_DIR), closedir); + if (!base_dir) { + LOG(INFO) << "Unable to open /lib/modules, skipping module loading."; + return true; + } + dirent* entry; + std::vector module_dirs; + while ((entry = readdir(base_dir.get()))) { + if (entry->d_type != DT_DIR) { + continue; + } + int dir_major, dir_minor; + if (sscanf(entry->d_name, "%d.%d", &dir_major, &dir_minor) != 2 || dir_major != major || + dir_minor != minor) { + continue; + } + module_dirs.emplace_back(entry->d_name); + } + + // Sort the directories so they are iterated over during module loading + // in a consistent order. Alphabetical sorting is fine here because the + // kernel version at the beginning of the directory name must match the + // current kernel version, so the sort only applies to a label that + // follows the kernel version, for example /lib/modules/5.4 vs. + // /lib/modules/5.4-gki. + std::sort(module_dirs.begin(), module_dirs.end()); + + for (const auto& module_dir : module_dirs) { + std::string dir_path = MODULE_BASE_DIR "/"; + dir_path.append(module_dir); + Modprobe m({dir_path}, GetModuleLoadList(recovery, dir_path)); + bool retval = m.LoadListedModules(!want_console); + int modules_loaded = m.GetModuleCount(); + if (modules_loaded > 0) { + return retval; + } + } + + Modprobe m({MODULE_BASE_DIR}, GetModuleLoadList(recovery, MODULE_BASE_DIR)); + bool retval = m.LoadListedModules(!want_console); + int modules_loaded = m.GetModuleCount(); + if (modules_loaded > 0) { + return retval; + } + return true; +} + int FirstStageMain(int argc, char** argv) { if (REBOOT_BOOTLOADER_ON_PANIC) { InstallRebootSignalHandlers(); @@ -190,18 +262,9 @@ int FirstStageMain(int argc, char** argv) { old_root_dir.reset(); } - std::string module_load_file = "modules.load"; - if (IsRecoveryMode() && !ForceNormalBoot(cmdline)) { - struct stat fileStat; - std::string recovery_load_path = "/lib/modules/modules.load.recovery"; - if (!stat(recovery_load_path.c_str(), &fileStat)) { - module_load_file = "modules.load.recovery"; - } - } - - Modprobe m({"/lib/modules"}, module_load_file); auto want_console = ALLOW_FIRST_STAGE_CONSOLE ? FirstStageConsole(cmdline) : 0; - if (!m.LoadListedModules(!want_console)) { + + if (!LoadKernelModules(IsRecoveryMode() && !ForceNormalBoot(cmdline), want_console)) { if (want_console != FirstStageConsoleParam::DISABLED) { LOG(ERROR) << "Failed to load kernel modules, starting console"; } else { From a311f7866fcfe7a60db184d61916a1f54ff5c9b0 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 2 Jun 2020 15:23:39 -0700 Subject: [PATCH 05/43] check_ms_os_desc: fix buffer overflow. Bug: http://b/155505587 Test: check_ms_os_desc Change-Id: Ie99715ab62571d24460c3a1fb859d22951f30cb8 Merged-In: Ie99715ab62571d24460c3a1fb859d22951f30cb8 --- adb/tools/check_ms_os_desc.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/adb/tools/check_ms_os_desc.cpp b/adb/tools/check_ms_os_desc.cpp index 8743ff725..8e858094a 100644 --- a/adb/tools/check_ms_os_desc.cpp +++ b/adb/tools/check_ms_os_desc.cpp @@ -131,8 +131,6 @@ static void check_ms_os_desc_v1(libusb_device_handle* device_handle, const std:: errx(1, "failed to retrieve MS OS v1 compat descriptor: %s", libusb_error_name(rc)); } - memcpy(&hdr, data.data(), data.size()); - struct __attribute__((packed)) ms_os_desc_v1_function { uint8_t bFirstInterfaceNumber; uint8_t reserved1; From 39bdf397fa48f62b18f39ff984a2cfa0a4524495 Mon Sep 17 00:00:00 2001 From: Woody Lin Date: Wed, 13 May 2020 20:50:54 +0800 Subject: [PATCH 06/43] libsparse: Limit block size to 64 MB Limit the block size to 64 MB before fastboot executable binary for windows 64-bit is released. Bug: 156057250 Change-Id: Ic4edb963a3d99f718d7630aba3f351729a84e994 --- libsparse/sparse.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libsparse/sparse.cpp b/libsparse/sparse.cpp index 24c6379cd..8622b4c39 100644 --- a/libsparse/sparse.cpp +++ b/libsparse/sparse.cpp @@ -136,11 +136,23 @@ static int write_all_blocks(struct sparse_file* s, struct output_file* out) { return 0; } +/* + * This is a workaround for 32-bit Windows: Limit the block size to 64 MB before + * fastboot executable binary for windows 64-bit is released (b/156057250). + */ +#define MAX_BACKED_BLOCK_SIZE ((unsigned int) (64UL << 20)) + int sparse_file_write(struct sparse_file* s, int fd, bool gz, bool sparse, bool crc) { + struct backed_block* bb; int ret; int chunks; struct output_file* out; + for (bb = backed_block_iter_new(s->backed_block_list); bb; bb = backed_block_iter_next(bb)) { + ret = backed_block_split(s->backed_block_list, bb, MAX_BACKED_BLOCK_SIZE); + if (ret) return ret; + } + chunks = sparse_count_chunks(s); out = output_file_open_fd(fd, s->block_size, s->len, gz, sparse, chunks, crc); From 46f25dee36b72d19447028dec1eb9dda273776e1 Mon Sep 17 00:00:00 2001 From: Alistair Delva Date: Mon, 8 Jun 2020 10:51:33 -0700 Subject: [PATCH 07/43] Add documentation for umount_all The mount_all and swapon_all commands are documented, but umount_all is not. Add some documentation. Bug: 142424832 Change-Id: I7e4dcb4d222b787350a79c9e312062cac9eeb4d8 --- init/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/init/README.md b/init/README.md index 0dd14900f..4be521902 100644 --- a/init/README.md +++ b/init/README.md @@ -639,6 +639,12 @@ provides the `aidl_lazy_test_1` interface. `umount ` > Unmount the filesystem mounted at that path. +`umount_all [ ]` +> Calls fs\_mgr\_umount\_all on the given fstab file. + If the fstab parameter is not specified, fstab.${ro.boot.fstab_suffix}, + fstab.${ro.hardware} or fstab.${ro.hardware.platform} will be scanned for + under /odm/etc, /vendor/etc, or / at runtime, in that order. + `verity_update_state ` > Internal implementation detail used to update dm-verity state and set the partition._mount-point_.verified properties used by adb remount From afaa5fbccc0506be150e00725988a0d65516d68e Mon Sep 17 00:00:00 2001 From: Alistair Delva Date: Mon, 8 Jun 2020 11:04:53 -0700 Subject: [PATCH 08/43] Respect ro.boot.fstab_suffix in swapon_all While mount_all and umount_all were updated to use ro.boot.fstab_suffix, I neglected to update swapon_all. Trivially copied from umount_all. Bug: 142424832 Change-Id: Icd706fe7a1fe16c687cd2811b0a3158d7d2e224e --- init/README.md | 5 ++++- init/builtins.cpp | 16 +++++++++++++--- init/check_builtins.cpp | 8 ++++++++ init/check_builtins.h | 1 + init/util.cpp | 15 +++++++++++++-- init/util.h | 2 ++ 6 files changed, 41 insertions(+), 6 deletions(-) diff --git a/init/README.md b/init/README.md index 4be521902..6f2c01f05 100644 --- a/init/README.md +++ b/init/README.md @@ -623,8 +623,11 @@ provides the `aidl_lazy_test_1` interface. `stop ` > Stop a service from running if it is currently running. -`swapon_all ` +`swapon_all [ ]` > Calls fs\_mgr\_swapon\_all on the given fstab file. + If the fstab parameter is not specified, fstab.${ro.boot.fstab_suffix}, + fstab.${ro.hardware} or fstab.${ro.hardware.platform} will be scanned for + under /odm/etc, /vendor/etc, or / at runtime, in that order. `symlink ` > Create a symbolic link at _path_ with the value _target_ diff --git a/init/builtins.cpp b/init/builtins.cpp index e918e12ae..0ac66f272 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -708,10 +708,20 @@ static Result do_umount_all(const BuiltinArguments& args) { return {}; } +/* swapon_all [ ] */ static Result do_swapon_all(const BuiltinArguments& args) { + auto swapon_all = ParseSwaponAll(args.args); + if (!swapon_all.ok()) return swapon_all.error(); + Fstab fstab; - if (!ReadFstabFromFile(args[1], &fstab)) { - return Error() << "Could not read fstab '" << args[1] << "'"; + if (swapon_all->empty()) { + if (!ReadDefaultFstab(&fstab)) { + return Error() << "Could not read default fstab"; + } + } else { + if (!ReadFstabFromFile(*swapon_all, &fstab)) { + return Error() << "Could not read fstab '" << *swapon_all << "'"; + } } if (!fs_mgr_swapon_all(fstab)) { @@ -1371,7 +1381,7 @@ const BuiltinFunctionMap& GetBuiltinFunctionMap() { {"setrlimit", {3, 3, {false, do_setrlimit}}}, {"start", {1, 1, {false, do_start}}}, {"stop", {1, 1, {false, do_stop}}}, - {"swapon_all", {1, 1, {false, do_swapon_all}}}, + {"swapon_all", {0, 1, {false, do_swapon_all}}}, {"enter_default_mount_ns", {0, 0, {false, do_enter_default_mount_ns}}}, {"symlink", {2, 2, {true, do_symlink}}}, {"sysclktz", {1, 1, {false, do_sysclktz}}}, diff --git a/init/check_builtins.cpp b/init/check_builtins.cpp index 450c079a8..481fa31e5 100644 --- a/init/check_builtins.cpp +++ b/init/check_builtins.cpp @@ -202,6 +202,14 @@ Result check_setrlimit(const BuiltinArguments& args) { return {}; } +Result check_swapon_all(const BuiltinArguments& args) { + auto options = ParseSwaponAll(args.args); + if (!options.ok()) { + return options.error(); + } + return {}; +} + Result check_sysclktz(const BuiltinArguments& args) { ReturnIfAnyArgsEmpty(); diff --git a/init/check_builtins.h b/init/check_builtins.h index 725a6fd65..dc1b752e7 100644 --- a/init/check_builtins.h +++ b/init/check_builtins.h @@ -37,6 +37,7 @@ Result check_restorecon(const BuiltinArguments& args); Result check_restorecon_recursive(const BuiltinArguments& args); Result check_setprop(const BuiltinArguments& args); Result check_setrlimit(const BuiltinArguments& args); +Result check_swapon_all(const BuiltinArguments& args); Result check_sysclktz(const BuiltinArguments& args); Result check_umount_all(const BuiltinArguments& args); Result check_wait(const BuiltinArguments& args); diff --git a/init/util.cpp b/init/util.cpp index 90ac50cd1..255434a1b 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -652,11 +652,22 @@ Result>> ParseRestorecon( return std::pair(flag, paths); } +Result ParseSwaponAll(const std::vector& args) { + if (args.size() <= 1) { + if (SelinuxGetVendorAndroidVersion() <= __ANDROID_API_Q__) { + return Error() << "swapon_all requires at least 1 argument"; + } + return {}; + } + return args[1]; +} + Result ParseUmountAll(const std::vector& args) { - if (SelinuxGetVendorAndroidVersion() <= __ANDROID_API_Q__) { - if (args.size() <= 1) { + if (args.size() <= 1) { + if (SelinuxGetVendorAndroidVersion() <= __ANDROID_API_Q__) { return Error() << "umount_all requires at least 1 argument"; } + return {}; } return args[1]; } diff --git a/init/util.h b/init/util.h index a7f813b53..3cdc9f408 100644 --- a/init/util.h +++ b/init/util.h @@ -92,6 +92,8 @@ Result ParseMountAll(const std::vector& args); Result>> ParseRestorecon( const std::vector& args); +Result ParseSwaponAll(const std::vector& args); + Result ParseUmountAll(const std::vector& args); void SetStdioToDevNull(char** argv); From 0217d52d0157b0d66e778df58ad99f50abb78366 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 1 Jun 2020 18:59:21 -0700 Subject: [PATCH 09/43] Move libadbd_auth, libadbd_fs to adbd_system_binaries. The required block inside the definition of "adbd" does nothing: we get the libraries it contains via direct dependency when building from source, and not at all when using a prebuilt. Move them to a phony rule that's explicitly listed in PRODUCT_PACKAGES. Bug: http://b/157709367 Test: treehugger Change-Id: I97c0889558482cfbe18ae91b39b6889d3fee877c (cherry picked from commit e572f2fc44be87f48a605c7c603401b712cbf841) --- adb/Android.bp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/adb/Android.bp b/adb/Android.bp index f8e5b38e3..50f62920c 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -605,16 +605,14 @@ cc_binary { ], } }, - - required: [ - "libadbd_auth", - "libadbd_fs", - ], } phony { - name: "adbd_system_binaries", + // Interface between adbd in a module and the system. + name: "adbd_system_api", required: [ + "libadbd_auth", + "libadbd_fs", "abb", "reboot", "set-verity-state", @@ -622,8 +620,10 @@ phony { } phony { - name: "adbd_system_binaries_recovery", + name: "adbd_system_api_recovery", required: [ + "libadbd_auth", + "libadbd_fs", "reboot.recovery", ], } From 11d167b57530ba5e2df1cb399d504ac44dc6732a Mon Sep 17 00:00:00 2001 From: Rick Yiu Date: Thu, 4 Jun 2020 14:28:19 +0800 Subject: [PATCH 10/43] Fine tune blkio setting to improve boot time Bug: 133200996 Test: boot time test Change-Id: I5262c28596adb7e849b202b8a163c190818f271a Merged-In: I5262c28596adb7e849b202b8a163c190818f271a --- rootdir/init.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rootdir/init.rc b/rootdir/init.rc index 01609db99..73ac7fd0d 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -162,7 +162,7 @@ on init chmod 0664 /dev/blkio/tasks chmod 0664 /dev/blkio/background/tasks write /dev/blkio/blkio.weight 1000 - write /dev/blkio/background/blkio.weight 500 + write /dev/blkio/background/blkio.weight 200 write /dev/blkio/blkio.group_idle 0 write /dev/blkio/background/blkio.group_idle 0 From 39e4a7102b3e9ec3d7f0e8013de622dad5958aa9 Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Mon, 1 Jun 2020 12:58:04 -0700 Subject: [PATCH 11/43] Set block size in dm-bow Fix block-level checkpointing to work correctly when used in combination with 512 byte hardware sectors and metadata encryption with dm-default-key v2. Bug: 153512828 Test: Parameter is passed to dm-bow based on first_api_level Change-Id: Ic0a071221559271db20b06b2f17459b5b041e02d Merged-In: Ic0a071221559271db20b06b2f17459b5b041e02d --- fs_mgr/fs_mgr.cpp | 24 ++++++++++++++++++++++-- fs_mgr/libdm/dm_target.cpp | 5 +++++ fs_mgr/libdm/include/libdm/dm_target.h | 5 ++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index b8385d335..19abe7800 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1120,8 +1120,28 @@ class CheckpointManager { } android::dm::DmTable table; - if (!table.AddTarget(std::make_unique( - 0, size, entry->blk_device))) { + auto bowTarget = + std::make_unique(0, size, entry->blk_device); + + // dm-bow uses the first block as a log record, and relocates the real first block + // elsewhere. For metadata encrypted devices, dm-bow sits below dm-default-key, and + // for post Android Q devices dm-default-key uses a block size of 4096 always. + // So if dm-bow's block size, which by default is the block size of the underlying + // hardware, is less than dm-default-key's, blocks will get broken up and I/O will + // fail as it won't be data_unit_size aligned. + // However, since it is possible there is an already shipping non + // metadata-encrypted device with smaller blocks, we must not change this for + // devices shipped with Q or earlier unless they explicitly selected dm-default-key + // v2 + constexpr unsigned int pre_gki_level = __ANDROID_API_Q__; + unsigned int options_format_version = android::base::GetUintProperty( + "ro.crypto.dm_default_key.options_format.version", + (android::fscrypt::GetFirstApiLevel() <= pre_gki_level ? 1 : 2)); + if (options_format_version > 1) { + bowTarget->SetBlockSize(4096); + } + + if (!table.AddTarget(std::move(bowTarget))) { LERROR << "Failed to add bow target"; return false; } diff --git a/fs_mgr/libdm/dm_target.cpp b/fs_mgr/libdm/dm_target.cpp index a59419877..250cb82c5 100644 --- a/fs_mgr/libdm/dm_target.cpp +++ b/fs_mgr/libdm/dm_target.cpp @@ -120,6 +120,11 @@ std::string DmTargetAndroidVerity::GetParameterString() const { return keyid_ + " " + block_device_; } +std::string DmTargetBow::GetParameterString() const { + if (!block_size_) return target_string_; + return target_string_ + " 1 block_size:" + std::to_string(block_size_); +} + std::string DmTargetSnapshot::name() const { if (mode_ == SnapshotStorageMode::Merge) { return "snapshot-merge"; diff --git a/fs_mgr/libdm/include/libdm/dm_target.h b/fs_mgr/libdm/include/libdm/dm_target.h index 57096ce2f..f986cfee0 100644 --- a/fs_mgr/libdm/include/libdm/dm_target.h +++ b/fs_mgr/libdm/include/libdm/dm_target.h @@ -175,11 +175,14 @@ class DmTargetBow final : public DmTarget { DmTargetBow(uint64_t start, uint64_t length, const std::string& target_string) : DmTarget(start, length), target_string_(target_string) {} + void SetBlockSize(uint32_t block_size) { block_size_ = block_size; } + std::string name() const override { return "bow"; } - std::string GetParameterString() const override { return target_string_; } + std::string GetParameterString() const override; private: std::string target_string_; + uint32_t block_size_ = 0; }; enum class SnapshotStorageMode { From 8f4bb45a7b09dc977b4e047ecc18150a93a362f1 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 8 Jun 2020 09:27:17 -0700 Subject: [PATCH 12/43] Stop using varargs in libkeyutils. It's error-prone, and our specific usage of it here upsets ubsan. Bug: https://issuetracker.google.com/158428513 Test: treehugger Change-Id: I3a6b68865e6b4c37ac005f5f24c3d6e1de7c5bac Merged-In: I3a6b68865e6b4c37ac005f5f24c3d6e1de7c5bac --- libkeyutils/keyutils.cpp | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/libkeyutils/keyutils.cpp b/libkeyutils/keyutils.cpp index 8f63f7058..1c5acc9ad 100644 --- a/libkeyutils/keyutils.cpp +++ b/libkeyutils/keyutils.cpp @@ -32,17 +32,7 @@ #include #include -// Deliberately not exposed. Callers should use the typed APIs instead. -static long keyctl(int cmd, ...) { - va_list va; - va_start(va, cmd); - unsigned long arg2 = va_arg(va, unsigned long); - unsigned long arg3 = va_arg(va, unsigned long); - unsigned long arg4 = va_arg(va, unsigned long); - unsigned long arg5 = va_arg(va, unsigned long); - va_end(va); - return syscall(__NR_keyctl, cmd, arg2, arg3, arg4, arg5); -} +// keyctl(2) is deliberately not exposed. Callers should use the typed APIs instead. key_serial_t add_key(const char* type, const char* description, const void* payload, size_t payload_length, key_serial_t ring_id) { @@ -50,30 +40,30 @@ key_serial_t add_key(const char* type, const char* description, const void* payl } key_serial_t keyctl_get_keyring_ID(key_serial_t id, int create) { - return keyctl(KEYCTL_GET_KEYRING_ID, id, create); + return syscall(__NR_keyctl, KEYCTL_GET_KEYRING_ID, id, create); } long keyctl_revoke(key_serial_t id) { - return keyctl(KEYCTL_REVOKE, id); + return syscall(__NR_keyctl, KEYCTL_REVOKE, id); } long keyctl_search(key_serial_t ring_id, const char* type, const char* description, key_serial_t dest_ring_id) { - return keyctl(KEYCTL_SEARCH, ring_id, type, description, dest_ring_id); + return syscall(__NR_keyctl, KEYCTL_SEARCH, ring_id, type, description, dest_ring_id); } long keyctl_setperm(key_serial_t id, int permissions) { - return keyctl(KEYCTL_SETPERM, id, permissions); + return syscall(__NR_keyctl, KEYCTL_SETPERM, id, permissions); } long keyctl_unlink(key_serial_t key, key_serial_t keyring) { - return keyctl(KEYCTL_UNLINK, key, keyring); + return syscall(__NR_keyctl, KEYCTL_UNLINK, key, keyring); } long keyctl_restrict_keyring(key_serial_t keyring, const char* type, const char* restriction) { - return keyctl(KEYCTL_RESTRICT_KEYRING, keyring, type, restriction); + return syscall(__NR_keyctl, KEYCTL_RESTRICT_KEYRING, keyring, type, restriction); } long keyctl_get_security(key_serial_t id, char* buffer, size_t buflen) { - return keyctl(KEYCTL_GET_SECURITY, id, buffer, buflen); + return syscall(__NR_keyctl, KEYCTL_GET_SECURITY, id, buffer, buflen); } From f044f2d4d4f0808e73efe61ca0514d1a7f92c365 Mon Sep 17 00:00:00 2001 From: Will Shiu Date: Fri, 3 Jan 2020 16:05:00 +0800 Subject: [PATCH 13/43] Adding wait for a device file before mount Adding the function, fs_mgr_wait_for_file(), to ensure the device file exists before to mount Bug: 158064601 Merged-In: I74a68224073932773be18a79f9334d83ea5b6947 Change-Id: I74a68224073932773be18a79f9334d83ea5b6947 (cherry picked from commit 589b7dfd83a47eb0dbaa1f7bfa95b92c449f18c0) --- fs_mgr/fs_mgr.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index b8385d335..e7ddf706e 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1757,6 +1757,11 @@ int fs_mgr_remount_userdata_into_checkpointing(Fstab* fstab) { // wrapper to __mount() and expects a fully prepared fstab_rec, // unlike fs_mgr_do_mount which does more things with avb / verity etc. int fs_mgr_do_mount_one(const FstabEntry& entry, const std::string& mount_point) { + // First check the filesystem if requested. + if (entry.fs_mgr_flags.wait && !WaitForFile(entry.blk_device, 20s)) { + LERROR << "Skipping mounting '" << entry.blk_device << "'"; + } + // Run fsck if needed prepare_fs_for_mount(entry.blk_device, entry); From b6b70c23c963831a202ac45b8ea9727293bbd8f3 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 4 Jun 2020 17:58:48 -0700 Subject: [PATCH 14/43] adbd: remove ifdefs guarding root/secure. The same adbd module prebuilt will get used for both user and userdebug builds in the post-APEX world, so we can't guard functionality with product variable ifdefs anymore. The code that was previously compiled out runs before we drop root, so the increased attack surface essentially consists of an attacker having control over system properties, and that likely implies that we're doomed already (either they have filesystem control, or they have code execution in init). Bug: http://b/158156979 Test: treehugger Change-Id: Ia70d3140189e5212beb813ff719355e30ca5fa04 (cherry picked from commit d076857c4f6c15a70f1186c4214592837f46c57d) --- adb/Android.bp | 11 ----------- adb/daemon/main.cpp | 31 +++++-------------------------- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/adb/Android.bp b/adb/Android.bp index f8e5b38e3..cf419737b 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -25,7 +25,6 @@ cc_defaults { "-Wthread-safety", "-Wvla", "-DADB_HOST=1", // overridden by adbd_defaults - "-DALLOW_ADBD_ROOT=0", // overridden by adbd_defaults "-DANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION=1", ], cpp_std: "experimental", @@ -81,16 +80,6 @@ cc_defaults { defaults: ["adb_defaults"], cflags: ["-UADB_HOST", "-DADB_HOST=0"], - product_variables: { - debuggable: { - cflags: [ - "-UALLOW_ADBD_ROOT", - "-DALLOW_ADBD_ROOT=1", - "-DALLOW_ADBD_DISABLE_VERITY", - "-DALLOW_ADBD_NO_AUTH", - ], - }, - }, } cc_defaults { diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp index 9e02e89ab..658e24456 100644 --- a/adb/daemon/main.cpp +++ b/adb/daemon/main.cpp @@ -62,23 +62,7 @@ #if defined(__ANDROID__) static const char* root_seclabel = nullptr; -static inline bool is_device_unlocked() { - return "orange" == android::base::GetProperty("ro.boot.verifiedbootstate", ""); -} - -static bool should_drop_capabilities_bounding_set() { - if (ALLOW_ADBD_ROOT || is_device_unlocked()) { - if (__android_log_is_debuggable()) { - return false; - } - } - return true; -} - static bool should_drop_privileges() { - // "adb root" not allowed, always drop privileges. - if (!ALLOW_ADBD_ROOT && !is_device_unlocked()) return true; - // The properties that affect `adb root` and `adb unroot` are ro.secure and // ro.debuggable. In this context the names don't make the expected behavior // particularly obvious. @@ -132,7 +116,7 @@ static void drop_privileges(int server_port) { // Don't listen on a port (default 5037) if running in secure mode. // Don't run as root if running in secure mode. if (should_drop_privileges()) { - const bool should_drop_caps = should_drop_capabilities_bounding_set(); + const bool should_drop_caps = !__android_log_is_debuggable(); if (should_drop_caps) { minijail_use_caps(jail.get(), CAP_TO_MASK(CAP_SETUID) | CAP_TO_MASK(CAP_SETGID)); @@ -224,15 +208,10 @@ int adbd_main(int server_port) { // descriptor will always be open. adbd_cloexec_auth_socket(); -#if defined(__ANDROID_RECOVERY__) - if (is_device_unlocked() || __android_log_is_debuggable()) { - auth_required = false; - } -#elif defined(ALLOW_ADBD_NO_AUTH) - // If ro.adb.secure is unset, default to no authentication required. - auth_required = android::base::GetBoolProperty("ro.adb.secure", false); -#elif defined(__ANDROID__) - if (is_device_unlocked()) { // allows no authentication when the device is unlocked. +#if defined(__ANDROID__) + // If we're on userdebug/eng or the device is unlocked, permit no-authentication. + bool device_unlocked = "orange" == android::base::GetProperty("ro.boot.verifiedbootstate", ""); + if (__android_log_is_debuggable() || device_unlocked) { auth_required = android::base::GetBoolProperty("ro.adb.secure", false); } #endif From 9b9584b1a373630c5d6860a4aed436622fcd2c7f Mon Sep 17 00:00:00 2001 From: Muhammad Qureshi Date: Mon, 8 Jun 2020 18:42:45 -0700 Subject: [PATCH 15/43] Increase size limit for pulled AStatsEvent Increase AStatsEvent max byte size to 50 KB for pulled events. All AStatsEvent instances are treated as pulled events unless AStatsEvent_write() is called. - Set the default max size to 50 KB - The starting buffer size is still ~4 KB for pushed and pulled events. - If a write would exceed the buffer bounds, double buffer size until write fits or 50 KB limit is exceeded in which case the overflow bit is set to true. - If AStatsEvent_write() is called, max size is set to ~4 KB. And if the current payload exceeds this limit, set overflow bit to true. - Fix error mask checking in stats_event_test. - Set ERROR_NO_ATOM_ID when atom id is missing. - Make sure tests don't hit ERROR_TOO_MANY_FIELDS when testing buffer overflow. - Rename event->size to event->numBytesWritten Fixes: 158214941 Test: libstatssocket_test Change-Id: Ia26aeb775f7e4f2ffe87707bab6d0119e21da10e --- libstats/push_compat/StatsEventCompat.cpp | 1 - libstats/socket/include/stats_event.h | 6 +- libstats/socket/stats_event.c | 81 ++++++++++++++------- libstats/socket/tests/stats_event_test.cpp | 78 ++++++++++++++++++-- libstats/socket/tests/stats_writer_test.cpp | 5 +- 5 files changed, 129 insertions(+), 42 deletions(-) diff --git a/libstats/push_compat/StatsEventCompat.cpp b/libstats/push_compat/StatsEventCompat.cpp index c17ca61f1..12a5dd4a7 100644 --- a/libstats/push_compat/StatsEventCompat.cpp +++ b/libstats/push_compat/StatsEventCompat.cpp @@ -224,7 +224,6 @@ void StatsEventCompat::addInt32Annotation(uint8_t annotationId, int32_t value) { int StatsEventCompat::writeToSocket() { if (useRSchema()) { - mAStatsEventApi.build(mEventR); return mAStatsEventApi.write(mEventR); } diff --git a/libstats/socket/include/stats_event.h b/libstats/socket/include/stats_event.h index 00dc76e3d..601a1815a 100644 --- a/libstats/socket/include/stats_event.h +++ b/libstats/socket/include/stats_event.h @@ -35,7 +35,6 @@ * AStatsEvent_addInt32Annotation(event, 2, 128); * AStatsEvent_writeFloat(event, 2.0); * - * AStatsEvent_build(event); * AStatsEvent_write(event); * AStatsEvent_release(event); * @@ -61,10 +60,11 @@ typedef struct AStatsEvent AStatsEvent; AStatsEvent* AStatsEvent_obtain(); /** - * Builds and finalizes the StatsEvent. + * Builds and finalizes the AStatsEvent for a pulled event. + * This should only be called for pulled AStatsEvents. * * After this function, the StatsEvent must not be modified in any way other than calling release or - * write. Build must be always be called before AStatsEvent_write. + * write. * * Build can be called multiple times without error. * If the event has been built before, this function is a no-op. diff --git a/libstats/socket/stats_event.c b/libstats/socket/stats_event.c index e63bc07db..a94b3a1ed 100644 --- a/libstats/socket/stats_event.c +++ b/libstats/socket/stats_event.c @@ -23,7 +23,9 @@ #define LOGGER_ENTRY_MAX_PAYLOAD 4068 // Max payload size is 4 bytes less as 4 bytes are reserved for stats_eventTag. // See android_util_Stats_Log.cpp -#define MAX_EVENT_PAYLOAD (LOGGER_ENTRY_MAX_PAYLOAD - 4) +#define MAX_PUSH_EVENT_PAYLOAD (LOGGER_ENTRY_MAX_PAYLOAD - 4) + +#define MAX_PULL_EVENT_PAYLOAD (50 * 1024) // 50 KB /* POSITIONS */ #define POS_NUM_ELEMENTS 1 @@ -70,12 +72,13 @@ struct AStatsEvent { // metadata field (e.g. timestamp) or an atom field. size_t lastFieldPos; // Number of valid bytes within the buffer. - size_t size; + size_t numBytesWritten; uint32_t numElements; uint32_t atomId; uint32_t errors; bool truncate; bool built; + size_t bufSize; }; static int64_t get_elapsed_realtime_ns() { @@ -87,14 +90,15 @@ static int64_t get_elapsed_realtime_ns() { AStatsEvent* AStatsEvent_obtain() { AStatsEvent* event = malloc(sizeof(AStatsEvent)); - event->buf = (uint8_t*)calloc(MAX_EVENT_PAYLOAD, 1); event->lastFieldPos = 0; - event->size = 2; // reserve first two bytes for outer event type and number of elements + event->numBytesWritten = 2; // reserve first 2 bytes for root event type and number of elements event->numElements = 0; event->atomId = 0; event->errors = 0; event->truncate = true; // truncate for both pulled and pushed atoms event->built = false; + event->bufSize = MAX_PUSH_EVENT_PAYLOAD; + event->buf = (uint8_t*)calloc(event->bufSize, 1); event->buf[0] = OBJECT_TYPE; AStatsEvent_writeInt64(event, get_elapsed_realtime_ns()); // write the timestamp @@ -128,19 +132,33 @@ void AStatsEvent_overwriteTimestamp(AStatsEvent* event, uint64_t timestampNs) { // Side-effect: modifies event->errors if the buffer would overflow static bool overflows(AStatsEvent* event, size_t size) { - if (event->size + size > MAX_EVENT_PAYLOAD) { + const size_t totalBytesNeeded = event->numBytesWritten + size; + if (totalBytesNeeded > MAX_PULL_EVENT_PAYLOAD) { event->errors |= ERROR_OVERFLOW; return true; } + + // Expand buffer if needed. + if (event->bufSize < MAX_PULL_EVENT_PAYLOAD && totalBytesNeeded > event->bufSize) { + do { + event->bufSize *= 2; + } while (event->bufSize <= totalBytesNeeded); + + if (event->bufSize > MAX_PULL_EVENT_PAYLOAD) { + event->bufSize = MAX_PULL_EVENT_PAYLOAD; + } + + event->buf = (uint8_t*)realloc(event->buf, event->bufSize); + } return false; } -// Side-effect: all append functions increment event->size if there is +// Side-effect: all append functions increment event->numBytesWritten if there is // sufficient space within the buffer to place the value static void append_byte(AStatsEvent* event, uint8_t value) { if (!overflows(event, sizeof(value))) { - event->buf[event->size] = value; - event->size += sizeof(value); + event->buf[event->numBytesWritten] = value; + event->numBytesWritten += sizeof(value); } } @@ -150,36 +168,36 @@ static void append_bool(AStatsEvent* event, bool value) { static void append_int32(AStatsEvent* event, int32_t value) { if (!overflows(event, sizeof(value))) { - memcpy(&event->buf[event->size], &value, sizeof(value)); - event->size += sizeof(value); + memcpy(&event->buf[event->numBytesWritten], &value, sizeof(value)); + event->numBytesWritten += sizeof(value); } } static void append_int64(AStatsEvent* event, int64_t value) { if (!overflows(event, sizeof(value))) { - memcpy(&event->buf[event->size], &value, sizeof(value)); - event->size += sizeof(value); + memcpy(&event->buf[event->numBytesWritten], &value, sizeof(value)); + event->numBytesWritten += sizeof(value); } } static void append_float(AStatsEvent* event, float value) { if (!overflows(event, sizeof(value))) { - memcpy(&event->buf[event->size], &value, sizeof(value)); - event->size += sizeof(float); + memcpy(&event->buf[event->numBytesWritten], &value, sizeof(value)); + event->numBytesWritten += sizeof(float); } } static void append_byte_array(AStatsEvent* event, const uint8_t* buf, size_t size) { if (!overflows(event, size)) { - memcpy(&event->buf[event->size], buf, size); - event->size += size; + memcpy(&event->buf[event->numBytesWritten], buf, size); + event->numBytesWritten += size; } } // Side-effect: modifies event->errors if buf is not properly null-terminated static void append_string(AStatsEvent* event, const char* buf) { - size_t size = strnlen(buf, MAX_EVENT_PAYLOAD); - if (size == MAX_EVENT_PAYLOAD) { + size_t size = strnlen(buf, MAX_PULL_EVENT_PAYLOAD); + if (size == MAX_PULL_EVENT_PAYLOAD) { event->errors |= ERROR_STRING_NOT_NULL_TERMINATED; return; } @@ -189,7 +207,7 @@ static void append_string(AStatsEvent* event, const char* buf) { } static void start_field(AStatsEvent* event, uint8_t typeId) { - event->lastFieldPos = event->size; + event->lastFieldPos = event->numBytesWritten; append_byte(event, typeId); event->numElements++; } @@ -292,7 +310,7 @@ uint32_t AStatsEvent_getAtomId(AStatsEvent* event) { } uint8_t* AStatsEvent_getBuffer(AStatsEvent* event, size_t* size) { - if (size) *size = event->size; + if (size) *size = event->numBytesWritten; return event->buf; } @@ -304,10 +322,10 @@ void AStatsEvent_truncateBuffer(AStatsEvent* event, bool truncate) { event->truncate = truncate; } -void AStatsEvent_build(AStatsEvent* event) { - if (event->built) return; - +static void build_internal(AStatsEvent* event, const bool push) { if (event->numElements > MAX_BYTE_VALUE) event->errors |= ERROR_TOO_MANY_FIELDS; + if (0 == event->atomId) event->errors |= ERROR_NO_ATOM_ID; + if (push && event->numBytesWritten > MAX_PUSH_EVENT_PAYLOAD) event->errors |= ERROR_OVERFLOW; // If there are errors, rewrite buffer. if (event->errors) { @@ -317,7 +335,7 @@ void AStatsEvent_build(AStatsEvent* event) { // Reset number of atom-level annotations to 0. event->buf[POS_ATOM_ID] = INT32_TYPE; // Now, write errors to the buffer immediately after the atom id. - event->size = POS_ATOM_ID + sizeof(uint8_t) + sizeof(uint32_t); + event->numBytesWritten = POS_ATOM_ID + sizeof(uint8_t) + sizeof(uint32_t); start_field(event, ERROR_TYPE); append_int32(event, event->errors); } @@ -326,12 +344,21 @@ void AStatsEvent_build(AStatsEvent* event) { // Truncate the buffer to the appropriate length in order to limit our // memory usage. - if (event->truncate) event->buf = (uint8_t*)realloc(event->buf, event->size); + if (event->truncate) { + event->buf = (uint8_t*)realloc(event->buf, event->numBytesWritten); + event->bufSize = event->numBytesWritten; + } +} + +void AStatsEvent_build(AStatsEvent* event) { + if (event->built) return; + + build_internal(event, false /* push */); event->built = true; } int AStatsEvent_write(AStatsEvent* event) { - AStatsEvent_build(event); - return write_buffer_to_statsd(event->buf, event->size, event->atomId); + build_internal(event, true /* push */); + return write_buffer_to_statsd(event->buf, event->numBytesWritten, event->atomId); } diff --git a/libstats/socket/tests/stats_event_test.cpp b/libstats/socket/tests/stats_event_test.cpp index 80ef145ae..cc2552119 100644 --- a/libstats/socket/tests/stats_event_test.cpp +++ b/libstats/socket/tests/stats_event_test.cpp @@ -343,27 +343,91 @@ TEST(StatsEventTest, TestNoAtomIdError) { AStatsEvent_build(event); uint32_t errors = AStatsEvent_getErrors(event); - EXPECT_NE(errors | ERROR_NO_ATOM_ID, 0); + EXPECT_EQ(errors & ERROR_NO_ATOM_ID, ERROR_NO_ATOM_ID); AStatsEvent_release(event); } -TEST(StatsEventTest, TestOverflowError) { +TEST(StatsEventTest, TestPushOverflowError) { + const char* str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const int writeCount = 120; // Number of times to write str in the event. + AStatsEvent* event = AStatsEvent_obtain(); AStatsEvent_setAtomId(event, 100); - // Add 1000 int32s to the event. Each int32 takes 5 bytes so this will + + // Add str to the event 120 times. Each str takes >35 bytes so this will // overflow the 4068 byte buffer. - for (int i = 0; i < 1000; i++) { - AStatsEvent_writeInt32(event, 0); + // We want to keep writeCount less than 127 to avoid hitting + // ERROR_TOO_MANY_FIELDS. + for (int i = 0; i < writeCount; i++) { + AStatsEvent_writeString(event, str); + } + AStatsEvent_write(event); + + uint32_t errors = AStatsEvent_getErrors(event); + EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW); + + AStatsEvent_release(event); +} + +TEST(StatsEventTest, TestPullOverflowError) { + const uint32_t atomId = 10100; + const vector bytes(430 /* number of elements */, 1 /* value of each element */); + const int writeCount = 120; // Number of times to write bytes in the event. + + AStatsEvent* event = AStatsEvent_obtain(); + AStatsEvent_setAtomId(event, atomId); + + // Add bytes to the event 120 times. Size of bytes is 430 so this will + // overflow the 50 KB pulled event buffer. + // We want to keep writeCount less than 127 to avoid hitting + // ERROR_TOO_MANY_FIELDS. + for (int i = 0; i < writeCount; i++) { + AStatsEvent_writeByteArray(event, bytes.data(), bytes.size()); } AStatsEvent_build(event); uint32_t errors = AStatsEvent_getErrors(event); - EXPECT_NE(errors | ERROR_OVERFLOW, 0); + EXPECT_EQ(errors & ERROR_OVERFLOW, ERROR_OVERFLOW); AStatsEvent_release(event); } +TEST(StatsEventTest, TestLargePull) { + const uint32_t atomId = 100; + const string str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const int writeCount = 120; // Number of times to write str in the event. + const int64_t startTime = android::elapsedRealtimeNano(); + + AStatsEvent* event = AStatsEvent_obtain(); + AStatsEvent_setAtomId(event, atomId); + + // Add str to the event 120 times. + // We want to keep writeCount less than 127 to avoid hitting + // ERROR_TOO_MANY_FIELDS. + for (int i = 0; i < writeCount; i++) { + AStatsEvent_writeString(event, str.c_str()); + } + AStatsEvent_build(event); + int64_t endTime = android::elapsedRealtimeNano(); + + size_t bufferSize; + uint8_t* buffer = AStatsEvent_getBuffer(event, &bufferSize); + uint8_t* bufferEnd = buffer + bufferSize; + + checkMetadata(&buffer, writeCount, startTime, endTime, atomId); + + // Check all instances of str have been written. + for (int i = 0; i < writeCount; i++) { + checkTypeHeader(&buffer, STRING_TYPE); + checkString(&buffer, str); + } + + EXPECT_EQ(buffer, bufferEnd); // Ensure that we have read the entire buffer. + EXPECT_EQ(AStatsEvent_getErrors(event), 0); + AStatsEvent_release(event); +} + TEST(StatsEventTest, TestAtomIdInvalidPositionError) { AStatsEvent* event = AStatsEvent_obtain(); AStatsEvent_writeInt32(event, 0); @@ -372,7 +436,7 @@ TEST(StatsEventTest, TestAtomIdInvalidPositionError) { AStatsEvent_build(event); uint32_t errors = AStatsEvent_getErrors(event); - EXPECT_NE(errors | ERROR_ATOM_ID_INVALID_POSITION, 0); + EXPECT_EQ(errors & ERROR_ATOM_ID_INVALID_POSITION, ERROR_ATOM_ID_INVALID_POSITION); AStatsEvent_release(event); } diff --git a/libstats/socket/tests/stats_writer_test.cpp b/libstats/socket/tests/stats_writer_test.cpp index 47f3517b8..749599ff3 100644 --- a/libstats/socket/tests/stats_writer_test.cpp +++ b/libstats/socket/tests/stats_writer_test.cpp @@ -20,12 +20,9 @@ #include "stats_socket.h" TEST(StatsWriterTest, TestSocketClose) { - EXPECT_TRUE(stats_log_is_closed()); - AStatsEvent* event = AStatsEvent_obtain(); AStatsEvent_setAtomId(event, 100); AStatsEvent_writeInt32(event, 5); - AStatsEvent_build(event); int successResult = AStatsEvent_write(event); AStatsEvent_release(event); @@ -36,4 +33,4 @@ TEST(StatsWriterTest, TestSocketClose) { AStatsSocket_close(); EXPECT_TRUE(stats_log_is_closed()); -} \ No newline at end of file +} From a5f8e50fbb8ebeab14110932fd53cb5fbfc1e3e6 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 9 Jun 2020 18:25:11 -0700 Subject: [PATCH 16/43] liblp: Remove tests for alignment_offset being 0. This test doesn't exist on AOSP anymore, and the bugs around alignment_offset have been fixed in R. There is no need for the test anymore. Bug: 157398966 Test: liblp_test gtest Change-Id: I00ac7486faf8db8b18f764e61db1d545feb0312c Merged-In: I563122282e940e07a7ece97ed1a9846ad1f3253c --- fs_mgr/liblp/device_test.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs_mgr/liblp/device_test.cpp b/fs_mgr/liblp/device_test.cpp index 99bff6e03..382d53d52 100644 --- a/fs_mgr/liblp/device_test.cpp +++ b/fs_mgr/liblp/device_test.cpp @@ -56,10 +56,6 @@ TEST_F(DeviceTest, BlockDeviceInfo) { // Having an alignment offset > alignment doesn't really make sense. EXPECT_LT(device_info.alignment_offset, device_info.alignment); - - if (IPropertyFetcher::GetInstance()->GetBoolProperty("ro.virtual_ab.enabled", false)) { - EXPECT_EQ(device_info.alignment_offset, 0); - } } TEST_F(DeviceTest, ReadSuperPartitionCurrentSlot) { From f2a5ed081a392149c189aab0579683dffc89f643 Mon Sep 17 00:00:00 2001 From: Ruchir Rastogi Date: Wed, 10 Jun 2020 20:43:53 -0700 Subject: [PATCH 17/43] Do not truncate AStatsEvent buffers Pushed atoms do not need to be truncated because only the bytes added to the buffer are written to the socket. Pulled atoms do not need to be truncated because within stats_pull_atom_callback.cpp, we only copy the valid parts of the buffer to the StatsEventParcel object. This improves performance by avoiding a needless call to realloc. + removed dead benchmarking code Test: m libstatssocket Test: atest libstatssocket_test Test: atest GtsStatsdHostTestCases Bug: 158717786 Change-Id: I6965f8832758203ca566336ba12d0acaf5f756d5 --- libstats/socket/Android.bp | 21 +------- libstats/socket/benchmark/main.cpp | 19 ------- .../benchmark/stats_event_benchmark.cpp | 54 ------------------- libstats/socket/include/stats_event.h | 3 -- libstats/socket/stats_event.c | 13 ----- libstats/socket/tests/stats_event_test.cpp | 2 +- 6 files changed, 2 insertions(+), 110 deletions(-) delete mode 100644 libstats/socket/benchmark/main.cpp delete mode 100644 libstats/socket/benchmark/stats_event_benchmark.cpp diff --git a/libstats/socket/Android.bp b/libstats/socket/Android.bp index 2bf0261b2..bf79ea244 100644 --- a/libstats/socket/Android.bp +++ b/libstats/socket/Android.bp @@ -89,25 +89,6 @@ cc_library_headers { min_sdk_version: "29", } -cc_benchmark { - name: "libstatssocket_benchmark", - srcs: [ - "benchmark/main.cpp", - "benchmark/stats_event_benchmark.cpp", - ], - cflags: [ - "-Wall", - "-Werror", - ], - static_libs: [ - "libstatssocket_private", - ], - shared_libs: [ - "libcutils", - "libgtest_prod", - ], -} - cc_test { name: "libstatssocket_test", srcs: [ @@ -128,7 +109,7 @@ cc_test { ], test_suites: ["device-tests", "mts"], test_config: "libstatssocket_test.xml", - //TODO(b/153588990): Remove when the build system properly separates + //TODO(b/153588990): Remove when the build system properly separates. //32bit and 64bit architectures. compile_multilib: "both", multilib: { diff --git a/libstats/socket/benchmark/main.cpp b/libstats/socket/benchmark/main.cpp deleted file mode 100644 index 5ebdf6e9a..000000000 --- a/libstats/socket/benchmark/main.cpp +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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. - */ - -#include - -BENCHMARK_MAIN(); diff --git a/libstats/socket/benchmark/stats_event_benchmark.cpp b/libstats/socket/benchmark/stats_event_benchmark.cpp deleted file mode 100644 index 3fc6e551f..000000000 --- a/libstats/socket/benchmark/stats_event_benchmark.cpp +++ /dev/null @@ -1,54 +0,0 @@ -/* - * 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. - */ - -#include "benchmark/benchmark.h" -#include "stats_event.h" - -static AStatsEvent* constructStatsEvent() { - AStatsEvent* event = AStatsEvent_obtain(); - AStatsEvent_setAtomId(event, 100); - - // randomly sample atom size - int numElements = rand() % 800; - for (int i = 0; i < numElements; i++) { - AStatsEvent_writeInt32(event, i); - } - - return event; -} - -static void BM_stats_event_truncate_buffer(benchmark::State& state) { - while (state.KeepRunning()) { - AStatsEvent* event = constructStatsEvent(); - AStatsEvent_build(event); - AStatsEvent_write(event); - AStatsEvent_release(event); - } -} - -BENCHMARK(BM_stats_event_truncate_buffer); - -static void BM_stats_event_full_buffer(benchmark::State& state) { - while (state.KeepRunning()) { - AStatsEvent* event = constructStatsEvent(); - AStatsEvent_truncateBuffer(event, false); - AStatsEvent_build(event); - AStatsEvent_write(event); - AStatsEvent_release(event); - } -} - -BENCHMARK(BM_stats_event_full_buffer); diff --git a/libstats/socket/include/stats_event.h b/libstats/socket/include/stats_event.h index 601a1815a..3d3baf9cf 100644 --- a/libstats/socket/include/stats_event.h +++ b/libstats/socket/include/stats_event.h @@ -157,9 +157,6 @@ uint32_t AStatsEvent_getAtomId(AStatsEvent* event); uint8_t* AStatsEvent_getBuffer(AStatsEvent* event, size_t* size); uint32_t AStatsEvent_getErrors(AStatsEvent* event); -// exposed for benchmarking only -void AStatsEvent_truncateBuffer(struct AStatsEvent* event, bool truncate); - #ifdef __cplusplus } #endif // __CPLUSPLUS diff --git a/libstats/socket/stats_event.c b/libstats/socket/stats_event.c index a94b3a1ed..f3e808756 100644 --- a/libstats/socket/stats_event.c +++ b/libstats/socket/stats_event.c @@ -76,7 +76,6 @@ struct AStatsEvent { uint32_t numElements; uint32_t atomId; uint32_t errors; - bool truncate; bool built; size_t bufSize; }; @@ -95,7 +94,6 @@ AStatsEvent* AStatsEvent_obtain() { event->numElements = 0; event->atomId = 0; event->errors = 0; - event->truncate = true; // truncate for both pulled and pushed atoms event->built = false; event->bufSize = MAX_PUSH_EVENT_PAYLOAD; event->buf = (uint8_t*)calloc(event->bufSize, 1); @@ -318,10 +316,6 @@ uint32_t AStatsEvent_getErrors(AStatsEvent* event) { return event->errors; } -void AStatsEvent_truncateBuffer(AStatsEvent* event, bool truncate) { - event->truncate = truncate; -} - static void build_internal(AStatsEvent* event, const bool push) { if (event->numElements > MAX_BYTE_VALUE) event->errors |= ERROR_TOO_MANY_FIELDS; if (0 == event->atomId) event->errors |= ERROR_NO_ATOM_ID; @@ -341,13 +335,6 @@ static void build_internal(AStatsEvent* event, const bool push) { } event->buf[POS_NUM_ELEMENTS] = event->numElements; - - // Truncate the buffer to the appropriate length in order to limit our - // memory usage. - if (event->truncate) { - event->buf = (uint8_t*)realloc(event->buf, event->numBytesWritten); - event->bufSize = event->numBytesWritten; - } } void AStatsEvent_build(AStatsEvent* event) { diff --git a/libstats/socket/tests/stats_event_test.cpp b/libstats/socket/tests/stats_event_test.cpp index cc2552119..9a1fac89f 100644 --- a/libstats/socket/tests/stats_event_test.cpp +++ b/libstats/socket/tests/stats_event_test.cpp @@ -18,7 +18,7 @@ #include #include -// Keep in sync stats_event.c. Consider moving to separate header file to avoid duplication. +// Keep in sync with stats_event.c. Consider moving to separate header file to avoid duplication. /* ERRORS */ #define ERROR_NO_TIMESTAMP 0x1 #define ERROR_NO_ATOM_ID 0x2 From 4e864e121640977cb6c1843121b796c6687249d4 Mon Sep 17 00:00:00 2001 From: Randall Huang Date: Fri, 12 Jun 2020 00:07:53 +0000 Subject: [PATCH 18/43] Try to recover corrupted ext4 /data with backup superblock If the superblock of /data is corrupted, fs_mgr would skip check_fs. But, e2fsck actually may reference backup superblock to recover the filesystem. This fix gives fs_mgr second chance to fix corrupted ext4 /data. Bug: 156200421 Test: boot with corrupted ext4 superblock Signed-off-by: Randall Huang Change-Id: Ieb89e7274d772962fe37927dcd62567dd5aa4657 (cherry picked from commit 72abd7b246f733e5f417e2bc529c9b482c4a778b) --- fs_mgr/fs_mgr.cpp | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 1dcc9e4a5..1c5bb6c7f 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -301,10 +301,13 @@ static bool is_ext4_superblock_valid(const struct ext4_super_block* es) { return true; } +static bool needs_block_encryption(const FstabEntry& entry); +static bool should_use_metadata_encryption(const FstabEntry& entry); + // Read the primary superblock from an ext4 filesystem. On failure return // false. If it's not an ext4 filesystem, also set FS_STAT_INVALID_MAGIC. -static bool read_ext4_superblock(const std::string& blk_device, struct ext4_super_block* sb, - int* fs_stat) { +static bool read_ext4_superblock(const std::string& blk_device, const FstabEntry& entry, + struct ext4_super_block* sb, int* fs_stat) { android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(blk_device.c_str(), O_RDONLY | O_CLOEXEC))); if (fd < 0) { @@ -321,7 +324,29 @@ static bool read_ext4_superblock(const std::string& blk_device, struct ext4_supe LINFO << "Invalid ext4 superblock on '" << blk_device << "'"; // not a valid fs, tune2fs, fsck, and mount will all fail. *fs_stat |= FS_STAT_INVALID_MAGIC; - return false; + + bool encrypted = should_use_metadata_encryption(entry) || needs_block_encryption(entry); + if (entry.mount_point == "/data" && + (!encrypted || android::base::StartsWith(blk_device, "/dev/block/dm-"))) { + // try backup superblock, if main superblock is corrupted + for (unsigned int blocksize = EXT4_MIN_BLOCK_SIZE; blocksize <= EXT4_MAX_BLOCK_SIZE; + blocksize *= 2) { + unsigned int superblock = blocksize * 8; + if (blocksize == EXT4_MIN_BLOCK_SIZE) superblock++; + + if (TEMP_FAILURE_RETRY(pread(fd, sb, sizeof(*sb), superblock * blocksize)) != + sizeof(*sb)) { + PERROR << "Can't read '" << blk_device << "' superblock"; + return false; + } + if (is_ext4_superblock_valid(sb) && + (1 << (10 + sb->s_log_block_size) == blocksize)) { + *fs_stat &= ~FS_STAT_INVALID_MAGIC; + break; + } + } + } + if (*fs_stat & FS_STAT_INVALID_MAGIC) return false; } *fs_stat |= FS_STAT_IS_EXT4; LINFO << "superblock s_max_mnt_count:" << sb->s_max_mnt_count << "," << blk_device; @@ -663,7 +688,7 @@ static int prepare_fs_for_mount(const std::string& blk_device, const FstabEntry& if (is_extfs(entry.fs_type)) { struct ext4_super_block sb; - if (read_ext4_superblock(blk_device, &sb, &fs_stat)) { + if (read_ext4_superblock(blk_device, entry, &sb, &fs_stat)) { if ((sb.s_feature_incompat & EXT4_FEATURE_INCOMPAT_RECOVER) != 0 || (sb.s_state & EXT4_VALID_FS) == 0) { LINFO << "Filesystem on " << blk_device << " was not cleanly shutdown; " @@ -693,7 +718,7 @@ static int prepare_fs_for_mount(const std::string& blk_device, const FstabEntry& entry.fs_mgr_flags.fs_verity || entry.fs_mgr_flags.ext_meta_csum)) { struct ext4_super_block sb; - if (read_ext4_superblock(blk_device, &sb, &fs_stat)) { + if (read_ext4_superblock(blk_device, entry, &sb, &fs_stat)) { tune_reserved_size(blk_device, entry, &sb, &fs_stat); tune_encrypt(blk_device, entry, &sb, &fs_stat); tune_verity(blk_device, entry, &sb, &fs_stat); From 755eb6b6069efb3edbbaa335899d34369ddea83a Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Fri, 12 Jun 2020 14:19:46 -0700 Subject: [PATCH 19/43] libsnapshot_test: hardcode alignment. SnapshotUpdateTest uses a relatively small super partition, which requires a small alignment and 0 alignment offset to work. For the purpose of this test, hardcode the alignment and offset. This test isn't about testing liblp or libdm. Fixes: 154355449 Fixes: 157437632 Bug: 158718136 Bug: 157633441 Bug: 154646936 Test: atest on devices with alignment of data partition >= 512KiB Change-Id: I1d0474f028cc824bd4197d0228350395239b3b81 (cherry picked from commit 31739669e91c7a3afc5bee23e43bc45fb4154667) Merged-In: I1d0474f028cc824bd4197d0228350395239b3b81 --- fs_mgr/libsnapshot/test_helpers.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/fs_mgr/libsnapshot/test_helpers.cpp b/fs_mgr/libsnapshot/test_helpers.cpp index f82a60275..de3d91246 100644 --- a/fs_mgr/libsnapshot/test_helpers.cpp +++ b/fs_mgr/libsnapshot/test_helpers.cpp @@ -52,10 +52,19 @@ android::base::unique_fd TestPartitionOpener::Open(const std::string& partition_ bool TestPartitionOpener::GetInfo(const std::string& partition_name, android::fs_mgr::BlockDeviceInfo* info) const { - if (partition_name == "super") { - return PartitionOpener::GetInfo(fake_super_path_, info); + if (partition_name != "super") { + return PartitionOpener::GetInfo(partition_name, info); } - return PartitionOpener::GetInfo(partition_name, info); + + if (PartitionOpener::GetInfo(fake_super_path_, info)) { + // SnapshotUpdateTest uses a relatively small super partition, which requires a small + // alignment and 0 offset to work. For the purpose of this test, hardcode the alignment + // and offset. This test isn't about testing liblp or libdm. + info->alignment_offset = 0; + info->alignment = std::min(info->alignment, static_cast(128_KiB)); + return true; + } + return false; } std::string TestPartitionOpener::GetDeviceString(const std::string& partition_name) const { From e28dc48de52af3e216fb3a85df382f3643b39496 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 16 Jun 2020 17:48:44 +0000 Subject: [PATCH 20/43] fsmgr: fix integer overflow in fs_mgr As the EXT4_MAX_BLOCK_SIZE defined as 65536 which reached maxium value of unsigned int. The superblock value maybe larger than 65536. This is found by the Integer Overflow Sanitizer. This patch fixed below boot error when userdata is corrupted: init: processing action (fs) from (/vendor/etc/init/hw/init.freescale.rc:221) init: [libfs_mgr]Invalid ext4 superblock on '/dev/block/by-name/userdata' init: InitFatalReboot: signal 6 init: #00 pc 00000000000af7e8 /system/bin/init (android::init::InitFatalReboot(int)+208) init: #01 pc 00000000000afbd0 /system/bin/init (android::init::InstallRebootSignalHandlers()::$_22::__invoke(int)+32) init: #02 pc 00000000000006bc [vdso:0000ffff9691b000] (__kernel_rt_sigreturn) init: #03 pc 000000000004e070 /system/lib64/bootstrap/libc.so (abort+176) init: #04 pc 000000000003427c /system/lib64/libfs_mgr.so (read_ext4_superblock(std::__1::basic_string, std::__1::allocator > const&, android::fs_mgr::FstabEntry const&, ext4_super_block*, int*)+1804) Test: boot with corrupted ext4 superblock Bug: 156200421 Signed-off-by: Haoran.Wang Change-Id: Ib1b69bf4623f69696cb637b226ec3359fc2ed409 (cherry picked from commit cb472b92e54ba1647c02397565958322f0d74929) --- fs_mgr/fs_mgr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 1c5bb6c7f..22d1284ec 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -331,7 +331,7 @@ static bool read_ext4_superblock(const std::string& blk_device, const FstabEntry // try backup superblock, if main superblock is corrupted for (unsigned int blocksize = EXT4_MIN_BLOCK_SIZE; blocksize <= EXT4_MAX_BLOCK_SIZE; blocksize *= 2) { - unsigned int superblock = blocksize * 8; + uint64_t superblock = blocksize * 8; if (blocksize == EXT4_MIN_BLOCK_SIZE) superblock++; if (TEMP_FAILURE_RETRY(pread(fd, sb, sizeof(*sb), superblock * blocksize)) != From 612fc47090e4bcff953806c855fe21f5c594aa7e Mon Sep 17 00:00:00 2001 From: "hyeeun.jun@samsung.com" Date: Wed, 29 Jan 2020 17:10:17 +0900 Subject: [PATCH 21/43] Fix Deadlock Issue On AppFuseBridge There are two locks used by AppFuseBridge. First is it's object lock, and the second is a mutex lock in app fuse library. There are two oppsite routines to get those locks. (Thread A) Got Java lock -> Try to get Native lock (Thread B) Got Native lock -> Try to get Java lock The order must be followed to obtain two locks. If not, the dead lock will be caused. Therefore we change the routine to get the mutex lock first, and the object lock later. Signed-off-by: hyeeun.jun@samsung.com Bug: https://issuetracker.google.com/issues/145707568 Bug: 157535024 Test: atest --test-mapping apex/blobstore Change-Id: I0ab002da9a0b7ca2f518d50ab477a080cabe3788 --- libappfuse/FuseBridgeLoop.cc | 14 ++++++++++++-- libappfuse/include/libappfuse/FuseBridgeLoop.h | 6 +++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/libappfuse/FuseBridgeLoop.cc b/libappfuse/FuseBridgeLoop.cc index f71d0c341..22f381c62 100644 --- a/libappfuse/FuseBridgeLoop.cc +++ b/libappfuse/FuseBridgeLoop.cc @@ -311,6 +311,8 @@ class BridgeEpollController : private EpollController { } }; +std::recursive_mutex FuseBridgeLoop::mutex_; + FuseBridgeLoop::FuseBridgeLoop() : opened_(true) { base::unique_fd epoll_fd(epoll_create1(EPOLL_CLOEXEC)); if (epoll_fd.get() == -1) { @@ -328,7 +330,7 @@ bool FuseBridgeLoop::AddBridge(int mount_id, base::unique_fd dev_fd, base::uniqu std::unique_ptr bridge( new FuseBridgeEntry(mount_id, std::move(dev_fd), std::move(proxy_fd))); - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_); if (!opened_) { LOG(ERROR) << "Tried to add a mount to a closed bridge"; return false; @@ -372,7 +374,7 @@ void FuseBridgeLoop::Start(FuseBridgeLoopCallback* callback) { const bool wait_result = epoll_controller_->Wait(bridges_.size(), &entries); LOG(VERBOSE) << "Receive epoll events"; { - std::lock_guard lock(mutex_); + std::lock_guard lock(mutex_); if (!(wait_result && ProcessEventLocked(entries, callback))) { for (auto it = bridges_.begin(); it != bridges_.end();) { callback->OnClosed(it->second->mount_id()); @@ -385,5 +387,13 @@ void FuseBridgeLoop::Start(FuseBridgeLoopCallback* callback) { } } +void FuseBridgeLoop::Lock() { + mutex_.lock(); +} + +void FuseBridgeLoop::Unlock() { + mutex_.unlock(); +} + } // namespace fuse } // namespace android diff --git a/libappfuse/include/libappfuse/FuseBridgeLoop.h b/libappfuse/include/libappfuse/FuseBridgeLoop.h index 6bfda9819..d5fc28fa4 100644 --- a/libappfuse/include/libappfuse/FuseBridgeLoop.h +++ b/libappfuse/include/libappfuse/FuseBridgeLoop.h @@ -50,6 +50,10 @@ class FuseBridgeLoop final { // thread from one which invokes |Start|. bool AddBridge(int mount_id, base::unique_fd dev_fd, base::unique_fd proxy_fd); + static void Lock(); + + static void Unlock(); + private: bool ProcessEventLocked(const std::unordered_set& entries, FuseBridgeLoopCallback* callback); @@ -60,7 +64,7 @@ class FuseBridgeLoop final { std::map> bridges_; // Lock for multi-threading. - std::mutex mutex_; + static std::recursive_mutex mutex_; bool opened_; From 709d56924170dc60dadbf3e69a27abe4c46c60ea Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 17 Jun 2020 09:23:14 -0700 Subject: [PATCH 22/43] liblog: fix reading pmsg d3ecc66b9c "liblog: support extended logger_entry headers" removed the logger_entry::msg variable and instead uses hdr_size as an offset from logger_entry to where the message starts in parent log_msg buffer. In pmsg, hdr_size is not recorded and therefore uninitialized when it was referenced, causing corruption when reading last logcat. This change uses sizeof(log_msg->entry) instead. Bug: 158263230 Test: last logcat works Merged-In: Ic01e73bf4d8ba8419cc770138565aa1210a6078b Change-Id: Ic01e73bf4d8ba8419cc770138565aa1210a6078b (cherry picked from commit 34d7bd98adc080041202cceec4626fd88b4b6e88) --- liblog/pmsg_reader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/liblog/pmsg_reader.cpp b/liblog/pmsg_reader.cpp index d006ba4a3..3bdc30f09 100644 --- a/liblog/pmsg_reader.cpp +++ b/liblog/pmsg_reader.cpp @@ -96,7 +96,7 @@ int PmsgRead(struct logger_list* logger_list, struct log_msg* log_msg) { ((logger_list->start.tv_sec != buf.l.realtime.tv_sec) || (logger_list->start.tv_nsec <= buf.l.realtime.tv_nsec)))) && (!logger_list->pid || (logger_list->pid == buf.p.pid))) { - char* msg = reinterpret_cast(&log_msg->entry) + log_msg->entry.hdr_size; + char* msg = reinterpret_cast(&log_msg->entry) + sizeof(log_msg->entry); *msg = buf.prio; fd = atomic_load(&logger_list->fd); if (fd <= 0) { From 502cb7af753d065886be22f5c30a258cd1b82ff4 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Mon, 15 Jun 2020 11:51:59 -0700 Subject: [PATCH 23/43] modprobe: Use more inclusive language for modprobe and libmodprobe blacklist is replaced with blocklist. Test: none Bug: 151950334 Merged-In: I59f9fde5900b9aee82aca1eab4a6ded3d136063b Change-Id: I59f9fde5900b9aee82aca1eab4a6ded3d136063b --- libmodprobe/include/modprobe/modprobe.h | 8 ++++---- libmodprobe/libmodprobe.cpp | 21 ++++++++++++--------- libmodprobe/libmodprobe_ext.cpp | 4 ++-- libmodprobe/libmodprobe_ext_test.cpp | 2 +- libmodprobe/libmodprobe_test.cpp | 10 +++++----- toolbox/modprobe.cpp | 10 +++++----- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/libmodprobe/include/modprobe/modprobe.h b/libmodprobe/include/modprobe/modprobe.h index a7687af16..4806b08b4 100644 --- a/libmodprobe/include/modprobe/modprobe.h +++ b/libmodprobe/include/modprobe/modprobe.h @@ -36,7 +36,7 @@ class Modprobe { std::vector* post_dependencies); void ResetModuleCount() { module_count_ = 0; } int GetModuleCount() { return module_count_; } - void EnableBlacklist(bool enable); + void EnableBlocklist(bool enable); void EnableVerbose(bool enable); private: @@ -55,7 +55,7 @@ class Modprobe { bool ParseSoftdepCallback(const std::vector& args); bool ParseLoadCallback(const std::vector& args); bool ParseOptionsCallback(const std::vector& args); - bool ParseBlacklistCallback(const std::vector& args); + bool ParseBlocklistCallback(const std::vector& args); void ParseKernelCmdlineOptions(); void ParseCfg(const std::string& cfg, std::function&)> f); @@ -65,8 +65,8 @@ class Modprobe { std::vector> module_post_softdep_; std::vector module_load_; std::unordered_map module_options_; - std::set module_blacklist_; + std::set module_blocklist_; std::unordered_set module_loaded_; int module_count_ = 0; - bool blacklist_enabled = false; + bool blocklist_enabled = false; }; diff --git a/libmodprobe/libmodprobe.cpp b/libmodprobe/libmodprobe.cpp index d19379676..07504c158 100644 --- a/libmodprobe/libmodprobe.cpp +++ b/libmodprobe/libmodprobe.cpp @@ -194,17 +194,18 @@ bool Modprobe::ParseOptionsCallback(const std::vector& args) { return true; } -bool Modprobe::ParseBlacklistCallback(const std::vector& args) { +bool Modprobe::ParseBlocklistCallback(const std::vector& args) { auto it = args.begin(); const std::string& type = *it++; - if (type != "blacklist") { - LOG(ERROR) << "non-blacklist line encountered in modules.blacklist"; + // +Legacy + if ((type != "blocklist") && (type != "blacklist")) { + LOG(ERROR) << "non-blocklist line encountered in modules.blocklist"; return false; } if (args.size() != 2) { - LOG(ERROR) << "lines in modules.blacklist must have exactly 2 entries, not " << args.size(); + LOG(ERROR) << "lines in modules.blocklist must have exactly 2 entries, not " << args.size(); return false; } @@ -214,7 +215,7 @@ bool Modprobe::ParseBlacklistCallback(const std::vector& args) { if (canonical_name.empty()) { return false; } - this->module_blacklist_.emplace(canonical_name); + this->module_blocklist_.emplace(canonical_name); return true; } @@ -331,16 +332,18 @@ Modprobe::Modprobe(const std::vector& base_paths, const std::string auto options_callback = std::bind(&Modprobe::ParseOptionsCallback, this, _1); ParseCfg(base_path + "/modules.options", options_callback); - auto blacklist_callback = std::bind(&Modprobe::ParseBlacklistCallback, this, _1); - ParseCfg(base_path + "/modules.blacklist", blacklist_callback); + auto blocklist_callback = std::bind(&Modprobe::ParseBlocklistCallback, this, _1); + ParseCfg(base_path + "/modules.blocklist", blocklist_callback); + // Legacy + ParseCfg(base_path + "/modules.blacklist", blocklist_callback); } ParseKernelCmdlineOptions(); android::base::SetMinimumLogSeverity(android::base::INFO); } -void Modprobe::EnableBlacklist(bool enable) { - blacklist_enabled = enable; +void Modprobe::EnableBlocklist(bool enable) { + blocklist_enabled = enable; } void Modprobe::EnableVerbose(bool enable) { diff --git a/libmodprobe/libmodprobe_ext.cpp b/libmodprobe/libmodprobe_ext.cpp index 658970874..84f71506a 100644 --- a/libmodprobe/libmodprobe_ext.cpp +++ b/libmodprobe/libmodprobe_ext.cpp @@ -80,8 +80,8 @@ bool Modprobe::Rmmod(const std::string& module_name) { bool Modprobe::ModuleExists(const std::string& module_name) { struct stat fileStat; - if (blacklist_enabled && module_blacklist_.count(module_name)) { - LOG(INFO) << "module " << module_name << " is blacklisted"; + if (blocklist_enabled && module_blocklist_.count(module_name)) { + LOG(INFO) << "module " << module_name << " is blocklisted"; return false; } auto deps = GetDependencies(module_name); diff --git a/libmodprobe/libmodprobe_ext_test.cpp b/libmodprobe/libmodprobe_ext_test.cpp index 9ee5ba7ab..e79bfaf84 100644 --- a/libmodprobe/libmodprobe_ext_test.cpp +++ b/libmodprobe/libmodprobe_ext_test.cpp @@ -72,7 +72,7 @@ bool Modprobe::Rmmod(const std::string& module_name) { bool Modprobe::ModuleExists(const std::string& module_name) { auto deps = GetDependencies(module_name); - if (blacklist_enabled && module_blacklist_.count(module_name)) { + if (blocklist_enabled && module_blocklist_.count(module_name)) { return false; } if (deps.empty()) { diff --git a/libmodprobe/libmodprobe_test.cpp b/libmodprobe/libmodprobe_test.cpp index eea0abd52..5919c49be 100644 --- a/libmodprobe/libmodprobe_test.cpp +++ b/libmodprobe/libmodprobe_test.cpp @@ -113,9 +113,9 @@ TEST(libmodprobe, Test) { "options test9.ko param_x=1 param_y=2 param_z=3\n" "options test100.ko param_1=1\n"; - const std::string modules_blacklist = - "blacklist test9.ko\n" - "blacklist test3.ko\n"; + const std::string modules_blocklist = + "blocklist test9.ko\n" + "blocklist test3.ko\n"; const std::string modules_load = "test4.ko\n" @@ -139,7 +139,7 @@ TEST(libmodprobe, Test) { 0600, getuid(), getgid())); ASSERT_TRUE(android::base::WriteStringToFile(modules_load, dir_path + "/modules.load", 0600, getuid(), getgid())); - ASSERT_TRUE(android::base::WriteStringToFile(modules_blacklist, dir_path + "/modules.blacklist", + ASSERT_TRUE(android::base::WriteStringToFile(modules_blocklist, dir_path + "/modules.blocklist", 0600, getuid(), getgid())); for (auto i = test_modules.begin(); i != test_modules.end(); ++i) { @@ -176,6 +176,6 @@ TEST(libmodprobe, Test) { EXPECT_TRUE(modules_loaded == expected_after_remove); - m.EnableBlacklist(true); + m.EnableBlocklist(true); EXPECT_FALSE(m.LoadWithAliases("test4", true)); } diff --git a/toolbox/modprobe.cpp b/toolbox/modprobe.cpp index 1b5f54e8f..3ffa74ecd 100644 --- a/toolbox/modprobe.cpp +++ b/toolbox/modprobe.cpp @@ -36,7 +36,7 @@ static void print_usage(void) { std::cerr << " modprobe [-alrqvsDb] [-d DIR] MODULE [symbol=value][...]" << std::endl; std::cerr << std::endl; std::cerr << "Options:" << std::endl; - std::cerr << " -b: Apply blacklist to module names too" << std::endl; + std::cerr << " -b: Apply blocklist to module names too" << std::endl; std::cerr << " -d: Load modules from DIR, option may be used multiple times" << std::endl; std::cerr << " -D: Print dependencies for modules only, do not load"; std::cerr << " -h: Print this help" << std::endl; @@ -59,7 +59,7 @@ extern "C" int modprobe_main(int argc, char** argv) { std::string module_parameters; std::vector mod_dirs; modprobe_mode mode = AddModulesMode; - bool blacklist = false; + bool blocklist = false; bool verbose = false; int rv = EXIT_SUCCESS; @@ -72,7 +72,7 @@ extern "C" int modprobe_main(int argc, char** argv) { check_mode(); break; case 'b': - blacklist = true; + blocklist = true; break; case 'd': mod_dirs.emplace_back(optarg); @@ -151,8 +151,8 @@ extern "C" int modprobe_main(int argc, char** argv) { Modprobe m(mod_dirs); m.EnableVerbose(verbose); - if (blacklist) { - m.EnableBlacklist(true); + if (blocklist) { + m.EnableBlocklist(true); } for (const auto& module : modules) { From e2318a2fcede97f826e5e1f68d6e9a5726b4fc2f Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Tue, 16 Jun 2020 12:03:57 -0700 Subject: [PATCH 24/43] Move zygote64 into the top-app stune group. Improve app startup performance before the new app is in the top-app cpuset. Test: boots, zygote64 in top-app stune group Bug: 159201879 Change-Id: I3aad4b4b1d2f54db9e7ba86db8a655d8552bad0a --- rootdir/init.zygote64_32.rc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rootdir/init.zygote64_32.rc b/rootdir/init.zygote64_32.rc index 70297484a..fb9e99b69 100644 --- a/rootdir/init.zygote64_32.rc +++ b/rootdir/init.zygote64_32.rc @@ -12,7 +12,7 @@ service zygote /system/bin/app_process64 -Xzygote /system/bin --zygote --start-s onrestart restart media onrestart restart netd onrestart restart wificond - writepid /dev/cpuset/foreground/tasks + task_profiles ProcessCapacityHigh MaxPerformance service zygote_secondary /system/bin/app_process32 -Xzygote /system/bin --zygote --socket-name=zygote_secondary --enable-lazy-preload class main @@ -22,4 +22,4 @@ service zygote_secondary /system/bin/app_process32 -Xzygote /system/bin --zygote socket zygote_secondary stream 660 root system socket usap_pool_secondary stream 660 root system onrestart restart zygote - writepid /dev/cpuset/foreground/tasks + task_profiles ProcessCapacityHigh MaxPerformance From 39c7961c90d2eca65ffd15006a1cf4e658ed9e33 Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Fri, 12 Jun 2020 07:32:23 -0700 Subject: [PATCH 25/43] Don't call block checkpoint functions above dm-default-key Bug: 156225476 Test: Build for f2fs and ext4 device, make sure checkpoints roll back and commit Merged-In: I7a772ff712dec9e69df175de840d69d296c65923 Change-Id: I7a772ff712dec9e69df175de840d69d296c65923 --- fs_mgr/fs_mgr.cpp | 17 ++++++++++------- fs_mgr/include/fs_mgr.h | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index b8385d335..d1f05f811 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1040,7 +1040,8 @@ static bool SupportsCheckpoint(FstabEntry* entry) { class CheckpointManager { public: - CheckpointManager(int needs_checkpoint = -1) : needs_checkpoint_(needs_checkpoint) {} + CheckpointManager(int needs_checkpoint = -1, bool metadata_encrypted = false) + : needs_checkpoint_(needs_checkpoint), metadata_encrypted_(metadata_encrypted) {} bool NeedsCheckpoint() { if (needs_checkpoint_ != UNKNOWN) { @@ -1058,7 +1059,7 @@ class CheckpointManager { return true; } - if (entry->fs_mgr_flags.checkpoint_blk) { + if (entry->fs_mgr_flags.checkpoint_blk && !metadata_encrypted_) { call_vdc({"checkpoint", "restoreCheckpoint", entry->blk_device}, nullptr); } @@ -1147,6 +1148,7 @@ class CheckpointManager { enum { UNKNOWN = -1, NO = 0, YES = 1 }; int needs_checkpoint_; + bool metadata_encrypted_; std::map device_map_; }; @@ -1775,11 +1777,11 @@ int fs_mgr_do_mount_one(const FstabEntry& entry, const std::string& mount_point) // in turn, and stop on 1st success, or no more match. static int fs_mgr_do_mount_helper(Fstab* fstab, const std::string& n_name, const std::string& n_blk_device, const char* tmp_mount_point, - int needs_checkpoint) { + int needs_checkpoint, bool metadata_encrypted) { int mount_errors = 0; int first_mount_errno = 0; std::string mount_point; - CheckpointManager checkpoint_manager(needs_checkpoint); + CheckpointManager checkpoint_manager(needs_checkpoint, metadata_encrypted); AvbUniquePtr avb_handle(nullptr); if (!fstab) { @@ -1889,12 +1891,13 @@ static int fs_mgr_do_mount_helper(Fstab* fstab, const std::string& n_name, } int fs_mgr_do_mount(Fstab* fstab, const char* n_name, char* n_blk_device, char* tmp_mount_point) { - return fs_mgr_do_mount_helper(fstab, n_name, n_blk_device, tmp_mount_point, -1); + return fs_mgr_do_mount_helper(fstab, n_name, n_blk_device, tmp_mount_point, -1, false); } int fs_mgr_do_mount(Fstab* fstab, const char* n_name, char* n_blk_device, char* tmp_mount_point, - bool needs_checkpoint) { - return fs_mgr_do_mount_helper(fstab, n_name, n_blk_device, tmp_mount_point, needs_checkpoint); + bool needs_checkpoint, bool metadata_encrypted) { + return fs_mgr_do_mount_helper(fstab, n_name, n_blk_device, tmp_mount_point, needs_checkpoint, + metadata_encrypted); } /* diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 86090c19e..2a67b8c9f 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -69,7 +69,7 @@ int fs_mgr_mount_all(android::fs_mgr::Fstab* fstab, int mount_mode); int fs_mgr_do_mount(android::fs_mgr::Fstab* fstab, const char* n_name, char* n_blk_device, char* tmp_mount_point); int fs_mgr_do_mount(android::fs_mgr::Fstab* fstab, const char* n_name, char* n_blk_device, - char* tmp_mount_point, bool need_cp); + char* tmp_mount_point, bool need_cp, bool metadata_encrypted); int fs_mgr_do_mount_one(const android::fs_mgr::FstabEntry& entry, const std::string& mount_point = ""); int fs_mgr_do_tmpfs_mount(const char *n_name); From 9aeae82f69ebb93ff0f6bd0e58f37d1d6b1dbac2 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 10 Jun 2020 23:50:02 -0700 Subject: [PATCH 26/43] remount: Do not allow remounting during checkpoints. Bug: 157540389 Test: manual test Change-Id: I5931a583e48ddac05f319629ae2f7f5f0f6cf032 Merged-In: I5931a583e48ddac05f319629ae2f7f5f0f6cf032 --- fs_mgr/Android.bp | 3 +++ fs_mgr/fs_mgr_remount.cpp | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/fs_mgr/Android.bp b/fs_mgr/Android.bp index f5daf9174..cd6459989 100644 --- a/fs_mgr/Android.bp +++ b/fs_mgr/Android.bp @@ -162,10 +162,13 @@ cc_binary { defaults: ["fs_mgr_defaults"], static_libs: [ "libavb_user", + "libutils", + "libvold_binder", ], shared_libs: [ "libbootloader_message", "libbase", + "libbinder", "libcutils", "libcrypto", "libext4_utils", diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp index 24cbad7c9..def1c2178 100644 --- a/fs_mgr/fs_mgr_remount.cpp +++ b/fs_mgr/fs_mgr_remount.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -31,6 +32,8 @@ #include #include #include +#include +#include #include #include #include @@ -103,8 +106,23 @@ void MyLogger(android::base::LogId id, android::base::LogSeverity severity, cons ::exit(0); // SUCCESS } +static android::sp GetVold() { + while (true) { + if (auto sm = android::defaultServiceManager()) { + if (auto binder = sm->getService(android::String16("vold"))) { + if (auto vold = android::interface_cast(binder)) { + return vold; + } + } + } + std::this_thread::sleep_for(2s); + } +} + } // namespace +using namespace std::chrono_literals; + static int do_remount(int argc, char* argv[]) { enum { SUCCESS = 0, @@ -118,6 +136,9 @@ static int do_remount(int argc, char* argv[]) { BAD_OVERLAY, NO_MOUNTS, REMOUNT_FAILED, + MUST_REBOOT, + BINDER_ERROR, + CHECKPOINTING } retval = SUCCESS; // If somehow this executable is delivered on a "user" build, it can @@ -191,6 +212,22 @@ static int do_remount(int argc, char* argv[]) { return NO_FSTAB; } + if (android::base::GetBoolProperty("ro.virtual_ab.enabled", false) && + !android::base::GetBoolProperty("ro.virtual_ab.retrofit", false)) { + // Virtual A/B devices can use /data as backing storage; make sure we're + // not checkpointing. + auto vold = GetVold(); + bool checkpointing = false; + if (!vold->isCheckpointing(&checkpointing).isOk()) { + LOG(ERROR) << "Could not determine checkpointing status."; + return BINDER_ERROR; + } + if (checkpointing) { + LOG(ERROR) << "Cannot use remount when a checkpoint is in progress."; + return CHECKPOINTING; + } + } + // Generate the list of supported overlayfs mount points. auto overlayfs_candidates = fs_mgr_overlayfs_candidate_list(fstab); From 21c02bbd4bc274a4e74fa030b0ca086bd87f9dea Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Tue, 16 Jun 2020 05:14:06 -0700 Subject: [PATCH 27/43] modprobe: Use more inclusive language for libmodprobe (Part Deux) Remove blacklist Test: none Bug: 151950334 Merged-In: I14ed08390a7db0b4b962343c61d60230751047ce Change-Id: I14ed08390a7db0b4b962343c61d60230751047ce --- libmodprobe/libmodprobe.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libmodprobe/libmodprobe.cpp b/libmodprobe/libmodprobe.cpp index 07504c158..5a6ae8be1 100644 --- a/libmodprobe/libmodprobe.cpp +++ b/libmodprobe/libmodprobe.cpp @@ -198,8 +198,7 @@ bool Modprobe::ParseBlocklistCallback(const std::vector& args) { auto it = args.begin(); const std::string& type = *it++; - // +Legacy - if ((type != "blocklist") && (type != "blacklist")) { + if (type != "blocklist") { LOG(ERROR) << "non-blocklist line encountered in modules.blocklist"; return false; } @@ -334,8 +333,6 @@ Modprobe::Modprobe(const std::vector& base_paths, const std::string auto blocklist_callback = std::bind(&Modprobe::ParseBlocklistCallback, this, _1); ParseCfg(base_path + "/modules.blocklist", blocklist_callback); - // Legacy - ParseCfg(base_path + "/modules.blacklist", blocklist_callback); } ParseKernelCmdlineOptions(); From 4ca9fa9e10268d7b24d77796a3cd5c7e456112fb Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 13 May 2020 06:25:20 -0700 Subject: [PATCH 28/43] llkd: Use more inclusive language Documentation is synchronized to match external, to ease updating. blacklist is replaced with ignorelist or ignore depending on context. Bug: 159345740 Test: none Merged-In: I6db7ad321684759e3c5ac1f66f940b6e8a5709a0 Change-Id: I6db7ad321684759e3c5ac1f66f940b6e8a5709a0 --- llkd/README.md | 362 ++++++++++++++++++++++++-------------------- llkd/include/llkd.h | 56 +++---- llkd/libllkd.cpp | 130 ++++++++-------- 3 files changed, 293 insertions(+), 255 deletions(-) diff --git a/llkd/README.md b/llkd/README.md index 191f98819..6f92f1474 100644 --- a/llkd/README.md +++ b/llkd/README.md @@ -1,199 +1,237 @@ -Android Live-LocK Daemon -======================== + -Android Live-LocK Daemon (llkd) is used to catch kernel deadlocks and mitigate. + -If a threadname is provided, a thread will be automatically spawned, otherwise -caller must call llkCheckMilliseconds in its main loop. Function will return -the period of time before the next expected call to this handler. +# Android Live-LocK Daemon (llkd) -Operations ----------- +Android 10 includes the Android Live-LocK Daemon +(`llkd`), which is designed to catch and mitigate kernel deadlocks. The `llkd` +component provides a default standalone implementation, but you can +alternatively integrate the `llkd` code into another service, either as part of +the main loop or as a separate thread. -There are two detection scenarios. Persistent D or Z state, and persistent +## Detection scenarios + +The `llkd` has two detection scenarios: Persistent D or Z state, and persistent stack signature. -If a thread is in D or Z state with no forward progress for longer than -ro.llk.timeout_ms, or ro.llk.[D|Z].timeout_ms, kill the process or parent -process respectively. If another scan shows the same process continues to -exist, then have a confirmed live-lock condition and need to panic. Panic -the kernel in a manner to provide the greatest bugreporting details as to the -condition. Add a alarm self watchdog should llkd ever get locked up that is -double the expected time to flow through the mainloop. Sampling is every -ro.llk_sample_ms. +### Persistent D or Z state -For usedebug releases only, persistent stack signature checking is enabled. -If a thread in any state but Z, has a persistent listed ro.llk.stack kernel -symbol always being reported, even if there is forward scheduling progress, for -longer than ro.llk.timeout_ms, or ro.llk.stack.timeout_ms, then issue a kill -to the process. If another scan shows the same process continues to exist, -then have a confirmed live-lock condition and need to panic. There is no -ABA detection since forward scheduling progress is allowed, thus the condition -for the symbols are: +If a thread is in D (uninterruptible sleep) or Z (zombie) state with no forward +progress for longer than `ro.llk.timeout_ms or ro.llk.[D|Z].timeout_ms`, the +`llkd` kills the process (or parent process). If a subsequent scan shows the +same process continues to exist, the `llkd` confirms a live-lock condition and +panics the kernel in a manner that provides the most detailed bug report for the +condition. -- Check is looking for " __symbol__+0x" or " __symbol__.cfi+0x" in - /proc/__pid__/stack. -- The __symbol__ should be rare and short lived enough that on a typical - system the function is seen at most only once in a sample over the timeout - period of ro.llk.stack.timeout_ms, samples occur every ro.llk.check_ms. This - can be the only way to prevent a false trigger as there is no ABA protection. -- Persistent continuously when the live lock condition exists. -- Should be just below the function that is calling the lock that could - contend, because if the lock is below or in the symbol function, the - symbol will show in all affected processes, not just the one that - caused the lockup. +The `llkd` includes a self watchdog that alarms if `llkd` locks up; watchdog is +double the expected time to flow through the mainloop and sampling is every +`ro.llk_sample_ms`. -Default will not monitor init, or [kthreadd] and all that [kthreadd] spawns. -This reduces the effectiveness of llkd by limiting its coverage. If there is -value in covering [kthreadd] spawned threads, the requirement will be that -the drivers not remain in a persistent 'D' state, or that they have mechanisms -to recover the thread should it be killed externally (this is good driver -coding hygiene, a common request to add such to publicly reviewed kernel.org -maintained drivers). For instance use wait_event_interruptible() instead of -wait_event(). The blacklists can be adjusted accordingly if these -conditions are met to cover kernel components. For the stack symbol checking, -there is an additional process blacklist so that we do not incide sepolicy -violations on services that block ptrace operations. +### Persistent stack signature -An accompanying gTest set have been added, and will setup a persistent D or Z -process, with and without forward progress, but not in a live-lock state -because that would require a buggy kernel, or a module or kernel modification -to stimulate. The test will check that llkd will mitigate first by killing -the appropriate process. D state is setup by vfork() waiting for exec() in -child process. Z state is setup by fork() and an un-waited for child process. -Should be noted that both of these conditions should never happen on Android -on purpose, and llkd effectively sweeps up processes that create these -conditions. If the test can, it will reconfigure llkd to expedite the test -duration by adjusting the ro.llk.* Android properties. Tests run the D state -with some scheduling progress to ensure that ABA checking prevents false -triggers. If 100% reliable ABA on platform, then ro.llk.killtest can be -set to false; however this will result in some of the unit tests to panic -kernel instead of deal with more graceful kill operation. +For userdebug releases, the `llkd` can detect kernel live-locks using persistent +stack signature checking. If a thread in any state except Z has a persistent +listed `ro.llk.stack` kernel symbol that is reported for longer than +`ro.llk.timeout_ms` or `ro.llk.stack.timeout_ms`, the `llkd` kills the process +(even if there is forward scheduling progress). If a subsequent scan shows the +same process continues to exist, the `llkd` confirms a live-lock condition and +panics the kernel in a manner that provides the most detailed bug report for the +condition. -Android Properties ------------------- +Note: Because forward scheduling progress is allowed, the `llkd` does not +perform [ABA detection](https://en.wikipedia.org/wiki/ABA_problem){:.external}. -The following are the Android Properties llkd respond to. -*prop*_ms named properties are in milliseconds. -Properties that use comma (*,*) separator for lists, use a leading separator to -preserve default and add or subtract entries with (*optional*) plus (*+*) and -minus (*-*) prefixes respectively. -For these lists, the string "*false*" is synonymous with an *empty* list, -and *blank* or *missing* resorts to the specified *default* value. +The `lldk` check persists continuously when the live lock condition exists and +looks for the composed strings `" symbol+0x"` or `" symbol.cfi+0x"` in the +`/proc/pid/stack` file on Linux. The list of symbols is in `ro.llk.stack` and +defaults to the comma-separated list of +"`cma_alloc,__get_user_pages,bit_wait_io,wait_on_page_bit_killable`". -#### ro.config.low_ram -device is configured with limited memory. +Symbols should be rare and short-lived enough that on a typical system the +function is seen only once in a sample over the timeout period of +`ro.llk.stack.timeout_ms` (samples occur every `ro.llk.check_ms`). Due to lack +of ABA protection, this is the only way to prevent a false trigger. The symbol +function must appear below the function calling the lock that could contend. If +the lock is below or in the symbol function, the symbol appears in all affected +processes, not just the one that caused the lockup. -#### ro.debuggable -device is configured for userdebug or eng build. +## Coverage -#### ro.llk.sysrq_t -default not ro.config.low_ram, or ro.debuggable if property is "eng". -if true do sysrq t (dump all threads). +The default implementation of `llkd` does not monitor `init`, `[kthreadd]`, or +`[kthreadd]` spawns. For the `llkd` to cover `[kthreadd]`-spawned threads: -#### ro.llk.enable -default false, allow live-lock daemon to be enabled. +* Drivers must not remain in a persistent D state, -#### llk.enable -default ro.llk.enable, and evaluated for eng. +OR -#### ro.khungtask.enable -default false, allow [khungtask] daemon to be enabled. +* Drivers must have mechanisms to recover the thread should it be killed + externally. For example, use `wait_event_interruptible()` instead of + `wait_event()`. -#### khungtask.enable -default ro.khungtask.enable and evaluated for eng. +If one of the above conditions is met, the `llkd` ignorelist can be adjusted to +cover kernel components. Stack symbol checking involves an additional process +ignore list to prevent sepolicy violations on services that block `ptrace` +operations. -#### ro.llk.mlockall -default false, enable call to mlockall(). +## Android properties -#### ro.khungtask.timeout -default value 12 minutes, [khungtask] maximum timelimit. +The `llkd` responds to several Android properties (listed below). -#### ro.llk.timeout_ms -default 10 minutes, D or Z maximum timelimit, double this value and it sets -the alarm watchdog for llkd. +* Properties named `prop_ms` are in milliseconds. +* Properties that use comma (,) separator for lists use a leading separator to + preserve the default entry, then add or subtract entries with optional plus + (+) and minus (-) prefixes respectively. For these lists, the string "false" + is synonymous with an empty list, and blank or missing entries resort to the + specified default value. -#### ro.llk.D.timeout_ms -default ro.llk.timeout_ms, D maximum timelimit. +### ro.config.low_ram -#### ro.llk.Z.timeout_ms -default ro.llk.timeout_ms, Z maximum timelimit. +Device is configured with limited memory. -#### ro.llk.stack.timeout_ms -default ro.llk.timeout_ms, -checking for persistent stack symbols maximum timelimit. -Only active on userdebug or eng builds. +### ro.debuggable -#### ro.llk.check_ms -default 2 minutes samples of threads for D or Z. +Device is configured for userdebug or eng build. -#### ro.llk.stack -default cma_alloc,__get_user_pages,bit_wait_io,wait_on_page_bit_killable -comma separated list of kernel symbols. -Look for kernel stack symbols that if ever persistently present can -indicate a subsystem is locked up. -Beware, check does not on purpose do forward scheduling ABA except by polling -every ro.llk_check_ms over the period ro.llk.stack.timeout_ms, so stack symbol -should be exceptionally rare and fleeting. -One must be convinced that it is virtually *impossible* for symbol to show up -persistently in all samples of the stack. -Again, looks for a match for either " **symbol**+0x" or " **symbol**.cfi+0x" -in stack expansion. -Only available on userdebug or eng builds, limited privileges due to security -concerns on user builds prevents this checking. +### ro.llk.sysrq_t -#### ro.llk.blacklist.process -default 0,1,2 (kernel, init and [kthreadd]) plus process names -init,[kthreadd],[khungtaskd],lmkd,llkd,watchdogd, -[watchdogd],[watchdogd/0],...,[watchdogd/***get_nprocs**-1*]. -Do not watch these processes. A process can be comm, cmdline or pid reference. -NB: automated default here can be larger than the current maximum property -size of 92. -NB: false is a very very very unlikely process to want to blacklist. +If property is "eng", the default is not `ro.config.low_ram` or `ro.debuggable`. +If true, dump all threads (`sysrq t`). -#### ro.llk.blacklist.parent -default 0,2,adbd&[setsid] (kernel, [kthreadd] and adbd *only for zombie setsid*). -Do not watch processes that have this parent. -An ampersand (*&*) separator is used to specify that the parent is ignored -only in combination with the target child process. -Ampersand was selected because it is never part of a process name, -however a setprop in the shell requires it to be escaped or quoted; -init rc file where this is normally specified does not have this issue. -A parent or target processes can be specified as comm, cmdline or pid reference. +### ro.llk.enable -#### ro.llk.blacklist.uid -default *empty* or false, comma separated list of uid numbers or names. -Do not watch processes that match this uid. +Allow live-lock daemon to be enabled. Default is false. -#### ro.llk.blacklist.process.stack -default process names init,lmkd.llkd,llkd,keystore,ueventd,apexd,logd. -This subset of processes are not monitored for live lock stack signatures. -Also prevents the sepolicy violation associated with processes that block -ptrace, as these can not be checked anyways. -Only active on userdebug and eng builds. +### llk.enable -Architectural Concerns ----------------------- +Evaluated for eng builds. Default is `ro.llk.enable`. -- built-in [khungtask] daemon is too generic and trips on driver code that - sits around in D state too much. To switch to S instead makes the task(s) - killable, so the drivers should be able to resurrect them if needed. -- Properties are limited to 92 characters. -- Create kernel module and associated gTest to actually test panic. -- Create gTest to test out blacklist (ro.llk.blacklist.*properties* generally - not be inputs). Could require more test-only interfaces to libllkd. -- Speed up gTest using something else than ro.llk.*properties*, which should - not be inputs as they should be baked into the product. +### ro.khungtask.enable + +Allow `[khungtask]` daemon to be enabled. Default is false. + +### khungtask.enable + +Evaluated for eng builds. Default is `ro.khungtask.enable`. + +### ro.llk.mlockall + +Enable call to `mlockall()`. Default is false. + +### ro.khungtask.timeout + +`[khungtask]` maximum time limit. Default is 12 minutes. + +### ro.llk.timeout_ms + +D or Z maximum time limit. Default is 10 minutes. Double this value to set the +alarm watchdog for `llkd`. + +### ro.llk.D.timeout_ms + +D maximum time limit. Default is `ro.llk.timeout_ms`. + +### ro.llk.Z.timeout_ms + +Z maximum time limit. Default is `ro.llk.timeout_ms`. + +### ro.llk.stack.timeout_ms + +Checks for persistent stack symbols maximum time limit. Default is +`ro.llk.timeout_ms`. **Active only on userdebug or eng builds**. + +### ro.llk.check_ms + +Samples of threads for D or Z. Default is two minutes. + +### ro.llk.stack + +Checks for kernel stack symbols that if persistently present can indicate a +subsystem is locked up. Default is +`cma_alloc,__get_user_pages,bit_wait_io,wait_on_page_bit_killable` +comma-separated list of kernel symbols. The check doesn't do forward scheduling +ABA except by polling every `ro.llk_check_ms` over the period +`ro.llk.stack.timeout_ms`, so stack symbols should be exceptionally rare and +fleeting (it is highly unlikely for a symbol to show up persistently in all +samples of the stack). Checks for a match for `" symbol+0x"` or +`" symbol.cfi+0x"` in stack expansion. **Available only on userdebug or eng +builds**; security concerns on user builds result in limited privileges that +prevent this check. + +### ro.llk.ignorelist.process + +The `llkd` does not watch the specified processes. Default is `0,1,2` (`kernel`, +`init`, and `[kthreadd]`) plus process names +`init,[kthreadd],[khungtaskd],lmkd,llkd,watchdogd, [watchdogd],[watchdogd/0],...,[watchdogd/get_nprocs-1]`. +A process can be a `comm`, `cmdline`, or `pid` reference. An automated default +can be larger than the current maximum property size of 92. + +Note: `false` is an extremely unlikely process to want to ignore. + +### ro.llk.ignorelist.parent + +The `llkd` does not watch processes that have the specified parent(s). Default +is `0,2,adbd&[setsid]` (`kernel`, `[kthreadd]`, and `adbd` only for zombie +`setsid`). An ampersand (&) separator specifies that the parent is ignored only +in combination with the target child process. Ampersand was selected because it +is never part of a process name; however, a `setprop` in the shell requires the +ampersand to be escaped or quoted, although the `init rc` file where this is +normally specified does not have this issue. A parent or target process can be a +`comm`, `cmdline`, or `pid` reference. + +### ro.llk.ignorelist.uid + +The `llkd` does not watch processes that match the specified uid(s). +Comma-separated list of uid numbers or names. Default is empty or false. + +### ro.llk.ignorelist.process.stack + +The `llkd` does not monitor the specified subset of processes for live lock stack +signatures. Default is process names +`init,lmkd.llkd,llkd,keystore,ueventd,apexd,logd`. Prevents the sepolicy +violation associated with processes that block `ptrace` (as these can't be +checked). **Active only on userdebug and eng builds**. For details on build +types, refer to [Building Android](/setup/build/building#choose-a-target). + +## Architectural concerns + +* Properties are limited to 92 characters. However, this is not limited for + defaults defined in the `include/llkd.h` file in the sources. +* The built-in `[khungtask]` daemon is too generic and trips on driver code that + sits around in D state too much. Switching drivers to sleep, or S state, + would make task(s) killable, and need to be resurrectable by drivers on an + as-need basis. + +## Library interface (optional) + +You can optionally incorporate the `llkd` into another privileged daemon using +the following C interface from the `libllkd` component: + +``` +#include "llkd.h" +bool llkInit(const char* threadname) /* return true if enabled */ +unsigned llkCheckMillseconds(void) /* ms to sleep for next check */ +``` + +If a threadname is provided, a thread automatically spawns, otherwise the caller +must call `llkCheckMilliseconds` in its main loop. The function returns the +period of time before the next expected call to this handler. diff --git a/llkd/include/llkd.h b/llkd/include/llkd.h index 3586ca1b1..4b20a56da 100644 --- a/llkd/include/llkd.h +++ b/llkd/include/llkd.h @@ -30,37 +30,37 @@ bool llkInit(const char* threadname); /* threadname NULL, not spawned */ unsigned llkCheckMilliseconds(void); /* clang-format off */ -#define LLK_ENABLE_WRITEABLE_PROPERTY "llk.enable" -#define LLK_ENABLE_PROPERTY "ro." LLK_ENABLE_WRITEABLE_PROPERTY -#define LLK_ENABLE_DEFAULT false /* "eng" and userdebug true */ -#define KHT_ENABLE_WRITEABLE_PROPERTY "khungtask.enable" -#define KHT_ENABLE_PROPERTY "ro." KHT_ENABLE_WRITEABLE_PROPERTY -#define LLK_ENABLE_SYSRQ_T_PROPERTY "ro.llk.sysrq_t" -#define LLK_ENABLE_SYSRQ_T_DEFAULT true -#define LLK_MLOCKALL_PROPERTY "ro.llk.mlockall" -#define LLK_MLOCKALL_DEFAULT true -#define LLK_KILLTEST_PROPERTY "ro.llk.killtest" -#define LLK_KILLTEST_DEFAULT true -#define LLK_TIMEOUT_MS_PROPERTY "ro.llk.timeout_ms" -#define KHT_TIMEOUT_PROPERTY "ro.khungtask.timeout" -#define LLK_D_TIMEOUT_MS_PROPERTY "ro.llk.D.timeout_ms" -#define LLK_Z_TIMEOUT_MS_PROPERTY "ro.llk.Z.timeout_ms" -#define LLK_STACK_TIMEOUT_MS_PROPERTY "ro.llk.stack.timeout_ms" -#define LLK_CHECK_MS_PROPERTY "ro.llk.check_ms" +#define LLK_ENABLE_WRITEABLE_PROPERTY "llk.enable" +#define LLK_ENABLE_PROPERTY "ro." LLK_ENABLE_WRITEABLE_PROPERTY +#define LLK_ENABLE_DEFAULT false /* "eng" and userdebug true */ +#define KHT_ENABLE_WRITEABLE_PROPERTY "khungtask.enable" +#define KHT_ENABLE_PROPERTY "ro." KHT_ENABLE_WRITEABLE_PROPERTY +#define LLK_ENABLE_SYSRQ_T_PROPERTY "ro.llk.sysrq_t" +#define LLK_ENABLE_SYSRQ_T_DEFAULT true +#define LLK_MLOCKALL_PROPERTY "ro.llk.mlockall" +#define LLK_MLOCKALL_DEFAULT true +#define LLK_KILLTEST_PROPERTY "ro.llk.killtest" +#define LLK_KILLTEST_DEFAULT true +#define LLK_TIMEOUT_MS_PROPERTY "ro.llk.timeout_ms" +#define KHT_TIMEOUT_PROPERTY "ro.khungtask.timeout" +#define LLK_D_TIMEOUT_MS_PROPERTY "ro.llk.D.timeout_ms" +#define LLK_Z_TIMEOUT_MS_PROPERTY "ro.llk.Z.timeout_ms" +#define LLK_STACK_TIMEOUT_MS_PROPERTY "ro.llk.stack.timeout_ms" +#define LLK_CHECK_MS_PROPERTY "ro.llk.check_ms" /* LLK_CHECK_MS_DEFAULT = actual timeout_ms / LLK_CHECKS_PER_TIMEOUT_DEFAULT */ -#define LLK_CHECKS_PER_TIMEOUT_DEFAULT 5 -#define LLK_CHECK_STACK_PROPERTY "ro.llk.stack" -#define LLK_CHECK_STACK_DEFAULT \ +#define LLK_CHECKS_PER_TIMEOUT_DEFAULT 5 +#define LLK_CHECK_STACK_PROPERTY "ro.llk.stack" +#define LLK_CHECK_STACK_DEFAULT \ "cma_alloc,__get_user_pages,bit_wait_io,wait_on_page_bit_killable" -#define LLK_BLACKLIST_PROCESS_PROPERTY "ro.llk.blacklist.process" -#define LLK_BLACKLIST_PROCESS_DEFAULT \ +#define LLK_IGNORELIST_PROCESS_PROPERTY "ro.llk.ignorelist.process" +#define LLK_IGNORELIST_PROCESS_DEFAULT \ "0,1,2,init,[kthreadd],[khungtaskd],lmkd,llkd,watchdogd,[watchdogd],[watchdogd/0]" -#define LLK_BLACKLIST_PARENT_PROPERTY "ro.llk.blacklist.parent" -#define LLK_BLACKLIST_PARENT_DEFAULT "0,2,[kthreadd],adbd&[setsid]" -#define LLK_BLACKLIST_UID_PROPERTY "ro.llk.blacklist.uid" -#define LLK_BLACKLIST_UID_DEFAULT "" -#define LLK_BLACKLIST_STACK_PROPERTY "ro.llk.blacklist.process.stack" -#define LLK_BLACKLIST_STACK_DEFAULT "init,lmkd.llkd,llkd,keystore,ueventd,apexd" +#define LLK_IGNORELIST_PARENT_PROPERTY "ro.llk.ignorelist.parent" +#define LLK_IGNORELIST_PARENT_DEFAULT "0,2,[kthreadd],adbd&[setsid]" +#define LLK_IGNORELIST_UID_PROPERTY "ro.llk.ignorelist.uid" +#define LLK_IGNORELIST_UID_DEFAULT "" +#define LLK_IGNORELIST_STACK_PROPERTY "ro.llk.ignorelist.process.stack" +#define LLK_IGNORELIST_STACK_DEFAULT "init,lmkd.llkd,llkd,keystore,ueventd,apexd" /* clang-format on */ __END_DECLS diff --git a/llkd/libllkd.cpp b/llkd/libllkd.cpp index 8ad9900ec..a24d900dc 100644 --- a/llkd/libllkd.cpp +++ b/llkd/libllkd.cpp @@ -98,26 +98,26 @@ seconds khtTimeout = duration_cast(llkTimeoutMs * (1 + LLK_CHECKS_PER_T std::unordered_set llkCheckStackSymbols; #endif -// Blacklist variables, initialized with comma separated lists of high false +// Ignorelist variables, initialized with comma separated lists of high false // positive and/or dangerous references, e.g. without self restart, for pid, // ppid, name and uid: // list of pids, or tids or names to skip. kernel pid (0), init pid (1), // [kthreadd] pid (2), ourselves, "init", "[kthreadd]", "lmkd", "llkd" or // combinations of watchdogd in kernel and user space. -std::unordered_set llkBlacklistProcess; +std::unordered_set llkIgnorelistProcess; // list of parent pids, comm or cmdline names to skip. default: // kernel pid (0), [kthreadd] (2), or ourselves, enforced and implied -std::unordered_set llkBlacklistParent; +std::unordered_set llkIgnorelistParent; // list of parent and target processes to skip. default: // adbd *and* [setsid] -std::unordered_map> llkBlacklistParentAndChild; +std::unordered_map> llkIgnorelistParentAndChild; // list of uids, and uid names, to skip, default nothing -std::unordered_set llkBlacklistUid; +std::unordered_set llkIgnorelistUid; #ifdef __PTRACE_ENABLED__ // list of names to skip stack checking. "init", "lmkd", "llkd", "keystore" or // "logd" (if not userdebug). -std::unordered_set llkBlacklistStack; +std::unordered_set llkIgnorelistStack; #endif class dir { @@ -626,9 +626,9 @@ std::string llkFormat(bool flag) { return flag ? "true" : "false"; } -std::string llkFormat(const std::unordered_set& blacklist) { +std::string llkFormat(const std::unordered_set& ignorelist) { std::string ret; - for (const auto& entry : blacklist) { + for (const auto& entry : ignorelist) { if (!ret.empty()) ret += ","; ret += entry; } @@ -636,10 +636,10 @@ std::string llkFormat(const std::unordered_set& blacklist) { } std::string llkFormat( - const std::unordered_map>& blacklist, + const std::unordered_map>& ignorelist, bool leading_comma = false) { std::string ret; - for (const auto& entry : blacklist) { + for (const auto& entry : ignorelist) { for (const auto& target : entry.second) { if (leading_comma || !ret.empty()) ret += ","; ret += entry.first + "&" + target; @@ -699,61 +699,61 @@ std::unordered_set llkSplit(const std::string& prop, const std::str } bool llkSkipName(const std::string& name, - const std::unordered_set& blacklist = llkBlacklistProcess) { - if (name.empty() || blacklist.empty()) return false; + const std::unordered_set& ignorelist = llkIgnorelistProcess) { + if (name.empty() || ignorelist.empty()) return false; - return blacklist.find(name) != blacklist.end(); + return ignorelist.find(name) != ignorelist.end(); } bool llkSkipProc(proc* procp, - const std::unordered_set& blacklist = llkBlacklistProcess) { + const std::unordered_set& ignorelist = llkIgnorelistProcess) { if (!procp) return false; - if (llkSkipName(std::to_string(procp->pid), blacklist)) return true; - if (llkSkipName(procp->getComm(), blacklist)) return true; - if (llkSkipName(procp->getCmdline(), blacklist)) return true; - if (llkSkipName(android::base::Basename(procp->getCmdline()), blacklist)) return true; + if (llkSkipName(std::to_string(procp->pid), ignorelist)) return true; + if (llkSkipName(procp->getComm(), ignorelist)) return true; + if (llkSkipName(procp->getCmdline(), ignorelist)) return true; + if (llkSkipName(android::base::Basename(procp->getCmdline()), ignorelist)) return true; return false; } const std::unordered_set& llkSkipName( const std::string& name, - const std::unordered_map>& blacklist) { + const std::unordered_map>& ignorelist) { static const std::unordered_set empty; - if (name.empty() || blacklist.empty()) return empty; - auto found = blacklist.find(name); - if (found == blacklist.end()) return empty; + if (name.empty() || ignorelist.empty()) return empty; + auto found = ignorelist.find(name); + if (found == ignorelist.end()) return empty; return found->second; } bool llkSkipPproc(proc* pprocp, proc* procp, const std::unordered_map>& - blacklist = llkBlacklistParentAndChild) { - if (!pprocp || !procp || blacklist.empty()) return false; - if (llkSkipProc(procp, llkSkipName(std::to_string(pprocp->pid), blacklist))) return true; - if (llkSkipProc(procp, llkSkipName(pprocp->getComm(), blacklist))) return true; - if (llkSkipProc(procp, llkSkipName(pprocp->getCmdline(), blacklist))) return true; + ignorelist = llkIgnorelistParentAndChild) { + if (!pprocp || !procp || ignorelist.empty()) return false; + if (llkSkipProc(procp, llkSkipName(std::to_string(pprocp->pid), ignorelist))) return true; + if (llkSkipProc(procp, llkSkipName(pprocp->getComm(), ignorelist))) return true; + if (llkSkipProc(procp, llkSkipName(pprocp->getCmdline(), ignorelist))) return true; return llkSkipProc(procp, - llkSkipName(android::base::Basename(pprocp->getCmdline()), blacklist)); + llkSkipName(android::base::Basename(pprocp->getCmdline()), ignorelist)); } bool llkSkipPid(pid_t pid) { - return llkSkipName(std::to_string(pid), llkBlacklistProcess); + return llkSkipName(std::to_string(pid), llkIgnorelistProcess); } bool llkSkipPpid(pid_t ppid) { - return llkSkipName(std::to_string(ppid), llkBlacklistParent); + return llkSkipName(std::to_string(ppid), llkIgnorelistParent); } bool llkSkipUid(uid_t uid) { // Match by number? - if (llkSkipName(std::to_string(uid), llkBlacklistUid)) { + if (llkSkipName(std::to_string(uid), llkIgnorelistUid)) { return true; } // Match by name? auto pwd = ::getpwuid(uid); return (pwd != nullptr) && __predict_true(pwd->pw_name != nullptr) && - __predict_true(pwd->pw_name[0] != '\0') && llkSkipName(pwd->pw_name, llkBlacklistUid); + __predict_true(pwd->pw_name[0] != '\0') && llkSkipName(pwd->pw_name, llkIgnorelistUid); } bool getValidTidDir(dirent* dp, std::string* piddir) { @@ -811,7 +811,7 @@ bool llkCheckStack(proc* procp, const std::string& piddir) { } // Don't check process that are known to block ptrace, save sepolicy noise. - if (llkSkipProc(procp, llkBlacklistStack)) return false; + if (llkSkipProc(procp, llkIgnorelistStack)) return false; auto kernel_stack = ReadFile(piddir + "/stack"); if (kernel_stack.empty()) { LOG(VERBOSE) << piddir << "/stack empty comm=" << procp->getComm() @@ -917,12 +917,12 @@ void llkLogConfig(void) { << LLK_CHECK_MS_PROPERTY "=" << llkFormat(llkCheckMs) << "\n" #ifdef __PTRACE_ENABLED__ << LLK_CHECK_STACK_PROPERTY "=" << llkFormat(llkCheckStackSymbols) << "\n" - << LLK_BLACKLIST_STACK_PROPERTY "=" << llkFormat(llkBlacklistStack) << "\n" + << LLK_IGNORELIST_STACK_PROPERTY "=" << llkFormat(llkIgnorelistStack) << "\n" #endif - << LLK_BLACKLIST_PROCESS_PROPERTY "=" << llkFormat(llkBlacklistProcess) << "\n" - << LLK_BLACKLIST_PARENT_PROPERTY "=" << llkFormat(llkBlacklistParent) - << llkFormat(llkBlacklistParentAndChild, true) << "\n" - << LLK_BLACKLIST_UID_PROPERTY "=" << llkFormat(llkBlacklistUid); + << LLK_IGNORELIST_PROCESS_PROPERTY "=" << llkFormat(llkIgnorelistProcess) << "\n" + << LLK_IGNORELIST_PARENT_PROPERTY "=" << llkFormat(llkIgnorelistParent) + << llkFormat(llkIgnorelistParentAndChild, true) << "\n" + << LLK_IGNORELIST_UID_PROPERTY "=" << llkFormat(llkIgnorelistUid); } void* llkThread(void* obj) { @@ -932,14 +932,14 @@ void* llkThread(void* obj) { std::string name = std::to_string(::gettid()); if (!llkSkipName(name)) { - llkBlacklistProcess.emplace(name); + llkIgnorelistProcess.emplace(name); } name = static_cast(obj); prctl(PR_SET_NAME, name.c_str()); if (__predict_false(!llkSkipName(name))) { - llkBlacklistProcess.insert(name); + llkIgnorelistProcess.insert(name); } - // No longer modifying llkBlacklistProcess. + // No longer modifying llkIgnorelistProcess. llkRunning = true; llkLogConfig(); while (llkRunning) { @@ -1122,12 +1122,12 @@ milliseconds llkCheck(bool checkRunning) { } if (pprocp) { if (llkSkipPproc(pprocp, procp)) break; - if (llkSkipProc(pprocp, llkBlacklistParent)) break; + if (llkSkipProc(pprocp, llkIgnorelistParent)) break; } else { - if (llkSkipName(std::to_string(ppid), llkBlacklistParent)) break; + if (llkSkipName(std::to_string(ppid), llkIgnorelistParent)) break; } - if ((llkBlacklistUid.size() != 0) && llkSkipUid(procp->getUid())) { + if ((llkIgnorelistUid.size() != 0) && llkSkipUid(procp->getUid())) { continue; } @@ -1320,29 +1320,29 @@ bool llkInit(const char* threadname) { if (debuggable) { llkCheckStackSymbols = llkSplit(LLK_CHECK_STACK_PROPERTY, LLK_CHECK_STACK_DEFAULT); } - std::string defaultBlacklistStack(LLK_BLACKLIST_STACK_DEFAULT); - if (!debuggable) defaultBlacklistStack += ",logd,/system/bin/logd"; - llkBlacklistStack = llkSplit(LLK_BLACKLIST_STACK_PROPERTY, defaultBlacklistStack); + std::string defaultIgnorelistStack(LLK_IGNORELIST_STACK_DEFAULT); + if (!debuggable) defaultIgnorelistStack += ",logd,/system/bin/logd"; + llkIgnorelistStack = llkSplit(LLK_IGNORELIST_STACK_PROPERTY, defaultIgnorelistStack); #endif - std::string defaultBlacklistProcess( - std::to_string(kernelPid) + "," + std::to_string(initPid) + "," + - std::to_string(kthreaddPid) + "," + std::to_string(::getpid()) + "," + - std::to_string(::gettid()) + "," LLK_BLACKLIST_PROCESS_DEFAULT); + std::string defaultIgnorelistProcess( + std::to_string(kernelPid) + "," + std::to_string(initPid) + "," + + std::to_string(kthreaddPid) + "," + std::to_string(::getpid()) + "," + + std::to_string(::gettid()) + "," LLK_IGNORELIST_PROCESS_DEFAULT); if (threadname) { - defaultBlacklistProcess += ","s + threadname; + defaultIgnorelistProcess += ","s + threadname; } for (int cpu = 1; cpu < get_nprocs_conf(); ++cpu) { - defaultBlacklistProcess += ",[watchdog/" + std::to_string(cpu) + "]"; + defaultIgnorelistProcess += ",[watchdog/" + std::to_string(cpu) + "]"; } - llkBlacklistProcess = llkSplit(LLK_BLACKLIST_PROCESS_PROPERTY, defaultBlacklistProcess); + llkIgnorelistProcess = llkSplit(LLK_IGNORELIST_PROCESS_PROPERTY, defaultIgnorelistProcess); if (!llkSkipName("[khungtaskd]")) { // ALWAYS ignore as special - llkBlacklistProcess.emplace("[khungtaskd]"); + llkIgnorelistProcess.emplace("[khungtaskd]"); } - llkBlacklistParent = llkSplit(LLK_BLACKLIST_PARENT_PROPERTY, - std::to_string(kernelPid) + "," + std::to_string(kthreaddPid) + - "," LLK_BLACKLIST_PARENT_DEFAULT); - // derive llkBlacklistParentAndChild by moving entries with '&' from above - for (auto it = llkBlacklistParent.begin(); it != llkBlacklistParent.end();) { + llkIgnorelistParent = llkSplit(LLK_IGNORELIST_PARENT_PROPERTY, + std::to_string(kernelPid) + "," + std::to_string(kthreaddPid) + + "," LLK_IGNORELIST_PARENT_DEFAULT); + // derive llkIgnorelistParentAndChild by moving entries with '&' from above + for (auto it = llkIgnorelistParent.begin(); it != llkIgnorelistParent.end();) { auto pos = it->find('&'); if (pos == std::string::npos) { ++it; @@ -1350,18 +1350,18 @@ bool llkInit(const char* threadname) { } auto parent = it->substr(0, pos); auto child = it->substr(pos + 1); - it = llkBlacklistParent.erase(it); + it = llkIgnorelistParent.erase(it); - auto found = llkBlacklistParentAndChild.find(parent); - if (found == llkBlacklistParentAndChild.end()) { - llkBlacklistParentAndChild.emplace(std::make_pair( + auto found = llkIgnorelistParentAndChild.find(parent); + if (found == llkIgnorelistParentAndChild.end()) { + llkIgnorelistParentAndChild.emplace(std::make_pair( std::move(parent), std::unordered_set({std::move(child)}))); } else { found->second.emplace(std::move(child)); } } - llkBlacklistUid = llkSplit(LLK_BLACKLIST_UID_PROPERTY, LLK_BLACKLIST_UID_DEFAULT); + llkIgnorelistUid = llkSplit(LLK_IGNORELIST_UID_PROPERTY, LLK_IGNORELIST_UID_DEFAULT); // internal watchdog ::signal(SIGALRM, llkAlarmHandler); From 618ca3400d4c7cf44dee018cdf09e1c9f1c4e5db Mon Sep 17 00:00:00 2001 From: Steve Muckle Date: Thu, 18 Jun 2020 17:02:15 -0700 Subject: [PATCH 29/43] fastboot: copy AVB footer on boot image to end of partition If the flashed boot image is smaller than the block device, the AVB footer will not be at the end of the partition. Although images are normally created to match the partition size the GKI boot.img must work on all devices, and the size of the boot partition will vary. Copy the AVB footer to the end of the partition before flashing, if it is not there already. Bug: 159377163 Change-Id: I5a5e25fb54dc9d6a2930fda63434968808ffa1f0 Merged-In: I5a5e25fb54dc9d6a2930fda63434968808ffa1f0 --- fastboot/fastboot.cpp | 68 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index 7f6e7230f..5307a0081 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -986,10 +986,69 @@ static bool has_vbmeta_partition() { fb->GetVar("partition-type:vbmeta_b", &partition_type) == fastboot::SUCCESS; } +static std::string fb_fix_numeric_var(std::string var) { + // Some bootloaders (angler, for example), send spurious leading whitespace. + var = android::base::Trim(var); + // Some bootloaders (hammerhead, for example) use implicit hex. + // This code used to use strtol with base 16. + if (!android::base::StartsWith(var, "0x")) var = "0x" + var; + return var; +} + +static void copy_boot_avb_footer(const std::string& partition, struct fastboot_buffer* buf) { + if (buf->sz < AVB_FOOTER_SIZE) { + return; + } + + std::string partition_size_str; + if (fb->GetVar("partition-size:" + partition, &partition_size_str) != fastboot::SUCCESS) { + die("cannot get boot partition size"); + } + + partition_size_str = fb_fix_numeric_var(partition_size_str); + int64_t partition_size; + if (!android::base::ParseInt(partition_size_str, &partition_size)) { + die("Couldn't parse partition size '%s'.", partition_size_str.c_str()); + } + if (partition_size == buf->sz) { + return; + } + if (partition_size < buf->sz) { + die("boot partition is smaller than boot image"); + } + + std::string data; + if (!android::base::ReadFdToString(buf->fd, &data)) { + die("Failed reading from boot"); + } + + uint64_t footer_offset = buf->sz - AVB_FOOTER_SIZE; + if (0 != data.compare(footer_offset, AVB_FOOTER_MAGIC_LEN, AVB_FOOTER_MAGIC)) { + return; + } + + int fd = make_temporary_fd("boot rewriting"); + if (!android::base::WriteStringToFd(data, fd)) { + die("Failed writing to modified boot"); + } + lseek(fd, partition_size - AVB_FOOTER_SIZE, SEEK_SET); + if (!android::base::WriteStringToFd(data.substr(footer_offset), fd)) { + die("Failed copying AVB footer in boot"); + } + close(buf->fd); + buf->fd = fd; + buf->sz = partition_size; + lseek(fd, 0, SEEK_SET); +} + static void flash_buf(const std::string& partition, struct fastboot_buffer *buf) { sparse_file** s; + if (partition == "boot" || partition == "boot_a" || partition == "boot_b") { + copy_boot_avb_footer(partition, buf); + } + // Rewrite vbmeta if that's what we're flashing and modification has been requested. if (g_disable_verity || g_disable_verification) { if (partition == "vbmeta" || partition == "vbmeta_a" || partition == "vbmeta_b") { @@ -1485,15 +1544,6 @@ static void do_oem_command(const std::string& cmd, std::vector* arg fb->RawCommand(command, ""); } -static std::string fb_fix_numeric_var(std::string var) { - // Some bootloaders (angler, for example), send spurious leading whitespace. - var = android::base::Trim(var); - // Some bootloaders (hammerhead, for example) use implicit hex. - // This code used to use strtol with base 16. - if (!android::base::StartsWith(var, "0x")) var = "0x" + var; - return var; -} - static unsigned fb_get_flash_block_size(std::string name) { std::string sizeString; if (fb->GetVar(name, &sizeString) != fastboot::SUCCESS || sizeString.empty()) { From cf7b6bad55b3c41d8289852488eb4496a4524ba5 Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Mon, 22 Jun 2020 17:47:23 +0100 Subject: [PATCH 30/43] Explicitly call restorecon_recursive on /metadata/apex On some devices we see a weird in which /metadata/apex will have a wrong selinux label. This will effectively prevent such devices from getting any apex updates. Since we haven't figured out a root cause for this bug, it's safer to explicitly call restorecon on /metadata/apex to make sure it's correct. This change shouldn't affect a normal boot flow, since /metadata/apex will already have a correct label and restorecon_recursive will be a no-op. Test: rm -Rf /metadata/apex && \ mkdir /metadata/apex && mkdir /metadata/apex/sessions Bug: 149317789 Change-Id: I971ffe35c93bb79d9e71106c24515ec0ee70333a --- rootdir/init.rc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rootdir/init.rc b/rootdir/init.rc index 73ac7fd0d..e7ba1f3c3 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -517,6 +517,12 @@ on post-fs mkdir /metadata/apex 0700 root system mkdir /metadata/apex/sessions 0700 root system + # On some devices we see a weird behaviour in which /metadata/apex doesn't + # have a correct label. To workaround this bug, explicitly call restorecon + # on /metadata/apex. For most of the boot sequences /metadata/apex will + # already have a correct selinux label, meaning that this call will be a + # no-op. + restorecon_recursive /metadata/apex mkdir /metadata/staged-install 0770 root system on late-fs From 8b637c8824aff515476cc29a48c8c30cc1307ed3 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 22 Jun 2020 10:15:04 -0700 Subject: [PATCH 31/43] logd: make drop_privs work if neither klogd or auditd are used Fix a bug that was causing cap_set_flag() fail and logd to exit. Bug: 159588327 Test: caps are set correctly and logd functions with both, one of, or none of klogd and auditd enabled. Merged-In: Ia51f078ad848535ce1ac29edd8a56a2b686a12cc Change-Id: Ia51f078ad848535ce1ac29edd8a56a2b686a12cc (cherry picked from commit 54b00a9e3cb61e090c5ff9403eedd6add55b48b1) --- logd/main.cpp | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/logd/main.cpp b/logd/main.cpp index 23bbf864d..acf9a071f 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -62,14 +62,13 @@ // has a 'sigstop' feature that sends SIGSTOP to a service immediately before calling exec(). This // allows debuggers, etc to be attached to logd at the very beginning, while still having init // handle the user, groups, capabilities, files, etc setup. -static int drop_privs(bool klogd, bool auditd) { - sched_param param = {}; - +static int DropPrivs(bool klogd, bool auditd) { if (set_sched_policy(0, SP_BACKGROUND) < 0) { android::prdebug("failed to set background scheduling policy"); return -1; } + sched_param param = {}; if (sched_setscheduler((pid_t)0, SCHED_BATCH, ¶m) < 0) { android::prdebug("failed to set batch scheduler"); return -1; @@ -84,21 +83,24 @@ static int drop_privs(bool klogd, bool auditd) { std::unique_ptr caps(cap_init(), cap_free); if (cap_clear(caps.get()) < 0) { + android::prdebug("cap_clear() failed"); return -1; } - std::vector cap_value; if (klogd) { - cap_value.emplace_back(CAP_SYSLOG); + cap_value_t cap_syslog = CAP_SYSLOG; + if (cap_set_flag(caps.get(), CAP_PERMITTED, 1, &cap_syslog, CAP_SET) < 0 || + cap_set_flag(caps.get(), CAP_EFFECTIVE, 1, &cap_syslog, CAP_SET) < 0) { + android::prdebug("Failed to set CAP_SYSLOG"); + return -1; + } } if (auditd) { - cap_value.emplace_back(CAP_AUDIT_CONTROL); - } - - if (cap_set_flag(caps.get(), CAP_PERMITTED, cap_value.size(), cap_value.data(), CAP_SET) < 0) { - return -1; - } - if (cap_set_flag(caps.get(), CAP_EFFECTIVE, cap_value.size(), cap_value.data(), CAP_SET) < 0) { - return -1; + cap_value_t cap_audit_control = CAP_AUDIT_CONTROL; + if (cap_set_flag(caps.get(), CAP_PERMITTED, 1, &cap_audit_control, CAP_SET) < 0 || + cap_set_flag(caps.get(), CAP_EFFECTIVE, 1, &cap_audit_control, CAP_SET) < 0) { + android::prdebug("Failed to set CAP_AUDIT_CONTROL"); + return -1; + } } if (cap_set_proc(caps.get()) < 0) { android::prdebug("failed to set CAP_SYSLOG or CAP_AUDIT_CONTROL (%d)", errno); @@ -332,7 +334,7 @@ int main(int argc, char* argv[]) { } bool auditd = __android_logger_property_get_bool("ro.logd.auditd", BOOL_DEFAULT_TRUE); - if (drop_privs(klogd, auditd) != 0) { + if (DropPrivs(klogd, auditd) != 0) { return EXIT_FAILURE; } From c1ca9f75f9da7a07fec140a669a05513648af871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 22 Jun 2020 01:11:24 -0700 Subject: [PATCH 32/43] add a new trigger for launching the bpfloader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: built and booted Bug: 150040815 Signed-off-by: Maciej Żenczykowski Merged-In: If80758b3d7bf499d428880efa5ed555076bfc291 Change-Id: If80758b3d7bf499d428880efa5ed555076bfc291 --- rootdir/init.rc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rootdir/init.rc b/rootdir/init.rc index 73ac7fd0d..7c90f8302 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -448,6 +448,9 @@ on late-init # Load persist properties and override properties (if enabled) from /data. trigger load_persist_props_action + # Should be before netd, but after apex, properties and logging is available. + trigger load_bpf_programs + # Now we can start zygote for devices with file based encryption trigger zygote-start From 6423ccd96be44d148466787f7419bc078b08c8b5 Mon Sep 17 00:00:00 2001 From: Keun young Park Date: Tue, 23 Jun 2020 20:32:54 -0700 Subject: [PATCH 33/43] Start statsd before starting zygote - zygote needs statsd for logging boot time event. - statsd starting later leads into stats logging failure which is reported by all child processes of zygote later. This brings lots of noise in statsd error. Bug: 159664734 Test: reboot and check if error log does not show up E statsd : Found dropped events: 1 error -19 last atom tag 240 from uid 10169 Change-Id: Ie585febb50a9668671c8fda41a872595baae8385 --- rootdir/init.rc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rootdir/init.rc b/rootdir/init.rc index e7ba1f3c3..13e890bb2 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -821,6 +821,7 @@ on zygote-start && property:persist.sys.fuse="" on zygote-start && property:ro.crypto.state=unencrypted # A/B update verifier that marks a successful boot. exec_start update_verifier_nonencrypted + start statsd start netd start zygote start zygote_secondary @@ -828,6 +829,7 @@ on zygote-start && property:ro.crypto.state=unencrypted on zygote-start && property:ro.crypto.state=unsupported # A/B update verifier that marks a successful boot. exec_start update_verifier_nonencrypted + start statsd start netd start zygote start zygote_secondary @@ -835,6 +837,7 @@ on zygote-start && property:ro.crypto.state=unsupported on zygote-start && property:ro.crypto.state=encrypted && property:ro.crypto.type=file # A/B update verifier that marks a successful boot. exec_start update_verifier_nonencrypted + start statsd start netd start zygote start zygote_secondary From a5c24c323d63a289cb55aeae447f546abc8bd2aa Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 24 Jun 2020 16:25:49 -0700 Subject: [PATCH 34/43] adbd: check auth id. When multiple auth requests come in (e.g. if someone connects over TCP and USB, or if we send a USB request, and then kill adb and try again), we need to disambiguate. Bug: http://b/159061108 Test: manual Change-Id: I0768dc3c1830456cb8cbd4395c23ef8f193cc420 --- adb/daemon/adb_wifi.cpp | 3 ++- adb/daemon/auth.cpp | 32 +++++++++++++++++++++++++------- adb/transport.h | 3 ++- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/adb/daemon/adb_wifi.cpp b/adb/daemon/adb_wifi.cpp index bce303b2a..2f9e9b4d0 100644 --- a/adb/daemon/adb_wifi.cpp +++ b/adb/daemon/adb_wifi.cpp @@ -42,7 +42,8 @@ static struct adisconnect adb_disconnect = {adb_disconnected, nullptr}; static void adb_disconnected(void* unused, atransport* t) { LOG(INFO) << "ADB wifi device disconnected"; - adbd_auth_tls_device_disconnected(auth_ctx, kAdbTransportTypeWifi, t->auth_id); + CHECK(t->auth_id.has_value()); + adbd_auth_tls_device_disconnected(auth_ctx, kAdbTransportTypeWifi, t->auth_id.value()); } // TODO(b/31559095): need bionic host so that we can use 'prop_info' returned diff --git a/adb/daemon/auth.cpp b/adb/daemon/auth.cpp index 2edf582d0..1a1e4ad62 100644 --- a/adb/daemon/auth.cpp +++ b/adb/daemon/auth.cpp @@ -207,15 +207,27 @@ void adbd_cloexec_auth_socket() { } static void adbd_auth_key_authorized(void* arg, uint64_t id) { - LOG(INFO) << "adb client authorized"; + LOG(INFO) << "adb client " << id << " authorized"; fdevent_run_on_main_thread([=]() { - LOG(INFO) << "arg = " << reinterpret_cast(arg); auto* transport = transport_from_callback_arg(arg); if (!transport) { - LOG(ERROR) << "authorization received for deleted transport, ignoring"; + LOG(ERROR) << "authorization received for deleted transport (" << id << "), ignoring"; return; } - transport->auth_id = id; + + if (transport->auth_id.has_value()) { + if (transport->auth_id.value() != id) { + LOG(ERROR) + << "authorization received, but auth id doesn't match, ignoring (expected " + << transport->auth_id.value() << ", got " << id << ")"; + return; + } + } else { + // Older versions (i.e. dogfood/beta builds) of libadbd_auth didn't pass the initial + // auth id to us, so we'll just have to trust it until R ships and we can retcon this. + transport->auth_id = id; + } + adbd_auth_verified(transport); }); } @@ -265,14 +277,20 @@ void adbd_auth_verified(atransport* t) { static void adb_disconnected(void* unused, atransport* t) { LOG(INFO) << "ADB disconnect"; - adbd_auth_notify_disconnect(auth_ctx, t->auth_id); + CHECK(t->auth_id.has_value()); + adbd_auth_notify_disconnect(auth_ctx, t->auth_id.value()); } void adbd_auth_confirm_key(atransport* t) { LOG(INFO) << "prompting user to authorize key"; t->AddDisconnect(&adb_disconnect); - adbd_auth_prompt_user(auth_ctx, t->auth_key.data(), t->auth_key.size(), - transport_to_callback_arg(t)); + if (adbd_auth_prompt_user_with_id) { + t->auth_id = adbd_auth_prompt_user_with_id(auth_ctx, t->auth_key.data(), t->auth_key.size(), + transport_to_callback_arg(t)); + } else { + adbd_auth_prompt_user(auth_ctx, t->auth_key.data(), t->auth_key.size(), + transport_to_callback_arg(t)); + } } void adbd_notify_framework_connected_key(atransport* t) { diff --git a/adb/transport.h b/adb/transport.h index 5bc1b5c49..26d804b3f 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -306,7 +307,7 @@ class atransport : public enable_weak_from_this { #if !ADB_HOST // Used to provide the key to the framework. std::string auth_key; - uint64_t auth_id; + std::optional auth_id; #endif bool IsTcpDevice() const { return type == kTransportLocal; } From d4bc3f2adb882c4d7858653ab38f6f5a0baf5171 Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Wed, 1 Jul 2020 12:34:17 -0700 Subject: [PATCH 35/43] libprocessgroup: Allow vendor profile attributes to override system ones In the current implementation vendor profile attributes do not override system ones and instead generate a warning. Fix that by overriding existing attribute if a new definition is found. Bug: 160318642 Test: add vendor attributes and confirm no warnings Signed-off-by: Suren Baghdasaryan Merged-In: I71a2ee4d4b3c585e7c9a01b791e973390d409cbc Change-Id: I71a2ee4d4b3c585e7c9a01b791e973390d409cbc --- libprocessgroup/task_profiles.cpp | 16 +++++++++++----- libprocessgroup/task_profiles.h | 1 + 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp index 4af4589da..a638fcac6 100644 --- a/libprocessgroup/task_profiles.cpp +++ b/libprocessgroup/task_profiles.cpp @@ -44,6 +44,11 @@ using android::base::WriteStringToFile; #define TASK_PROFILE_DB_FILE "/etc/task_profiles.json" #define TASK_PROFILE_DB_VENDOR_FILE "/vendor/etc/task_profiles.json" +void ProfileAttribute::Reset(const CgroupController& controller, const std::string& file_name) { + controller_ = controller; + file_name_ = file_name; +} + bool ProfileAttribute::GetPathForTask(int tid, std::string* path) const { std::string subgroup; if (!controller()->GetTaskGroup(tid, &subgroup)) { @@ -380,15 +385,16 @@ bool TaskProfiles::Load(const CgroupMap& cg_map, const std::string& file_name) { std::string controller_name = attr[i]["Controller"].asString(); std::string file_attr = attr[i]["File"].asString(); - if (attributes_.find(name) == attributes_.end()) { - auto controller = cg_map.FindController(controller_name); - if (controller.HasValue()) { + auto controller = cg_map.FindController(controller_name); + if (controller.HasValue()) { + auto iter = attributes_.find(name); + if (iter == attributes_.end()) { attributes_[name] = std::make_unique(controller, file_attr); } else { - LOG(WARNING) << "Controller " << controller_name << " is not found"; + iter->second->Reset(controller, file_attr); } } else { - LOG(WARNING) << "Attribute " << name << " is already defined"; + LOG(WARNING) << "Controller " << controller_name << " is not found"; } } diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h index 28bc00c10..2983a09b3 100644 --- a/libprocessgroup/task_profiles.h +++ b/libprocessgroup/task_profiles.h @@ -33,6 +33,7 @@ class ProfileAttribute { const CgroupController* controller() const { return &controller_; } const std::string& file_name() const { return file_name_; } + void Reset(const CgroupController& controller, const std::string& file_name); bool GetPathForTask(int tid, std::string* path) const; From 9285b496bd4ec2c50694042609576b72c8a16a0b Mon Sep 17 00:00:00 2001 From: Boleyn Su Date: Fri, 26 Jun 2020 18:08:07 +0900 Subject: [PATCH 36/43] ReadFileSymbolicLink checks /system/bin/ps According to https://android.googlesource.com/platform/build/+/refs/heads/android10-dev/core/Makefile#140, /default.prop may not be a symbolic link. Bug: 158552949 Test: atest CtsInitTestCases Change-Id: I19bde577fa50280e1ed6fb8fdbe846655abb930b Merged-In: I0d3f96c1656dfe02bfa0e801680f7fa887afd1d9 (cherry picked from commit 6f47feaccdbe849c982e0de9068e38be85006bfb) (cherry picked from commit 755a3dd78d913f812e5eddb9980557fa83678b3a) --- init/util_test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/init/util_test.cpp b/init/util_test.cpp index 96a5b5511..08343dcb5 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -61,8 +61,9 @@ TEST(util, ReadFileWorldWiteable) { TEST(util, ReadFileSymbolicLink) { errno = 0; - // lrw------- 1 root root 23 2008-12-31 19:00 default.prop -> system/etc/prop.default - auto file_contents = ReadFile("/default.prop"); + // lrwxrwxrwx 1 root shell 6 2020-06-26 09:55 /system/bin/ps -> toybox + auto file_contents = ReadFile("/system/bin/ps"); + EXPECT_EQ(ELOOP, errno); ASSERT_FALSE(file_contents.ok()); EXPECT_EQ("open() failed: Too many symbolic links encountered", From 0a0f4b163a5fba68a656fd55cfaa6bca6ce69436 Mon Sep 17 00:00:00 2001 From: Hung-ying Tyan Date: Fri, 3 Jul 2020 14:26:59 +0800 Subject: [PATCH 37/43] Remove SkipMountingPartitions from vendor libfstab SkipMountingPartitions() should only be called from a system process as the config file is in /system_ext. Remove it from the vendor variant of libfstab. Bug: 158301941 Test: build hardware/interfaces/boot/1.1/default/boot_control and check symbol Change-Id: I834183a623c0711dd79b89486fc3fc80f9710801 Merged-In: I834183a623c0711dd79b89486fc3fc80f9710801 (cherry picked from e7cb09d2264a9071425a45bd76d4e6be45607d5f) --- fs_mgr/Android.bp | 8 ++++++++ fs_mgr/fs_mgr_fstab.cpp | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/fs_mgr/Android.bp b/fs_mgr/Android.bp index cd6459989..ac784b249 100644 --- a/fs_mgr/Android.bp +++ b/fs_mgr/Android.bp @@ -149,6 +149,14 @@ cc_library_static { darwin: { enabled: false, }, + vendor: { + cflags: [ + // Skipping entries in fstab should only be done in a system + // process as the config file is in /system_ext. + // Remove the op from the vendor variant. + "-DNO_SKIP_MOUNT", + ], + }, }, export_include_dirs: ["include_fstab"], header_libs: [ diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index fa4ac3904..8a2207870 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -690,7 +690,9 @@ bool ReadFstabFromFile(const std::string& path, Fstab* fstab) { TransformFstabForDsu(fstab, Split(lp_names, ",")); } +#ifndef NO_SKIP_MOUNT SkipMountingPartitions(fstab); +#endif EnableMandatoryFlags(fstab); return true; @@ -720,11 +722,14 @@ bool ReadFstabFromDt(Fstab* fstab, bool log) { return false; } +#ifndef NO_SKIP_MOUNT SkipMountingPartitions(fstab); +#endif return true; } +#ifndef NO_SKIP_MOUNT // For GSI to skip mounting /product and /system_ext, until there are well-defined interfaces // between them and /system. Otherwise, the GSI flashed on /system might not be able to work with // device-specific /product and /system_ext. skip_mount.cfg belongs to system_ext partition because @@ -756,6 +761,7 @@ bool SkipMountingPartitions(Fstab* fstab) { return true; } +#endif // Loads the fstab file and combines with fstab entries passed in from device tree. bool ReadDefaultFstab(Fstab* fstab) { From ba5dfd76ded0502660e4cf695a4c249516adecdf Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 1 Jul 2020 12:32:46 -0700 Subject: [PATCH 38/43] liblp: Force 10.0 metadata on downgrade to Q. Q liblp only supports 10.0 super partition metadata, so forcefully downgrade the current metadata version too. On retrofit Virtual A/B devices, the metadata version is at most 10.1, because the new VIRTUAL_AB flag is not set on retrofit devices. In version 10.1, two per-partition flags: UPDATED and DISABLED are introduced. - The updated flag is set when the device undergoes a Virtual A/B update before. Clear it. - The disabled flag should only be set on metadata files used by libfiemap ImageManager. It shouldn't be used on super partition metadata. Hence, this CL should only clear UPDATED flag. Test: R->R->Q OTA Bug: 159590481 Change-Id: I8b548c8ce36a75197e7172a77f9207fd44fe4670 --- fs_mgr/liblp/builder.cpp | 17 ++++++++---- fs_mgr/liblp/include/liblp/builder.h | 6 +++-- fs_mgr/liblp/utility.cpp | 39 ++++++++++++++++++++++++++++ fs_mgr/liblp/utility.h | 4 +++ 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index 2f516fa9b..5554f7794 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -204,11 +204,18 @@ std::unique_ptr MetadataBuilder::NewForUpdate(const IPartitionO } } - if (IPropertyFetcher::GetInstance()->GetBoolProperty("ro.virtual_ab.enabled", false) && - !always_keep_source_slot) { - if (!UpdateMetadataForInPlaceSnapshot(metadata.get(), source_slot_number, - target_slot_number)) { - return nullptr; + if (IPropertyFetcher::GetInstance()->GetBoolProperty("ro.virtual_ab.enabled", false)) { + if (always_keep_source_slot) { + // always_keep_source_slot implies the target build does not support snapshots. + // Clear unsupported attributes. + SetMetadataHeaderV0(metadata.get()); + } else { + // !always_keep_source_slot implies the target build supports snapshots. Do snapshot + // updates. + if (!UpdateMetadataForInPlaceSnapshot(metadata.get(), source_slot_number, + target_slot_number)) { + return nullptr; + } } } diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h index bd3915061..6c7a70728 100644 --- a/fs_mgr/liblp/include/liblp/builder.h +++ b/fs_mgr/liblp/include/liblp/builder.h @@ -209,8 +209,10 @@ class MetadataBuilder { // metadata may not have the target slot's devices listed yet, in which // case, it is automatically upgraded to include all available block // devices. - // If |always_keep_source_slot| is set, on a Virtual A/B device, source slot - // partitions are kept. This is useful when applying a downgrade package. + // If |always_keep_source_slot| is set, on a Virtual A/B device + // - source slot partitions are kept. + // - UPDATED flag is cleared. + // This is useful when applying a downgrade package. static std::unique_ptr NewForUpdate(const IPartitionOpener& opener, const std::string& source_partition, uint32_t source_slot_number, diff --git a/fs_mgr/liblp/utility.cpp b/fs_mgr/liblp/utility.cpp index 48c5c8318..d8e171b10 100644 --- a/fs_mgr/liblp/utility.cpp +++ b/fs_mgr/liblp/utility.cpp @@ -15,6 +15,7 @@ */ #include +#include #include #include #include @@ -29,6 +30,7 @@ #include #include +#include #include #include @@ -285,5 +287,42 @@ bool UpdateMetadataForInPlaceSnapshot(LpMetadata* metadata, uint32_t source_slot return true; } +inline std::string ToHexString(uint64_t value) { + return android::base::StringPrintf("0x%" PRIx64, value); +} + +void SetMetadataHeaderV0(LpMetadata* metadata) { + if (metadata->header.minor_version <= LP_METADATA_MINOR_VERSION_MIN) { + return; + } + LINFO << "Forcefully setting metadata header version " << LP_METADATA_MAJOR_VERSION << "." + << metadata->header.minor_version << " to " << LP_METADATA_MAJOR_VERSION << "." + << LP_METADATA_MINOR_VERSION_MIN; + metadata->header.minor_version = LP_METADATA_MINOR_VERSION_MIN; + metadata->header.header_size = sizeof(LpMetadataHeaderV1_0); + + // Retrofit Virtual A/B devices should have version 10.1, so flags shouldn't be set. + // Warn if this is the case, but zero it out anyways. + if (metadata->header.flags) { + LWARN << "Zeroing unexpected flags: " << ToHexString(metadata->header.flags); + } + + // Zero out all fields beyond LpMetadataHeaderV0. + static_assert(sizeof(metadata->header) > sizeof(LpMetadataHeaderV1_0)); + memset(reinterpret_cast(&metadata->header) + sizeof(LpMetadataHeaderV1_0), 0, + sizeof(metadata->header) - sizeof(LpMetadataHeaderV1_0)); + + // Clear partition attributes unknown to V0. + // On retrofit Virtual A/B devices, UPDATED flag may be set, so only log info here. + for (auto& partition : metadata->partitions) { + if (partition.attributes & ~LP_PARTITION_ATTRIBUTE_MASK_V0) { + LINFO << "Clearing " << GetPartitionName(partition) + << " partition attribute: " << ToHexString(partition.attributes); + } + + partition.attributes &= LP_PARTITION_ATTRIBUTE_MASK_V0; + } +} + } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/liblp/utility.h b/fs_mgr/liblp/utility.h index 06617696c..1d86edd4e 100644 --- a/fs_mgr/liblp/utility.h +++ b/fs_mgr/liblp/utility.h @@ -103,6 +103,10 @@ bool SetBlockReadonly(int fd, bool readonly); bool UpdateMetadataForInPlaceSnapshot(LpMetadata* metadata, uint32_t source_slot_number, uint32_t target_slot_number); +// Forcefully set metadata header version to 1.0, clearing any incompatible flags and attributes +// so that when downgrading to a build with liblp V0, the device still boots. +void SetMetadataHeaderV0(LpMetadata* metadata); + } // namespace fs_mgr } // namespace android From 44033e75cc8857586f6eb8d34466d4a9224fe832 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Tue, 7 Jul 2020 13:23:07 -0700 Subject: [PATCH 39/43] Try locking after mounting metadata In rescue mode, if /metadata is mounted but /metadata/ota does not exist, immediately unmount /metadata and fallback to the code path when /metadata is not mounted; that is, old partitions are overwritten. Test: in recovery, select wipe then immediately sideload Bug: 160457903 Change-Id: I412d62b7005c81a7126106edc471622e6a7ef813 --- fs_mgr/libsnapshot/snapshot.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 0739fab74..a287c353c 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -2516,7 +2516,19 @@ std::unique_ptr SnapshotManager::EnsureMetadataMounted() { LOG(INFO) << "EnsureMetadataMounted does nothing in Android mode."; return std::unique_ptr(new AutoUnmountDevice()); } - return AutoUnmountDevice::New(device_->GetMetadataDir()); + auto ret = AutoUnmountDevice::New(device_->GetMetadataDir()); + if (ret == nullptr) return nullptr; + + // In rescue mode, it is possible to erase and format metadata, but /metadata/ota is not + // created to execute snapshot updates. Hence, subsequent calls is likely to fail because + // Lock*() fails. By failing early and returning nullptr here, update_engine_sideload can + // treat this case as if /metadata is not mounted. + if (!LockShared()) { + LOG(WARNING) << "/metadata is mounted, but errors occur when acquiring a shared lock. " + "Subsequent calls to SnapshotManager will fail. Unmounting /metadata now."; + return nullptr; + } + return ret; } bool SnapshotManager::HandleImminentDataWipe(const std::function& callback) { From bd6d2cded8a5f7e3f602ed7574e80954a8b9104d Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Tue, 7 Jul 2020 14:28:41 -0700 Subject: [PATCH 40/43] Wrap flock with TEMP_FAILURE_RETRY. flock may return EINTR. There are code using LockShared() to test existance of the directory. Don't fail spuriously. Test: pass Bug: 160457903 Change-Id: I51628abe05599422eb3f344781d8f3acd653c822 --- fs_mgr/libsnapshot/snapshot.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index a287c353c..4178349ed 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1846,7 +1846,7 @@ auto SnapshotManager::OpenFile(const std::string& file, int lock_flags) PLOG(ERROR) << "Open failed: " << file; return nullptr; } - if (lock_flags != 0 && flock(fd, lock_flags) < 0) { + if (lock_flags != 0 && TEMP_FAILURE_RETRY(flock(fd, lock_flags)) < 0) { PLOG(ERROR) << "Acquire flock failed: " << file; return nullptr; } @@ -1857,7 +1857,7 @@ auto SnapshotManager::OpenFile(const std::string& file, int lock_flags) } SnapshotManager::LockedFile::~LockedFile() { - if (flock(fd_, LOCK_UN) < 0) { + if (TEMP_FAILURE_RETRY(flock(fd_, LOCK_UN)) < 0) { PLOG(ERROR) << "Failed to unlock file: " << path_; } } From 5c8457ef9a150d9ae62d18cfab2725e1fac50d5c Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 9 Jul 2020 08:47:24 -0700 Subject: [PATCH 41/43] init: skip RejectsCriticalAndOneshotService for devices launched before R This restriction is only added for devices launching with R or later. Bug: 160663765 Test: this test runs when appropriate Merged-In: I2353bfa7f598bd19ba57498cc5bbad7a3ed34707 Change-Id: I2353bfa7f598bd19ba57498cc5bbad7a3ed34707 (cherry picked from commit 0e40ba3183b684fb6b3e62d542125a5a7440b98a) --- init/init_test.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/init/init_test.cpp b/init/init_test.cpp index 07b472455..fa6574026 100644 --- a/init/init_test.cpp +++ b/init/init_test.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include "action.h" @@ -32,6 +33,8 @@ #include "service_parser.h" #include "util.h" +using android::base::GetIntProperty; + namespace android { namespace init { @@ -240,6 +243,10 @@ TEST(init, EventTriggerOrderMultipleFiles) { } TEST(init, RejectsCriticalAndOneshotService) { + if (GetIntProperty("ro.product.first_api_level", 10000) < 30) { + GTEST_SKIP() << "Test only valid for devices launching with R or later"; + } + std::string init_script = R"init( service A something From fb60e6c9aed973759e1fbd66a1dfbfc5b7cdaef6 Mon Sep 17 00:00:00 2001 From: Victor Khimenko Date: Thu, 18 Jun 2020 22:01:13 +0200 Subject: [PATCH 42/43] Make libbacktrace buildable for native_bridge Bug: http://b/153609531 Test: m -j64 libbacktrace.native_bridge Change-Id: I2b8a881b4e952f3b68dbcaeb14f147a6d955b406 Merged-In: I2b8a881b4e952f3b68dbcaeb14f147a6d955b406 --- libbacktrace/Android.bp | 5 +++++ libprocinfo/Android.bp | 2 ++ libunwindstack/Android.bp | 7 +++++++ libutils/Android.bp | 2 ++ 4 files changed, 16 insertions(+) diff --git a/libbacktrace/Android.bp b/libbacktrace/Android.bp index c4cfa6fbe..c9e45124a 100644 --- a/libbacktrace/Android.bp +++ b/libbacktrace/Android.bp @@ -96,6 +96,8 @@ cc_defaults { cc_library { name: "libbacktrace", vendor_available: false, + // TODO(b/153609531): remove when no longer needed. + native_bridge_supported: true, recovery_available: true, apex_available: [ "//apex_available:platform", @@ -120,6 +122,9 @@ cc_library { recovery: { cflags: ["-DNO_LIBDEXFILE_SUPPORT"], }, + native_bridge: { + cflags: ["-DNO_LIBDEXFILE_SUPPORT"], + }, }, } diff --git a/libprocinfo/Android.bp b/libprocinfo/Android.bp index 15b0d89d8..ae4574276 100644 --- a/libprocinfo/Android.bp +++ b/libprocinfo/Android.bp @@ -27,6 +27,8 @@ cc_library { name: "libprocinfo", defaults: ["libprocinfo_defaults"], vendor_available: true, + // TODO(b/153609531): remove when no longer needed. + native_bridge_supported: true, recovery_available: true, vndk: { enabled: true, diff --git a/libunwindstack/Android.bp b/libunwindstack/Android.bp index 563c2c296..9b974c237 100644 --- a/libunwindstack/Android.bp +++ b/libunwindstack/Android.bp @@ -113,6 +113,8 @@ cc_library { name: "libunwindstack", vendor_available: true, recovery_available: true, + // TODO(b/153609531): remove when no longer needed. + native_bridge_supported: true, vndk: { enabled: true, support_system_process: true, @@ -134,6 +136,11 @@ cc_library { exclude_srcs: ["DexFile.cpp"], exclude_shared_libs: ["libdexfile_support"], }, + native_bridge: { + cflags: ["-UDEXFILE_SUPPORT"], + exclude_srcs: ["DexFile.cpp"], + exclude_shared_libs: ["libdexfile_support"], + }, }, apex_available: [ diff --git a/libutils/Android.bp b/libutils/Android.bp index 3a30a9e77..85f428067 100644 --- a/libutils/Android.bp +++ b/libutils/Android.bp @@ -175,6 +175,8 @@ cc_library { cc_library { name: "libutilscallstack", defaults: ["libutils_defaults"], + // TODO(b/153609531): remove when no longer needed. + native_bridge_supported: true, srcs: [ "CallStack.cpp", From c8766043aa294fce85b4a26ef029737184cb2c0a Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Sun, 12 Jul 2020 20:28:07 -0700 Subject: [PATCH 43/43] extend syscall minijail for clang code coverage cutover to clang-based code coverage uses extra system call (ftruncate) when writing coverage data. exposed while generating coverage for media.extractor. Bug: 160917521 Test: build, boot, kill -37 extractors for {arm,x86}x{32,64} Change-Id: I555b168c7aab43caa590df951861b92e8fa14bc3 --- code_coverage/seccomp_policy/code_coverage.arm.policy | 1 + code_coverage/seccomp_policy/code_coverage.arm64.policy | 1 + code_coverage/seccomp_policy/code_coverage.policy.def | 2 ++ code_coverage/seccomp_policy/code_coverage.x86.policy | 1 + code_coverage/seccomp_policy/code_coverage.x86_64.policy | 1 + 5 files changed, 6 insertions(+) diff --git a/code_coverage/seccomp_policy/code_coverage.arm.policy b/code_coverage/seccomp_policy/code_coverage.arm.policy index d6784e371..b80910f19 100644 --- a/code_coverage/seccomp_policy/code_coverage.arm.policy +++ b/code_coverage/seccomp_policy/code_coverage.arm.policy @@ -6,6 +6,7 @@ openat: 1 write: 1 fcntl64: 1 fstat64: 1 +ftruncate64: 1 geteuid32: 1 _llseek: 1 mmap2: 1 diff --git a/code_coverage/seccomp_policy/code_coverage.arm64.policy b/code_coverage/seccomp_policy/code_coverage.arm64.policy index 4c3dd2664..7040ea258 100644 --- a/code_coverage/seccomp_policy/code_coverage.arm64.policy +++ b/code_coverage/seccomp_policy/code_coverage.arm64.policy @@ -6,6 +6,7 @@ openat: 1 write: 1 fcntl: 1 fstat: 1 +ftruncate: 1 geteuid: 1 lseek: 1 mmap: 1 diff --git a/code_coverage/seccomp_policy/code_coverage.policy.def b/code_coverage/seccomp_policy/code_coverage.policy.def index f136084bc..599c4a458 100644 --- a/code_coverage/seccomp_policy/code_coverage.policy.def +++ b/code_coverage/seccomp_policy/code_coverage.policy.def @@ -22,6 +22,7 @@ write: 1 #if defined(__LP64__) fcntl: 1 fstat: 1 +ftruncate: 1 geteuid: 1 lseek: 1 mmap: 1 @@ -29,6 +30,7 @@ rt_sigreturn: 1 #else fcntl64: 1 fstat64: 1 +ftruncate64: 1 geteuid32: 1 _llseek: 1 mmap2: 1 diff --git a/code_coverage/seccomp_policy/code_coverage.x86.policy b/code_coverage/seccomp_policy/code_coverage.x86.policy index 24ff8b9c0..f8e0cc0f4 100644 --- a/code_coverage/seccomp_policy/code_coverage.x86.policy +++ b/code_coverage/seccomp_policy/code_coverage.x86.policy @@ -6,6 +6,7 @@ openat: 1 write: 1 fcntl64: 1 fstat64: 1 +ftruncate64: 1 geteuid32: 1 _llseek: 1 mmap2: 1 diff --git a/code_coverage/seccomp_policy/code_coverage.x86_64.policy b/code_coverage/seccomp_policy/code_coverage.x86_64.policy index 308103654..dcf2f9a11 100644 --- a/code_coverage/seccomp_policy/code_coverage.x86_64.policy +++ b/code_coverage/seccomp_policy/code_coverage.x86_64.policy @@ -6,6 +6,7 @@ openat: 1 write: 1 fcntl: 1 fstat: 1 +ftruncate: 1 geteuid: 1 lseek: 1 mmap: 1