From 9ede7ec273539d7ad6820bdf3f1f2f28433a9fb3 Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Mon, 7 Sep 2020 10:27:25 +0100 Subject: [PATCH] Only store result of mount_all that mounted userdata During boot sequence there can be multiple calls to mount_all. For the userspace reboot to correctly remount userdata, we need to store the return code of the one that was responsible in mounting userdata. Test: adb root Test: adb shell setprop init.userspace_reboot.is_supported 1 Test: adb reboot userspace Test: checked dmsg Bug: 166353152 Change-Id: Id0ae15f3bcf65fa54e4e72b76f64716c053af7fb --- fs_mgr/fs_mgr.cpp | 24 ++++++++++++++---------- fs_mgr/include/fs_mgr.h | 14 +++++++++++++- init/builtins.cpp | 14 +++++++++++--- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index d6fd5130f..47961d3a1 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1296,14 +1296,15 @@ static bool IsMountPointMounted(const std::string& mount_point) { // When multiple fstab records share the same mount_point, it will try to mount each // one in turn, and ignore any duplicates after a first successful mount. // Returns -1 on error, and FS_MGR_MNTALL_* otherwise. -int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { +MountAllResult fs_mgr_mount_all(Fstab* fstab, int mount_mode) { int encryptable = FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE; int error_count = 0; CheckpointManager checkpoint_manager; AvbUniquePtr avb_handle(nullptr); + bool userdata_mounted = false; if (fstab->empty()) { - return FS_MGR_MNTALL_FAIL; + return {FS_MGR_MNTALL_FAIL, userdata_mounted}; } // Keep i int to prevent unsigned integer overflow from (i = top_idx - 1), @@ -1343,7 +1344,7 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { } // Terrible hack to make it possible to remount /data. - // TODO: refact fs_mgr_mount_all and get rid of this. + // TODO: refactor fs_mgr_mount_all and get rid of this. if (mount_mode == MOUNT_MODE_ONLY_USERDATA && current_entry.mount_point != "/data") { continue; } @@ -1380,7 +1381,7 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { if (!avb_handle) { LERROR << "Failed to open AvbHandle"; set_type_property(encryptable); - return FS_MGR_MNTALL_FAIL; + return {FS_MGR_MNTALL_FAIL, userdata_mounted}; } } if (avb_handle->SetUpAvbHashtree(¤t_entry, true /* wait_for_verity_dev */) == @@ -1422,7 +1423,7 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { if (status == FS_MGR_MNTALL_FAIL) { // Fatal error - no point continuing. - return status; + return {status, userdata_mounted}; } if (status != FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE) { @@ -1437,11 +1438,14 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { nullptr)) { LERROR << "Encryption failed"; set_type_property(encryptable); - return FS_MGR_MNTALL_FAIL; + return {FS_MGR_MNTALL_FAIL, userdata_mounted}; } } } + if (current_entry.mount_point == "/data") { + userdata_mounted = true; + } // Success! Go get the next one. continue; } @@ -1541,9 +1545,9 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { #endif if (error_count) { - return FS_MGR_MNTALL_FAIL; + return {FS_MGR_MNTALL_FAIL, userdata_mounted}; } else { - return encryptable; + return {encryptable, userdata_mounted}; } } @@ -1765,8 +1769,8 @@ int fs_mgr_remount_userdata_into_checkpointing(Fstab* fstab) { } LINFO << "Remounting /data"; // TODO(b/143970043): remove this hack after fs_mgr_mount_all is refactored. - int result = fs_mgr_mount_all(fstab, MOUNT_MODE_ONLY_USERDATA); - return result == FS_MGR_MNTALL_FAIL ? -1 : 0; + auto result = fs_mgr_mount_all(fstab, MOUNT_MODE_ONLY_USERDATA); + return result.code == FS_MGR_MNTALL_FAIL ? -1 : 0; } return 0; } diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 2a67b8c9f..11e36645e 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -60,8 +60,20 @@ enum mount_mode { #define FS_MGR_MNTALL_DEV_NOT_ENCRYPTED 1 #define FS_MGR_MNTALL_DEV_NOT_ENCRYPTABLE 0 #define FS_MGR_MNTALL_FAIL (-1) + +struct MountAllResult { + // One of the FS_MGR_MNTALL_* returned code defined above. + int code; + // Whether userdata was mounted as a result of |fs_mgr_mount_all| call. + bool userdata_mounted; +}; + // fs_mgr_mount_all() updates fstab entries that reference device-mapper. -int fs_mgr_mount_all(android::fs_mgr::Fstab* fstab, int mount_mode); +// Returns a |MountAllResult|. The first element is one of the FS_MNG_MNTALL_* return codes +// defined above, and the second element tells whether this call to fs_mgr_mount_all was responsible +// for mounting userdata. Later is required for init to correctly enqueue fs-related events as part +// of userdata remount during userspace reboot. +MountAllResult fs_mgr_mount_all(android::fs_mgr::Fstab* fstab, int mount_mode); #define FS_MGR_DOMNT_FAILED (-1) #define FS_MGR_DOMNT_BUSY (-2) diff --git a/init/builtins.cpp b/init/builtins.cpp index 597c32d05..d00d1b121 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -662,18 +662,26 @@ static Result do_mount_all(const BuiltinArguments& args) { } } - auto mount_fstab_return_code = fs_mgr_mount_all(&fstab, mount_all->mode); + auto mount_fstab_result = fs_mgr_mount_all(&fstab, mount_all->mode); SetProperty(prop_name, std::to_string(t.duration().count())); if (mount_all->import_rc) { import_late(mount_all->rc_paths); } + if (mount_fstab_result.userdata_mounted) { + // This call to fs_mgr_mount_all mounted userdata. Keep the result in + // order for userspace reboot to correctly remount userdata. + LOG(INFO) << "Userdata mounted using " + << (mount_all->fstab_path.empty() ? "(default fstab)" : mount_all->fstab_path) + << " result : " << mount_fstab_result.code; + initial_mount_fstab_return_code = mount_fstab_result.code; + } + if (queue_event) { /* queue_fs_event will queue event based on mount_fstab return code * and return processed return code*/ - initial_mount_fstab_return_code = mount_fstab_return_code; - auto queue_fs_result = queue_fs_event(mount_fstab_return_code, false); + auto queue_fs_result = queue_fs_event(mount_fstab_result.code, false); if (!queue_fs_result.ok()) { return Error() << "queue_fs_event() failed: " << queue_fs_result.error(); }