From 995b24692cd557b71b46be0a402ad8b0ee5c1a09 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 6 Oct 2020 13:42:41 +0800 Subject: Fixes De data is not backed up correctly when user locked (1/n) https://android.googlesource.com/platform/frameworks/native/+/fda19ecdd6df8f43d3368781a633792723fb9965/cmds/installd/InstalldNativeService.cpp#933 When an APK is stage installed, snapshotAppData(..., FLAG_STORAGE_DE) is called before user unlocked. The function bails out early at #937 because the folder is still encrypted. We should move the if block at #958 above so we can back up De data correctly even when Ce directories not ready. Note we have to check if the De folder exists before making a copy. (Cherry-picked from 1d643c77d3fcb7dfe0bf4dabed2964bbfca04c6f) Bug: 169594054 Test: atest StagedRollbackTest Merged-In: I2ca810bd9495de3bed58378a41b47863c6e8f8dd Change-Id: I2ca810bd9495de3bed58378a41b47863c6e8f8dd --- cmds/installd/InstalldNativeService.cpp | 51 +++++++++++++++++---------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 34727270e4..856b545c7c 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -876,6 +876,33 @@ binder::Status InstalldNativeService::snapshotAppData( auto scope_guard = android::base::make_scope_guard(deleter); + if (storageFlags & FLAG_STORAGE_DE) { + auto from = create_data_user_de_package_path(volume_uuid, user, package_name); + auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId); + auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user, + snapshotId, package_name); + + int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode); + if (rc != 0) { + return error(rc, "Failed to create folder " + to); + } + + rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */); + if (rc != 0) { + return error(rc, "Failed clearing existing snapshot " + rollback_package_path); + } + + // Check if we have data to copy. + if (access(from.c_str(), F_OK) == 0) { + rc = copy_directory_recursive(from.c_str(), to.c_str()); + } + if (rc != 0) { + res = error(rc, "Failed copying " + from + " to " + to); + clear_de_on_exit = true; + return res; + } + } + // The app may not have any data at all, in which case it's OK to skip here. auto from_ce = create_data_user_ce_package_path(volume_uuid, user, package_name); if (access(from_ce.c_str(), F_OK) != 0) { @@ -901,30 +928,6 @@ binder::Status InstalldNativeService::snapshotAppData( LOG(WARNING) << "Failed to clear code_cache of app " << packageName; } - if (storageFlags & FLAG_STORAGE_DE) { - auto from = create_data_user_de_package_path(volume_uuid, user, package_name); - auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId); - auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user, - snapshotId, package_name); - - int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode); - if (rc != 0) { - return error(rc, "Failed to create folder " + to); - } - - rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */); - if (rc != 0) { - return error(rc, "Failed clearing existing snapshot " + rollback_package_path); - } - - rc = copy_directory_recursive(from.c_str(), to.c_str()); - if (rc != 0) { - res = error(rc, "Failed copying " + from + " to " + to); - clear_de_on_exit = true; - return res; - } - } - if (storageFlags & FLAG_STORAGE_CE) { auto from = create_data_user_ce_package_path(volume_uuid, user, package_name); auto to = create_data_misc_ce_rollback_path(volume_uuid, user, snapshotId); -- cgit v1.2.3 From 7c227cc333b85938a1ad0f860655bb83567ca755 Mon Sep 17 00:00:00 2001 From: Jon Spivack Date: Tue, 27 Oct 2020 19:29:14 -0700 Subject: libbinder: Add ClientCounterCallbackImpl to LazyServiceRegistrar This extra layer of indirection below ClientCounterCallback fixes a shared pointer ownership issue between LazyServiceRegistrar and ServiceManager. It also allows for implementation changes (like this one) without changing headers and breaking VNDK. Bug: 170212632 Test: Manual (Went through reproduction steps in bug on cf_x86_phone-userdebug) Test: atest aidl_lazy_test Change-Id: I4164a6d44e567c752726953e85aee0e91c6b525e Merged-In: I4164a6d44e567c752726953e85aee0e91c6b525e --- libs/binder/LazyServiceRegistrar.cpp | 46 +++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index 6f49aa1607..f2c5139b56 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -29,16 +29,12 @@ namespace internal { using AidlServiceManager = android::os::IServiceManager; -class ClientCounterCallback : public ::android::os::BnClientCallback { +class ClientCounterCallbackImpl : public ::android::os::BnClientCallback { public: - ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {} + ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {} bool registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags); - - /** - * Set a flag to prevent services from automatically shutting down - */ void forcePersist(bool persist); protected: @@ -69,7 +65,23 @@ private: bool mForcePersist; }; -bool ClientCounterCallback::registerService(const sp& service, const std::string& name, +class ClientCounterCallback { +public: + ClientCounterCallback(); + + bool registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags); + + /** + * Set a flag to prevent services from automatically shutting down + */ + void forcePersist(bool persist); + +private: + sp mImpl; +}; + +bool ClientCounterCallbackImpl::registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags) { auto manager = interface_cast(asBinder(defaultServiceManager())); @@ -95,7 +107,7 @@ bool ClientCounterCallback::registerService(const sp& service, const st return true; } -void ClientCounterCallback::forcePersist(bool persist) { +void ClientCounterCallbackImpl::forcePersist(bool persist) { mForcePersist = persist; if(!mForcePersist) { // Attempt a shutdown in case the number of clients hit 0 while the flag was on @@ -107,7 +119,7 @@ void ClientCounterCallback::forcePersist(bool persist) { * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple * invocations could occur on different threads however. */ -Status ClientCounterCallback::onClients(const sp& service, bool clients) { +Status ClientCounterCallbackImpl::onClients(const sp& service, bool clients) { if (clients) { mNumConnectedServices++; } else { @@ -122,7 +134,7 @@ Status ClientCounterCallback::onClients(const sp& service, bool clients return Status::ok(); } -void ClientCounterCallback::tryShutdown() { +void ClientCounterCallbackImpl::tryShutdown() { if(mNumConnectedServices > 0) { // Should only shut down if there are no clients return; @@ -143,7 +155,6 @@ void ClientCounterCallback::tryShutdown() { bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk(); - if (!success) { ALOGI("Failed to unregister service %s", entry.first.c_str()); break; @@ -168,6 +179,19 @@ void ClientCounterCallback::tryShutdown() { } } +ClientCounterCallback::ClientCounterCallback() { + mImpl = new ClientCounterCallbackImpl(); +} + +bool ClientCounterCallback::registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags) { + return mImpl->registerService(service, name, allowIsolated, dumpFlags); +} + +void ClientCounterCallback::forcePersist(bool persist) { + mImpl->forcePersist(persist); +} + } // namespace internal LazyServiceRegistrar::LazyServiceRegistrar() { -- cgit v1.2.3