Revert "Fix vulnerability in MemoryIntArray" am: 29139a8ae5 am: 86699f980f am: 65cf055ad9 (116a4e88) · Commits · e / os / android_frameworks_base

core/java/android/util/MemoryIntArray.java

+37 −22

Original line number	Original line	Diff line number	Diff line
	@@ -35,13 +35,13 @@ import java.util.UUID;
	* each other.		* each other.
	* <p>		* <p>
	* The data structure is designed to have one owner process that can		* The data structure is designed to have one owner process that can
	* read/write. There may be multiple client processes that can only read.		* read/write. There may be multiple client processes that can only read or
	* The owner process is the process that created the array. The shared		* read/write depending how the data structure was configured when
	* memory is pinned (not reclaimed by the system) until the owning process		* instantiated. The owner process is the process that created the array.
	* dies or the data structure is closed. This class is <strong>not</strong>		* The shared memory is pinned (not reclaimed by the system) until the
	* thread safe. You should not interact with an instance of this class		* owning process dies or the data structure is closed. This class
	* once it is closed. If you pass back to the owner process an instance		* is <strong>not</strong> thread safe. You should not interact with
	* it will be read only even in the owning process.		* an instance of this class once it is closed.
	* </p>		* </p>
	*		*
	* @hide		* @hide
	@@ -51,7 +51,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {

	private static final int MAX_SIZE = 1024;		private static final int MAX_SIZE = 1024;

	private final boolean mIsOwner;		private final int mOwnerPid;
			private final boolean mClientWritable;
	private final long mMemoryAddr;		private final long mMemoryAddr;
	private int mFd;		private int mFd;

	@@ -63,25 +64,31 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	* @param clientWritable Whether other processes can write to the array.		* @param clientWritable Whether other processes can write to the array.
	* @throws IOException If an error occurs while accessing the shared memory.		* @throws IOException If an error occurs while accessing the shared memory.
	*/		*/
	public MemoryIntArray(int size) throws IOException {		public MemoryIntArray(int size, boolean clientWritable) throws IOException {
	if (size > MAX_SIZE) {		if (size > MAX_SIZE) {
	throw new IllegalArgumentException("Max size is " + MAX_SIZE);		throw new IllegalArgumentException("Max size is " + MAX_SIZE);
	}		}
	mIsOwner = true;		mOwnerPid = Process.myPid();
			mClientWritable = clientWritable;
	final String name = UUID.randomUUID().toString();		final String name = UUID.randomUUID().toString();
	mFd = nativeCreate(name, size);		mFd = nativeCreate(name, size);
	mMemoryAddr = nativeOpen(mFd, mIsOwner);		mMemoryAddr = nativeOpen(mFd, true, clientWritable);
	}		}

	private MemoryIntArray(Parcel parcel) throws IOException {		private MemoryIntArray(Parcel parcel) throws IOException {
	mIsOwner = false;		mOwnerPid = parcel.readInt();
			mClientWritable = (parcel.readInt() == 1);
	ParcelFileDescriptor pfd = parcel.readParcelable(null);		ParcelFileDescriptor pfd = parcel.readParcelable(null);
	if (pfd == null) {		if (pfd == null) {
	throw new IOException("No backing file descriptor");		throw new IOException("No backing file descriptor");
	}		}
	mFd = pfd.detachFd();		mFd = pfd.detachFd();
	mMemoryAddr = nativeOpen(mFd, mIsOwner);		final long memoryAddress = parcel.readLong();
	mCloseGuard.open("close");		if (isOwner()) {
			mMemoryAddr = memoryAddress;
			} else {
			mMemoryAddr = nativeOpen(mFd, false, mClientWritable);
			}
	}		}

	/**		/**
	@@ -89,7 +96,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	*/		*/
	public boolean isWritable() {		public boolean isWritable() {
	enforceNotClosed();		enforceNotClosed();
	return mIsOwner;		return isOwner() \|\| mClientWritable;
	}		}

	/**		/**
	@@ -102,7 +109,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	public int get(int index) throws IOException {		public int get(int index) throws IOException {
	enforceNotClosed();		enforceNotClosed();
	enforceValidIndex(index);		enforceValidIndex(index);
	return nativeGet(mFd, mMemoryAddr, index);		return nativeGet(mFd, mMemoryAddr, index, isOwner());
	}		}

	/**		/**
	@@ -118,7 +125,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	enforceNotClosed();		enforceNotClosed();
	enforceWritable();		enforceWritable();
	enforceValidIndex(index);		enforceValidIndex(index);
	nativeSet(mFd, mMemoryAddr, index, value);		nativeSet(mFd, mMemoryAddr, index, value, isOwner());
	}		}

	/**		/**
	@@ -139,7 +146,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	@Override		@Override
	public void close() throws IOException {		public void close() throws IOException {
	if (!isClosed()) {		if (!isClosed()) {
	nativeClose(mFd, mMemoryAddr, mIsOwner);		nativeClose(mFd, mMemoryAddr, isOwner());
	mFd = -1;		mFd = -1;
	}		}
	}		}
	@@ -166,7 +173,10 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	public void writeToParcel(Parcel parcel, int flags) {		public void writeToParcel(Parcel parcel, int flags) {
	ParcelFileDescriptor pfd = ParcelFileDescriptor.adoptFd(mFd);		ParcelFileDescriptor pfd = ParcelFileDescriptor.adoptFd(mFd);
	try {		try {
			parcel.writeInt(mOwnerPid);
			parcel.writeInt(mClientWritable ? 1 : 0);
	parcel.writeParcelable(pfd, flags & ~Parcelable.PARCELABLE_WRITE_RETURN_VALUE);		parcel.writeParcelable(pfd, flags & ~Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
			parcel.writeLong(mMemoryAddr);
	} finally {		} finally {
	pfd.detachFd();		pfd.detachFd();
	}		}
	@@ -192,6 +202,10 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	return mFd;		return mFd;
	}		}

			private boolean isOwner() {
			return mOwnerPid == Process.myPid();
			}

	private void enforceNotClosed() {		private void enforceNotClosed() {
	if (isClosed()) {		if (isClosed()) {
	throw new IllegalStateException("cannot interact with a closed instance");		throw new IllegalStateException("cannot interact with a closed instance");
	@@ -213,10 +227,10 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	}		}

	private native int nativeCreate(String name, int size);		private native int nativeCreate(String name, int size);
	private native long nativeOpen(int fd, boolean owner);		private native long nativeOpen(int fd, boolean owner, boolean writable);
	private native void nativeClose(int fd, long memoryAddr, boolean owner);		private native void nativeClose(int fd, long memoryAddr, boolean owner);
	private native int nativeGet(int fd, long memoryAddr, int index);		private native int nativeGet(int fd, long memoryAddr, int index, boolean owner);
	private native void nativeSet(int fd, long memoryAddr, int index, int value);		private native void nativeSet(int fd, long memoryAddr, int index, int value, boolean owner);
	private native int nativeSize(int fd);		private native int nativeSize(int fd);

	/**		/**
	@@ -233,7 +247,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {
	try {		try {
	return new MemoryIntArray(parcel);		return new MemoryIntArray(parcel);
	} catch (IOException ioe) {		} catch (IOException ioe) {
	throw new IllegalArgumentException("Error unparceling MemoryIntArray");		Log.e(TAG, "Error unparceling MemoryIntArray");
			return null;
	}		}
	}		}

core/jni/android_util_MemoryIntArray.cpp

+8 −24

Original line number	Original line	Diff line number	Diff line
	@@ -54,7 +54,7 @@ static jint android_util_MemoryIntArray_create(JNIEnv* env, jobject clazz, jstri
	}		}

	static jlong android_util_MemoryIntArray_open(JNIEnv* env, jobject clazz, jint fd,		static jlong android_util_MemoryIntArray_open(JNIEnv* env, jobject clazz, jint fd,
	jboolean owner)		jboolean owner, jboolean writable)
	{		{
	if (fd < 0) {		if (fd < 0) {
	jniThrowException(env, "java/io/IOException", "bad file descriptor");		jniThrowException(env, "java/io/IOException", "bad file descriptor");
	@@ -67,35 +67,19 @@ static jlong android_util_MemoryIntArray_open(JNIEnv* env, jobject clazz, jint f
	return -1;		return -1;
	}		}

	// IMPORTANT: Ashmem allows the caller to change its size until		int protMode = (owner \|\| writable) ? (PROT_READ \| PROT_WRITE) : PROT_READ;
	// it is memory mapped for the first time which lazily creates
	// the underlying VFS file. So the size we get above may not
	// reflect the size of the underlying shared memory region. Therefore,
	// we first memory map to set the size in stone an verify if
	// the underlying ashmem region has the same size as the one we
	// memory mapped. This is critical as we use the underlying
	// ashmem size for boundary checks and memory unmapping.
	int protMode = owner ? (PROT_READ \| PROT_WRITE) : PROT_READ;
	void* ashmemAddr = mmap(NULL, ashmemSize, protMode, MAP_SHARED, fd, 0);		void* ashmemAddr = mmap(NULL, ashmemSize, protMode, MAP_SHARED, fd, 0);
	if (ashmemAddr == MAP_FAILED) {		if (ashmemAddr == MAP_FAILED) {
	jniThrowException(env, "java/io/IOException", "cannot mmap ashmem");		jniThrowException(env, "java/io/IOException", "cannot mmap ashmem");
	return -1;		return -1;
	}		}

	// Check if the mapped size is the same as the ashmem region.
	int mmapedSize = ashmem_get_size_region(fd);
	if (mmapedSize != ashmemSize) {
	munmap(reinterpret_cast<void *>(ashmemAddr), ashmemSize);
	jniThrowException(env, "java/io/IOException", "bad file descriptor");
	return -1;
	}

	if (owner) {		if (owner) {
	int size = ashmemSize / sizeof(std::atomic_int);		int size = ashmemSize / sizeof(std::atomic_int);
	new (ashmemAddr) std::atomic_int[size];		new (ashmemAddr) std::atomic_int[size];
	}		}

	if (owner) {		if (owner && !writable) {
	int setProtResult = ashmem_set_prot_region(fd, PROT_READ);		int setProtResult = ashmem_set_prot_region(fd, PROT_READ);
	if (setProtResult < 0) {		if (setProtResult < 0) {
	jniThrowException(env, "java/io/IOException", "cannot set ashmem prot mode");		jniThrowException(env, "java/io/IOException", "cannot set ashmem prot mode");
	@@ -137,7 +121,7 @@ static void android_util_MemoryIntArray_close(JNIEnv* env, jobject clazz, jint f
	}		}

	static jint android_util_MemoryIntArray_get(JNIEnv* env, jobject clazz,		static jint android_util_MemoryIntArray_get(JNIEnv* env, jobject clazz,
	jint fd, jlong address, jint index)		jint fd, jlong address, jint index, jboolean owner)
	{		{
	if (fd < 0) {		if (fd < 0) {
	jniThrowException(env, "java/io/IOException", "bad file descriptor");		jniThrowException(env, "java/io/IOException", "bad file descriptor");
	@@ -154,7 +138,7 @@ static jint android_util_MemoryIntArray_get(JNIEnv* env, jobject clazz,
	}		}

	static void android_util_MemoryIntArray_set(JNIEnv* env, jobject clazz,		static void android_util_MemoryIntArray_set(JNIEnv* env, jobject clazz,
	jint fd, jlong address, jint index, jint newValue)		jint fd, jlong address, jint index, jint newValue, jboolean owner)
	{		{
	if (fd < 0) {		if (fd < 0) {
	jniThrowException(env, "java/io/IOException", "bad file descriptor");		jniThrowException(env, "java/io/IOException", "bad file descriptor");
	@@ -187,10 +171,10 @@ static jint android_util_MemoryIntArray_size(JNIEnv* env, jobject clazz, jint fd

	static const JNINativeMethod methods[] = {		static const JNINativeMethod methods[] = {
	{"nativeCreate", "(Ljava/lang/String;I)I", (void*)android_util_MemoryIntArray_create},		{"nativeCreate", "(Ljava/lang/String;I)I", (void*)android_util_MemoryIntArray_create},
	{"nativeOpen", "(IZ)J", (void*)android_util_MemoryIntArray_open},		{"nativeOpen", "(IZZ)J", (void*)android_util_MemoryIntArray_open},
	{"nativeClose", "(IJZ)V", (void*)android_util_MemoryIntArray_close},		{"nativeClose", "(IJZ)V", (void*)android_util_MemoryIntArray_close},
	{"nativeGet", "(IJI)I", (void*)android_util_MemoryIntArray_get},		{"nativeGet", "(IJIZ)I", (void*)android_util_MemoryIntArray_get},
	{"nativeSet", "(IJII)V", (void*) android_util_MemoryIntArray_set},		{"nativeSet", "(IJIIZ)V", (void*) android_util_MemoryIntArray_set},
	{"nativeSize", "(I)I", (void*) android_util_MemoryIntArray_size},		{"nativeSize", "(I)I", (void*) android_util_MemoryIntArray_size},
	};		};

core/tests/utiltests/Android.mk

+0 −2

Original line number	Original line	Diff line number	Diff line
	@@ -12,8 +12,6 @@ LOCAL_MODULE_TAGS := tests
	LOCAL_SRC_FILES := $(call all-java-files-under, src)		LOCAL_SRC_FILES := $(call all-java-files-under, src)
	LOCAL_SRC_FILES += src/android/util/IRemoteMemoryIntArray.aidl		LOCAL_SRC_FILES += src/android/util/IRemoteMemoryIntArray.aidl

	LOCAL_JNI_SHARED_LIBRARIES := libmemoryintarraytest libcutils libc++

	LOCAL_STATIC_JAVA_LIBRARIES := \		LOCAL_STATIC_JAVA_LIBRARIES := \
	android-support-test \		android-support-test \
	mockito-target		mockito-target

core/tests/utiltests/jni/Android.bp

deleted100644 → 0

+0 −27

Original line number	Original line	Diff line number	Diff line
	// Copyright (C) 2016 The Android Open Source Project
	//
	// Licensed under the Apache License, Version 2.0 (the "License");
	// you may not use this file except in compliance with the License.
	// You may obtain a copy of the License at
	//
	// http://www.apache.org/licenses/LICENSE-2.0
	//
	// Unless required by applicable law or agreed to in writing, software
	// distributed under the License is distributed on an "AS IS" BASIS,
	// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
	// See the License for the specific language governing permissions and
	// limitations under the License.

	cc_library_shared {
	name: "libmemoryintarraytest",
	shared_libs: [
	"libcutils",
	],
	clang: true,
	stl: "libc++",
	srcs: [
	"registration.cpp",
	"android_util_MemoryIntArrayTest.cpp",
	],
	cflags: ["-Werror"],
	}
	No newline at end of file

core/tests/utiltests/jni/android_util_MemoryIntArrayTest.cpp

deleted100644 → 0

+0 −66

Original line number	Original line	Diff line number	Diff line
	/*
	* Copyright (C) 2016 The Android Open Source Project
	*
	* Licensed under the Apache License, Version 2.0 (the "License");
	* you may not use this file except in compliance with the License.
	* You may obtain a copy of the License at
	*
	* http://www.apache.org/licenses/LICENSE-2.0
	*
	* Unless required by applicable law or agreed to in writing, software
	* distributed under the License is distributed on an "AS IS" BASIS,
	* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
	* See the License for the specific language governing permissions and
	* limitations under the License.
	*/

	#include <atomic>
	#include <jni.h>
	#include <cutils/ashmem.h>
	#include <linux/ashmem.h>
	#include <sys/ioctl.h>
	#include <sys/mman.h>

	jint android_util_MemoryIntArrayTest_createAshmem(__attribute__((unused)) JNIEnv* env,
	__attribute__((unused)) jobject clazz,
	jstring name, jint size)
	{

	if (name == NULL) {
	return -1;
	}

	if (size < 0) {
	return -1;
	}

	const char* nameStr = env->GetStringUTFChars(name, NULL);
	const int ashmemSize = sizeof(std::atomic_int) * size;
	int fd = ashmem_create_region(nameStr, ashmemSize);
	env->ReleaseStringUTFChars(name, nameStr);

	if (fd < 0) {
	return -1;
	}

	int setProtResult = ashmem_set_prot_region(fd, PROT_READ \| PROT_WRITE);
	if (setProtResult < 0) {
	return -1;
	}

	return fd;
	}

	void android_util_MemoryIntArrayTest_setAshmemSize(__attribute__((unused)) JNIEnv* env,
	__attribute__((unused)) jobject clazz, jint fd, jint size)
	{
	if (fd < 0) {
	return;
	}

	if (size < 0) {
	return;
	}

	ioctl(fd, ASHMEM_SET_SIZE, size);
	}