Invalidate idmap when target updates

When the target package update, check if the idmap file must change.
If so, propagate the idmap changes to the targets overlay paths, and
invalidate cached overlay ApkAssets in ResourcesManager.

Bug: 147794117
Bug: 150877400
Test: OverlayRemountedTest
Test: libandroidfw_tests
Change-Id: I6115c30bae3672b188a5ff270720a0eea15b43b5

8 files changed, 78 insertions, 19 deletions

diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index e55ea6c00545..75fc7f714ce3 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -149,15 +149,21 @@ Status Idmap2Service::createIdmap(const std::string& target_apk_path,
     return error(idmap.GetErrorMessage());
   }
 
+  // idmap files are mapped with mmap in libandroidfw. Deleting and recreating the idmap guarantees
+  // that existing memory maps will continue to be valid and unaffected.
+  unlink(idmap_path.c_str());
+
   umask(kIdmapFilePermissionMask);
   std::ofstream fout(idmap_path);
   if (fout.fail()) {
     return error("failed to open idmap path " + idmap_path);
   }
+
   BinaryStreamVisitor visitor(fout);
   (*idmap)->accept(&visitor);
   fout.close();
   if (fout.fail()) {
+    unlink(idmap_path.c_str());
     return error("failed to write to idmap path " + idmap_path);
   }
 
diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index d00366bd38f4..2e2612b5a412 100644
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -344,7 +344,7 @@ public class ResourcesManager {
         ApkAssets apkAssets = null;
         if (mLoadedApkAssets != null) {
             apkAssets = mLoadedApkAssets.get(newKey);
-            if (apkAssets != null) {
+            if (apkAssets != null && apkAssets.isUpToDate()) {
                 return apkAssets;
             }
         }
@@ -353,7 +353,7 @@ public class ResourcesManager {
         final WeakReference<ApkAssets> apkAssetsRef = mCachedApkAssets.get(newKey);
         if (apkAssetsRef != null) {
             apkAssets = apkAssetsRef.get();
-            if (apkAssets != null) {
+            if (apkAssets != null && apkAssets.isUpToDate()) {
                 if (mLoadedApkAssets != null) {
                     mLoadedApkAssets.put(newKey, apkAssets);
                 }
diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp
index 918e7af12d31..05f4d6b63a4c 100644
--- a/libs/androidfw/ApkAssets.cpp
+++ b/libs/androidfw/ApkAssets.cpp
@@ -385,7 +385,7 @@ std::unique_ptr<const ApkAssets> ApkAssets::LoadOverlay(const std::string& idmap
   const StringPiece idmap_data(
       reinterpret_cast<const char*>(idmap_asset->getBuffer(true /*wordAligned*/)),
       static_cast<size_t>(idmap_asset->getLength()));
-  std::unique_ptr<const LoadedIdmap> loaded_idmap = LoadedIdmap::Load(idmap_data);
+  std::unique_ptr<const LoadedIdmap> loaded_idmap = LoadedIdmap::Load(idmap_path, idmap_data);
   if (loaded_idmap == nullptr) {
     LOG(ERROR) << "failed to load IDMAP " << idmap_path;
     return {};
@@ -538,8 +538,9 @@ bool ApkAssets::IsUpToDate() const {
     // Loaders are invalidated by the app, not the system, so assume they are up to date.
     return true;
   }
+  return (!loaded_idmap_ || loaded_idmap_->IsUpToDate()) &&
+      last_mod_time_ == getFileModDate(path_.c_str());
 
-  return last_mod_time_ == getFileModDate(path_.c_str());
 }
 
 }  // namespace android
diff --git a/libs/androidfw/Idmap.cpp b/libs/androidfw/Idmap.cpp
index 0b2fd9ec982d..eb6ee9525bb9 100644
--- a/libs/androidfw/Idmap.cpp
+++ b/libs/androidfw/Idmap.cpp
@@ -20,6 +20,7 @@
 
 #include "android-base/logging.h"
 #include "android-base/stringprintf.h"
+#include "androidfw/misc.h"
 #include "androidfw/ResourceTypes.h"
 #include "androidfw/Util.h"
 #include "utils/ByteOrder.h"
@@ -192,7 +193,9 @@ static bool IsValidIdmapHeader(const StringPiece& data) {
   return true;
 }
 
-LoadedIdmap::LoadedIdmap(const Idmap_header* header,
+LoadedIdmap::LoadedIdmap(std::string&& idmap_path,
+                         const time_t last_mod_time,
+                         const Idmap_header* header,
                          const Idmap_data_header* data_header,
                          const Idmap_target_entry* target_entries,
                          const Idmap_overlay_entry* overlay_entries,
@@ -201,7 +204,9 @@ LoadedIdmap::LoadedIdmap(const Idmap_header* header,
        data_header_(data_header),
        target_entries_(target_entries),
        overlay_entries_(overlay_entries),
-       string_pool_(string_pool) {
+       string_pool_(string_pool),
+       idmap_path_(std::move(idmap_path)),
+       idmap_last_mod_time_(last_mod_time) {
 
   size_t length = strnlen(reinterpret_cast<const char*>(header_->overlay_path),
                           arraysize(header_->overlay_path));
@@ -212,7 +217,8 @@ LoadedIdmap::LoadedIdmap(const Idmap_header* header,
   target_apk_path_.assign(reinterpret_cast<const char*>(header_->target_path), length);
 }
 
-std::unique_ptr<const LoadedIdmap> LoadedIdmap::Load(const StringPiece& idmap_data) {
+std::unique_ptr<const LoadedIdmap> LoadedIdmap::Load(const StringPiece& idmap_path,
+                                                     const StringPiece& idmap_data) {
   ATRACE_CALL();
   if (!IsValidIdmapHeader(idmap_data)) {
     return {};
@@ -275,10 +281,14 @@ std::unique_ptr<const LoadedIdmap> LoadedIdmap::Load(const StringPiece& idmap_da
 
   // Can't use make_unique because LoadedIdmap constructor is private.
   std::unique_ptr<LoadedIdmap> loaded_idmap = std::unique_ptr<LoadedIdmap>(
-      new LoadedIdmap(header, data_header, target_entries, overlay_entries,
-                      idmap_string_pool.release()));
+      new LoadedIdmap(idmap_path.to_string(), getFileModDate(idmap_path.data()), header,
+                      data_header, target_entries, overlay_entries, idmap_string_pool.release()));
 
   return std::move(loaded_idmap);
 }
 
+bool LoadedIdmap::IsUpToDate() const {
+  return idmap_last_mod_time_ == getFileModDate(idmap_path_.c_str());
+}
+
 }  // namespace android
diff --git a/libs/androidfw/include/androidfw/Idmap.h b/libs/androidfw/include/androidfw/Idmap.h
index ccb57f373473..ecc1ce65d124 100644
--- a/libs/androidfw/include/androidfw/Idmap.h
+++ b/libs/androidfw/include/androidfw/Idmap.h
@@ -142,7 +142,13 @@ class IdmapResMap {
 class LoadedIdmap {
  public:
   // Loads an IDMAP from a chunk of memory. Returns nullptr if the IDMAP data was malformed.
-  static std::unique_ptr<const LoadedIdmap> Load(const StringPiece& idmap_data);
+  static std::unique_ptr<const LoadedIdmap> Load(const StringPiece& idmap_path,
+                                                 const StringPiece& idmap_data);
+
+  // Returns the path to the IDMAP.
+  inline const std::string& IdmapPath() const {
+    return idmap_path_;
+  }
 
   // Returns the path to the RRO (Runtime Resource Overlay) APK for which this IDMAP was generated.
   inline const std::string& OverlayApkPath() const {
@@ -167,6 +173,10 @@ class LoadedIdmap {
     return OverlayDynamicRefTable(data_header_, overlay_entries_, target_assigned_package_id);
   }
 
+  // Returns whether the idmap file on disk has not been modified since the construction of this
+  // LoadedIdmap.
+  bool IsUpToDate() const;
+
  protected:
   // Exposed as protected so that tests can subclass and mock this class out.
   LoadedIdmap() = default;
@@ -177,13 +187,17 @@ class LoadedIdmap {
   const Idmap_overlay_entry* overlay_entries_;
   const std::unique_ptr<ResStringPool> string_pool_;
 
+  const std::string idmap_path_;
   std::string overlay_apk_path_;
   std::string target_apk_path_;
+  const time_t idmap_last_mod_time_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LoadedIdmap);
 
-  explicit LoadedIdmap(const Idmap_header* header,
+  explicit LoadedIdmap(std::string&& idmap_path,
+                       time_t last_mod_time,
+                       const Idmap_header* header,
                        const Idmap_data_header* data_header,
                        const Idmap_target_entry* target_entries,
                        const Idmap_overlay_entry* overlay_entries,
diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp
index 41ba637da5d7..7aa0dbbafab3 100644
--- a/libs/androidfw/tests/Idmap_test.cpp
+++ b/libs/androidfw/tests/Idmap_test.cpp
@@ -38,7 +38,7 @@ class IdmapTest : public ::testing::Test {
  protected:
   void SetUp() override {
     // Move to the test data directory so the idmap can locate the overlay APK.
-    std::string original_path = base::GetExecutableDirectory();
+    original_path = base::GetExecutableDirectory();
     chdir(GetTestDataPath().c_str());
 
     system_assets_ = ApkAssets::Load("system/system.apk");
@@ -49,10 +49,14 @@ class IdmapTest : public ::testing::Test {
 
     overlayable_assets_ = ApkAssets::Load("overlayable/overlayable.apk");
     ASSERT_NE(nullptr, overlayable_assets_);
+  }
+
+  void TearDown() override {
     chdir(original_path.c_str());
   }
 
  protected:
+  std::string original_path;
   std::unique_ptr<const ApkAssets> system_assets_;
   std::unique_ptr<const ApkAssets> overlay_assets_;
   std::unique_ptr<const ApkAssets> overlayable_assets_;
@@ -221,8 +225,7 @@ TEST_F(IdmapTest, OverlaidResourceHasSameName) {
 
 TEST_F(IdmapTest, OverlayLoaderInterop) {
   std::string contents;
-  auto loader_assets = ApkAssets::LoadTable(GetTestDataPath() + "/loader/resources.arsc",
-                                            PROPERTY_LOADER);
+  auto loader_assets = ApkAssets::LoadTable("loader/resources.arsc", PROPERTY_LOADER);
 
   AssetManager2 asset_manager;
   asset_manager.SetApkAssets({overlayable_assets_.get(), loader_assets.get(),
@@ -241,4 +244,25 @@ TEST_F(IdmapTest, OverlayLoaderInterop) {
   ASSERT_EQ(GetStringFromApkAssets(asset_manager, val, cookie), "loader");
 }
 
+TEST_F(IdmapTest, OverlayAssetsIsUpToDate) {
+  std::string idmap_contents;
+  ASSERT_TRUE(base::ReadFileToString("overlay/overlay.idmap", &idmap_contents));
+
+  TemporaryFile temp_file;
+  ASSERT_TRUE(base::WriteStringToFile(idmap_contents, temp_file.path));
+
+  auto apk_assets = ApkAssets::LoadOverlay(temp_file.path);
+  ASSERT_NE(nullptr, apk_assets);
+  ASSERT_TRUE(apk_assets->IsUpToDate());
+
+  unlink(temp_file.path);
+  ASSERT_FALSE(apk_assets->IsUpToDate());
+  sleep(2);
+
+  base::WriteStringToFile("hello", temp_file.path);
+  sleep(2);
+
+  ASSERT_FALSE(apk_assets->IsUpToDate());
+}
+
 }  // namespace
diff --git a/services/core/java/com/android/server/om/IdmapManager.java b/services/core/java/com/android/server/om/IdmapManager.java
index 43fc7ed0e39d..90c85ada9def 100644
--- a/services/core/java/com/android/server/om/IdmapManager.java
+++ b/services/core/java/com/android/server/om/IdmapManager.java
@@ -63,20 +63,23 @@ class IdmapManager {
         mIdmapDaemon = IdmapDaemon.getInstance();
     }
 
+    /**
+     * Creates the idmap for the target/overlay combination and returns whether the idmap file was
+     * modified.
+     */
     boolean createIdmap(@NonNull final PackageInfo targetPackage,
             @NonNull final PackageInfo overlayPackage, int userId) {
         if (DEBUG) {
             Slog.d(TAG, "create idmap for " + targetPackage.packageName + " and "
                     + overlayPackage.packageName);
         }
-        final int sharedGid = UserHandle.getSharedAppGid(targetPackage.applicationInfo.uid);
         final String targetPath = targetPackage.applicationInfo.getBaseCodePath();
         final String overlayPath = overlayPackage.applicationInfo.getBaseCodePath();
         try {
             int policies = calculateFulfilledPolicies(targetPackage, overlayPackage, userId);
             boolean enforce = enforceOverlayable(overlayPackage);
             if (mIdmapDaemon.verifyIdmap(targetPath, overlayPath, policies, enforce, userId)) {
-                return true;
+                return false;
             }
             return mIdmapDaemon.createIdmap(targetPath, overlayPath, policies,
                     enforce, userId) != null;
diff --git a/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java b/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java
index d108e76e37df..05a4a38feef1 100644
--- a/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java
+++ b/services/core/java/com/android/server/om/OverlayManagerServiceImpl.java
@@ -700,14 +700,15 @@ final class OverlayManagerServiceImpl {
         final PackageInfo overlayPackage = mPackageManager.getPackageInfo(overlayPackageName,
                 userId);
 
-        // Immutable RROs targeting to "android", ie framework-res.apk, are handled by native layers.
+        // Immutable RROs targeting to "android", ie framework-res.apk, are handled by native
+        // layers.
+        boolean modified = false;
         if (targetPackage != null && overlayPackage != null
                 && !("android".equals(targetPackageName)
                     && !isPackageConfiguredMutable(overlayPackageName))) {
-            mIdmapManager.createIdmap(targetPackage, overlayPackage, userId);
+            modified |= mIdmapManager.createIdmap(targetPackage, overlayPackage, userId);
         }
 
-        boolean modified = false;
         if (overlayPackage != null) {
             modified |= mSettings.setBaseCodePath(overlayPackageName, userId,
                     overlayPackage.applicationInfo.getBaseCodePath());