From c6f19688f82f287a9ea2afdc33a822ad3395611c Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 26 Feb 2021 17:46:34 +0900 Subject: [PATCH] Detect the absence of the default fstab This is a follow-up of I828ce999be6d786bf46dd5655dfda81d046906ab. The change introduced a behavioral change that fstab is read twice: before root is changed to /first_stage_ramdisk, and once again after that. Previously, that happend only after the root is switched. That change caused a problem when there is no fstab in DT and fstab is provided via a file. The fstab file has been at /first_stage_ramdisk/fstab. because that file was supposed to be read after the root switch. With the change, init fails to read the fstab during the first attempt because there is no /fstab. at the moment. Here comes the problem. Although it failed to read fstab, DoCreateService() is invoked because ReadFirstStageFstab() doesn't report the failure; it returns an empty fstab object. As a result, DoCreateDevices() is called but it doesn't create the dm linear device because it couldn't find an fstab entry having `logical` option. Then after /first_stage_ramdisk becomes the root, the fstab file is correctly read. But since the prior run of DoCreateDevices() is recorded as 'done', init doesn't try to do that again; dm linear device is never created. Then we fail to mount any of the logical partitions. This change fixes the problem by modifying ReadFirstStageFstab() function so that the failure is correctly reported back to the caller. When it fails, DoCreateDevices() is not called. Bug: N/A Test: Watch TH Change-Id: Idf2dbc6c0fb6c311ab3f5ff1f28315f7daa2b4ce --- init/first_stage_mount.cpp | 46 +++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index de72f23c3..a11bb2837 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -44,6 +44,7 @@ #include "block_dev_initializer.h" #include "devices.h" +#include "result.h" #include "snapuserd_transition.h" #include "switch_root.h" #include "uevent.h" @@ -51,6 +52,7 @@ #include "util.h" using android::base::ReadFileToString; +using android::base::Result; using android::base::Split; using android::base::StringPrintf; using android::base::Timer; @@ -81,7 +83,7 @@ class FirstStageMount { // The factory method to create either FirstStageMountVBootV1 or FirstStageMountVBootV2 // based on device tree configurations. - static std::unique_ptr Create(); + static Result> Create(); bool DoCreateDevices(); // Creates devices and logical partitions from storage devices bool DoFirstStageMount(); // Mounts fstab entries read from device tree. bool InitDevices(); @@ -160,7 +162,7 @@ static inline bool IsDtVbmetaCompatible(const Fstab& fstab) { return is_android_dt_value_expected("vbmeta/compatible", "android,vbmeta"); } -static Fstab ReadFirstStageFstab() { +static Result ReadFirstStageFstab() { Fstab fstab; if (!ReadFstabFromDt(&fstab)) { if (ReadDefaultFstab(&fstab)) { @@ -170,7 +172,7 @@ static Fstab ReadFirstStageFstab() { }), fstab.end()); } else { - LOG(INFO) << "Failed to fstab for first stage mount"; + return Error() << "failed to read default fstab for first stage mount"; } } return fstab; @@ -236,12 +238,16 @@ FirstStageMount::FirstStageMount(Fstab fstab) : need_dm_verity_(false), fstab_(s super_partition_name_ = fs_mgr_get_super_partition_name(); } -std::unique_ptr FirstStageMount::Create() { +Result> FirstStageMount::Create() { auto fstab = ReadFirstStageFstab(); - if (IsDtVbmetaCompatible(fstab)) { - return std::make_unique(std::move(fstab)); + if (!fstab.ok()) { + return fstab.error(); + } + + if (IsDtVbmetaCompatible(*fstab)) { + return std::make_unique(std::move(*fstab)); } else { - return std::make_unique(std::move(fstab)); + return std::make_unique(std::move(*fstab)); } } @@ -836,12 +842,12 @@ bool FirstStageMountVBootV2::InitAvbHandle() { // ---------------- // Creates devices and logical partitions from storage devices bool DoCreateDevices() { - std::unique_ptr handle = FirstStageMount::Create(); - if (!handle) { - LOG(ERROR) << "Failed to create FirstStageMount"; + auto fsm = FirstStageMount::Create(); + if (!fsm.ok()) { + LOG(ERROR) << "Failed to create FirstStageMount: " << fsm.error(); return false; } - return handle->DoCreateDevices(); + return (*fsm)->DoCreateDevices(); } // Mounts partitions specified by fstab in device tree. @@ -852,17 +858,17 @@ bool DoFirstStageMount(bool create_devices) { return true; } - std::unique_ptr handle = FirstStageMount::Create(); - if (!handle) { - LOG(ERROR) << "Failed to create FirstStageMount"; + auto fsm = FirstStageMount::Create(); + if (!fsm.ok()) { + LOG(ERROR) << "Failed to create FirstStageMount " << fsm.error(); return false; } if (create_devices) { - if (!handle->DoCreateDevices()) return false; + if (!(*fsm)->DoCreateDevices()) return false; } - return handle->DoFirstStageMount(); + return (*fsm)->DoFirstStageMount(); } void SetInitAvbVersionInRecovery() { @@ -872,8 +878,12 @@ void SetInitAvbVersionInRecovery() { } auto fstab = ReadFirstStageFstab(); + if (!fstab.ok()) { + LOG(ERROR) << fstab.error(); + return; + } - if (!IsDtVbmetaCompatible(fstab)) { + if (!IsDtVbmetaCompatible(*fstab)) { LOG(INFO) << "Skipped setting INIT_AVB_VERSION (not vbmeta compatible)"; return; } @@ -883,7 +893,7 @@ void SetInitAvbVersionInRecovery() { // We only set INIT_AVB_VERSION when the AVB verification succeeds, i.e., the // Open() function returns a valid handle. // We don't need to mount partitions here in recovery mode. - FirstStageMountVBootV2 avb_first_mount(std::move(fstab)); + FirstStageMountVBootV2 avb_first_mount(std::move(*fstab)); if (!avb_first_mount.InitDevices()) { LOG(ERROR) << "Failed to init devices for INIT_AVB_VERSION"; return;