Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix for race in writeToParcel and unparcel

Browse Source 
Don't access the parcelled data while it might be recycled
by another thread.

Also make a local reference of mMap, which could be modified
by another thread.

Bug: 33325384
Test: Hard to repro, so no test
Change-Id: I77f6545ac174236a3da69c0c0f3a01b2eda3f34b
This commit is contained in:
Amith Yamasani
2016-12-15 14:23:24 -08:00
parent e298756bf8
commit 694753465b
1 changed files with 61 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

116
core/java/android/os/BaseBundle.java
View File

@@ -213,7 +213,7 @@ public class BaseBundle {
* If the underlying data are stored as a Parcel, unparcel them
* using the currently assigned class loader.
*/
/* package */ synchronized void unparcel() {
/* package */ void unparcel() {
synchronized (this) {
final Parcel parcelledData = mParcelledData;
if (parcelledData == null) {
@@ -319,34 +319,37 @@ public class BaseBundle {
}
void copyInternal(BaseBundle from, boolean deep) {
if (from.mParcelledData != null) {
if (from.isEmptyParcel()) {
mParcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
} else {
mParcelledData = Parcel.obtain();
mParcelledData.appendFrom(from.mParcelledData, 0, from.mParcelledData.dataSize());
mParcelledData.setDataPosition(0);
}
} else {
mParcelledData = null;
}
if (from.mMap != null) {
if (!deep) {
mMap = new ArrayMap<>(from.mMap);
} else {
final ArrayMap<String, Object> fromMap = from.mMap;
final int N = fromMap.size();
mMap = new ArrayMap<>(N);
for (int i=0; i<N; i++) {
mMap.append(fromMap.keyAt(i), deepcopyValue(fromMap.valueAt(i)));
synchronized (from) {
if (from.mParcelledData != null) {
if (from.isEmptyParcel()) {
mParcelledData = NoImagePreloadHolder.EMPTY_PARCEL;
} else {
mParcelledData = Parcel.obtain();
mParcelledData.appendFrom(from.mParcelledData, 0,
from.mParcelledData.dataSize());
mParcelledData.setDataPosition(0);
}
} else {
mParcelledData = null;
}
} else {
mMap = null;
}
mClassLoader = from.mClassLoader;
if (from.mMap != null) {
if (!deep) {
mMap = new ArrayMap<>(from.mMap);
} else {
final ArrayMap<String, Object> fromMap = from.mMap;
final int N = fromMap.size();
mMap = new ArrayMap<>(N);
for (int i = 0; i < N; i++) {
mMap.append(fromMap.keyAt(i), deepcopyValue(fromMap.valueAt(i)));
}
}
} else {
mMap = null;
}
mClassLoader = from.mClassLoader;
}
}
Object deepcopyValue(Object value) {
@@ -1441,39 +1444,42 @@ public class BaseBundle {
void writeToParcelInner(Parcel parcel, int flags) {
// Keep implementation in sync with writeToParcel() in
// frameworks/native/libs/binder/PersistableBundle.cpp.
final Parcel parcelledData;
final ArrayMap<String, Object> map;
synchronized (this) {
parcelledData = mParcelledData;
}
if (parcelledData != null) {
if (isEmptyParcel()) {
parcel.writeInt(0);
} else {
int length = parcelledData.dataSize();
parcel.writeInt(length);
parcel.writeInt(BUNDLE_MAGIC);
parcel.appendFrom(parcelledData, 0, length);
}
} else {
// Special case for empty bundles.
if (mMap == null || mMap.size() <= 0) {
parcel.writeInt(0);
// unparcel() can race with this method and cause the parcel to recycle
// at the wrong time. So synchronize access the mParcelledData's content.
if (mParcelledData != null) {
if (mParcelledData == NoImagePreloadHolder.EMPTY_PARCEL) {
parcel.writeInt(0);
} else {
int length = mParcelledData.dataSize();
parcel.writeInt(length);
parcel.writeInt(BUNDLE_MAGIC);
parcel.appendFrom(mParcelledData, 0, length);
}
return;
}
int lengthPos = parcel.dataPosition();
parcel.writeInt(-1); // dummy, will hold length
parcel.writeInt(BUNDLE_MAGIC);
int startPos = parcel.dataPosition();
parcel.writeArrayMapInternal(mMap);
int endPos = parcel.dataPosition();
// Backpatch length
parcel.setDataPosition(lengthPos);
int length = endPos - startPos;
parcel.writeInt(length);
parcel.setDataPosition(endPos);
map = mMap;
}
// Special case for empty bundles.
if (map == null || map.size() <= 0) {
parcel.writeInt(0);
return;
}
int lengthPos = parcel.dataPosition();
parcel.writeInt(-1); // dummy, will hold length
parcel.writeInt(BUNDLE_MAGIC);
int startPos = parcel.dataPosition();
parcel.writeArrayMapInternal(map);
int endPos = parcel.dataPosition();
// Backpatch length
parcel.setDataPosition(lengthPos);
int length = endPos - startPos;
parcel.writeInt(length);
parcel.setDataPosition(endPos);
}
/**