From 3d49ac4660db7722d15c65302c33e318f837e0ed Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Mon, 10 Aug 2020 16:33:24 -0700 Subject: [PATCH] Fix ApkAssets mNativePtr accesses Actually acquire the monitor it claims to be guarded by. Since it was a final field, it's still not entirely clear what that means. Clear mNativePtr when we deallocate its referent. This prevents native heap corruption if methods here are called from another finalizer. Remove mOpen and use mNativePtr instead. This probably does not fix the bug below. But let's get rid of the possible heap corruption source we know about. Bug: 159041693 Test: Build and boot Change-Id: I6f0f6250ca8b6a4274c346ae99f1f94cab5844e1 --- core/java/android/content/res/ApkAssets.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/core/java/android/content/res/ApkAssets.java b/core/java/android/content/res/ApkAssets.java index bc418061e1d11..b0437ac7284ed 100644 --- a/core/java/android/content/res/ApkAssets.java +++ b/core/java/android/content/res/ApkAssets.java @@ -101,14 +101,11 @@ public final class ApkAssets { public @interface FormatType {} @GuardedBy("this") - private final long mNativePtr; + private long mNativePtr; // final, except cleared in finalizer. @Nullable @GuardedBy("this") - private final StringBlock mStringBlock; - - @GuardedBy("this") - private boolean mOpen = true; + private final StringBlock mStringBlock; // null or closed if mNativePtr = 0. @PropertyFlags private final int mFlags; @@ -380,12 +377,16 @@ public final class ApkAssets { /** @hide */ @Nullable public OverlayableInfo getOverlayableInfo(String overlayableName) throws IOException { - return nativeGetOverlayableInfo(mNativePtr, overlayableName); + synchronized (this) { + return nativeGetOverlayableInfo(mNativePtr, overlayableName); + } } /** @hide */ public boolean definesOverlayable() throws IOException { - return nativeDefinesOverlayable(mNativePtr); + synchronized (this) { + return nativeDefinesOverlayable(mNativePtr); + } } /** @@ -412,12 +413,12 @@ public final class ApkAssets { */ public void close() { synchronized (this) { - if (mOpen) { - mOpen = false; + if (mNativePtr != 0) { if (mStringBlock != null) { mStringBlock.close(); } nativeDestroy(mNativePtr); + mNativePtr = 0; } } }