From 9ac82420cb501cc16e4431f3fc767877a9964bd0 Mon Sep 17 00:00:00 2001 From: Matt Gilbride Date: Tue, 20 Aug 2024 21:23:22 +0000 Subject: [PATCH 1/4] Support vendor partition in non-debuggable pVMs Use the existence of /proc/device-tree/avf/vendor_hashtree_descriptor_root_digest (rather than kernel param androidboot.microdroid.mount_vendor=1) to know if the vendor partition is requested. Bug: 340506965 Test: TH Change-Id: I0ac1c773e44454fd9c52559d833dc8eca211889c --- init/first_stage_mount.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index ece430b70..c26b31e93 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -156,6 +156,13 @@ static Result ReadFirstStageFstabAndroid() { return fstab; } +static bool IsRequestingMicrodroidVendorPartition(const std::string& cmdline) { + if (virtualization::IsEnableTpuAssignableDeviceFlagEnabled()) { + return access("/proc/device-tree/avf/vendor_hashtree_descriptor_root_digest", F_OK) == 0; + } + return cmdline.find("androidboot.microdroid.mount_vendor=1") != std::string::npos; +} + // Note: this is a temporary solution to avoid blocking devs that depend on /vendor partition in // Microdroid. For the proper solution the /vendor fstab should probably be defined in the DT. // TODO(b/285855430): refactor this @@ -166,7 +173,7 @@ static Result ReadFirstStageFstabMicrodroid(const std::string& cmdline) { if (!ReadDefaultFstab(&fstab)) { return Error() << "failed to read fstab"; } - if (cmdline.find("androidboot.microdroid.mount_vendor=1") == std::string::npos) { + if (!IsRequestingMicrodroidVendorPartition(cmdline)) { // We weren't asked to mount /vendor partition, filter it out from the fstab. auto predicate = [](const auto& entry) { return entry.mount_point == "/vendor"; }; fstab.erase(std::remove_if(fstab.begin(), fstab.end(), predicate), fstab.end()); From e60b760e7463bcf66b7c1da3dd6d70e2f19f30db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Wed, 18 Sep 2024 18:11:14 +0000 Subject: [PATCH 2/4] start netd earlier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this change we're moving the asynchronous netd startup ahead of the async statd and *synchronous* update_verifier. This is desirable as we want a netd failure (which could happen due to some mainline incompatibility wrt. bpf or mainline shipped shared libs: resolver or netd updatable) to be considered a signal for a bad boot. It's still asynchronous though, so it's not ideal. Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: Ib3e252f085f569864feddaf20ac80858a3bb969d --- init/README.md | 7 ++++--- rootdir/init.rc | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/init/README.md b/init/README.md index de57208e8..560c5280f 100644 --- a/init/README.md +++ b/init/README.md @@ -501,9 +501,10 @@ have been omitted. reformatted here if it couldn't mount in first-stage init. 6. `post-fs-data-checkpointed` - Triggered when vold has completed committing a checkpoint after an OTA update. Not triggered if checkpointing is not needed or supported. - 7. `zygote-start` - Start the zygote. - 8. `early-boot` - After zygote has started. - 9. `boot` - After `early-boot` actions have completed. + 7. `bpf-progs-loaded` - Starts things that want to start ASAP but need eBPF (incl. netd) + 8. `zygote-start` - Start the zygote. + 9. `early-boot` - After zygote has started. + 10. `boot` - After `early-boot` actions have completed. Commands -------- diff --git a/rootdir/init.rc b/rootdir/init.rc index 63e3d0646..41b94515b 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -560,6 +560,7 @@ on late-init # Should be before netd, but after apex, properties and logging is available. trigger load_bpf_programs + trigger bpf-progs-loaded # Now we can start zygote. trigger zygote-start @@ -1109,6 +1110,9 @@ on post-fs-data on property:vold.checkpoint_committed=1 trigger post-fs-data-checkpointed +on bpf-progs-loaded + start netd + # It is recommended to put unnecessary data/ initialization from post-fs-data # to start-zygote in device's init.rc to unblock zygote start. on zygote-start @@ -1116,7 +1120,6 @@ on zygote-start # A/B update verifier that marks a successful boot. exec_start update_verifier start statsd - start netd start zygote start zygote_secondary From b4b3950e528cc62e874e59d052b2e9a600de708a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Mon, 23 Sep 2024 21:24:00 +0000 Subject: [PATCH 3/4] Fix the trigger name for loading bpf programs. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trigger name should be load-bpf-programs, not load_bpf_programs. Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I00cff0a3dd971de39dfc3226b140be972854ea28 --- rootdir/init.rc | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/rootdir/init.rc b/rootdir/init.rc index d80416db8..1acd63774 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -567,7 +567,7 @@ on late-init trigger post-fs-data # Should be before netd, but after apex, properties and logging is available. - trigger load_bpf_programs + trigger load-bpf-programs trigger bpf-progs-loaded # Now we can start zygote. @@ -1110,6 +1110,19 @@ on post-fs-data on property:vold.checkpoint_committed=1 trigger post-fs-data-checkpointed +# It is important that we start bpfloader after: +# - /sys/fs/bpf is already mounted, +# - apex (incl. rollback) is initialized (so that we can load bpf +# programs shipped as part of apex mainline modules) +# - logd is ready for us to log stuff +# +# At the same time we want to be as early as possible to reduce races and thus +# failures (before memory is fragmented, and cpu is busy running tons of other +# stuff) and we absolutely want to be before netd and the system boot slot is +# considered to have booted successfully. +on load-bpf-programs + exec_start bpfloader + on bpf-progs-loaded start netd @@ -1280,7 +1293,7 @@ on property:net.tcp_def_init_rwnd=* # controlling access. On older kernels, the paranoid value is the only means of # controlling access. It is normally 3 (allow only root), but the shell user # can lower it to 1 (allowing thread-scoped pofiling) via security.perf_harden. -on load_bpf_programs && property:sys.init.perf_lsm_hooks=1 +on load-bpf-programs && property:sys.init.perf_lsm_hooks=1 write /proc/sys/kernel/perf_event_paranoid -1 on property:security.perf_harden=0 && property:sys.init.perf_lsm_hooks="" write /proc/sys/kernel/perf_event_paranoid 1 From d26f39ab0eacaf5318b814a1e4aa859660d52287 Mon Sep 17 00:00:00 2001 From: Armelle Laine Date: Thu, 19 Sep 2024 07:46:55 +0000 Subject: [PATCH 4/4] trusty: storage: proxy: FS_READY property setting on vendor only Bug: 367965796 Test: launch_cvd --noresume --console=true \ --extra_kernel_cmdline='androidboot.selinux=permissive' \ --secure_hals=guest_keymint_trusty_insecure Change-Id: I4d5ea1762f7cf9edfd8cbc00e2aec13caae965f4 --- trusty/storage/proxy/Android.bp | 7 ++++++- trusty/storage/proxy/storage.c | 26 ++++++++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/trusty/storage/proxy/Android.bp b/trusty/storage/proxy/Android.bp index e362b8b97..7ef0e6f83 100644 --- a/trusty/storage/proxy/Android.bp +++ b/trusty/storage/proxy/Android.bp @@ -47,7 +47,12 @@ cc_binary { "libtrustystorageinterface", "libtrusty", ], - + target: { + vendor: { + // vendor variant requires this flag + cflags: ["-DVENDOR_FS_READY_PROPERTY"], + }, + }, cflags: [ "-Wall", "-Werror", diff --git a/trusty/storage/proxy/storage.c b/trusty/storage/proxy/storage.c index ca39f6aab..72c4e93fe 100644 --- a/trusty/storage/proxy/storage.c +++ b/trusty/storage/proxy/storage.c @@ -54,6 +54,8 @@ static const char *ssdir_name; /* List head for storage mapping, elements added at init, and never removed */ static struct storage_mapping_node* storage_mapping_head; +#ifdef VENDOR_FS_READY_PROPERTY + /* * Properties set to 1 after we have opened a file under ssdir_name. The backing * files for both TD and TDP are currently located under /data/vendor/ss and can @@ -75,16 +77,6 @@ static struct storage_mapping_node* storage_mapping_head; static bool fs_ready_set = false; static bool fs_ready_rw_set = false; -static enum sync_state fs_state; -static enum sync_state fd_state[FD_TBL_SIZE]; - -static bool alternate_mode; - -static struct { - struct storage_file_read_resp hdr; - uint8_t data[MAX_READ_SIZE]; -} read_rsp; - static bool property_set_helper(const char* prop) { int rc = property_set(prop, "1"); if (rc == 0) { @@ -96,6 +88,18 @@ static bool property_set_helper(const char* prop) { return rc == 0; } +#endif // #ifdef VENDOR_FS_READY_PROPERTY + +static enum sync_state fs_state; +static enum sync_state fd_state[FD_TBL_SIZE]; + +static bool alternate_mode; + +static struct { + struct storage_file_read_resp hdr; + uint8_t data[MAX_READ_SIZE]; +} read_rsp; + static uint32_t insert_fd(int open_flags, int fd, struct storage_mapping_node* node) { uint32_t handle = fd; @@ -535,6 +539,7 @@ int storage_file_open(struct storage_msg* msg, const void* r, size_t req_len, free(path); path = NULL; +#ifdef VENDOR_FS_READY_PROPERTY /* a backing file has been opened, notify any waiting init steps */ if (!fs_ready_set || !fs_ready_rw_set) { bool is_checkpoint_active = false; @@ -552,6 +557,7 @@ int storage_file_open(struct storage_msg* msg, const void* r, size_t req_len, } } } +#endif // #ifdef VENDOR_FS_READY_PROPERTY return ipc_respond(msg, &resp, sizeof(resp));