Fix vulnerability in MemoryIntArray (d08cf2b0) · Commits · e / os / android_frameworks_base

core/java/android/util/MemoryIntArray.java

+21 −37

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

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

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

	@@ -64,31 +63,24 @@ 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, boolean clientWritable) throws IOException {		public MemoryIntArray(int size) 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);
	}		}
	mOwnerPid = Process.myPid();		mIsOwner = true;
	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, true, clientWritable);		mMemoryAddr = nativeOpen(mFd, mIsOwner);
	}		}

	private MemoryIntArray(Parcel parcel) throws IOException {		private MemoryIntArray(Parcel parcel) throws IOException {
	mOwnerPid = parcel.readInt();		mIsOwner = false;
	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();
	final long memoryAddress = parcel.readLong();		mMemoryAddr = nativeOpen(mFd, mIsOwner);
	if (isOwner()) {
	mMemoryAddr = memoryAddress;
	} else {
	mMemoryAddr = nativeOpen(mFd, false, mClientWritable);
	}
	}		}

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

	/**		/**
	@@ -109,7 +101,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, isOwner());		return nativeGet(mFd, mMemoryAddr, index);
	}		}

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

	/**		/**
	@@ -146,7 +138,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, isOwner());		nativeClose(mFd, mMemoryAddr, mIsOwner);
	mFd = -1;		mFd = -1;
	}		}
	}		}
	@@ -173,10 +165,7 @@ 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();
	}		}
	@@ -202,10 +191,6 @@ 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");
	@@ -227,10 +212,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, boolean writable);		private native long nativeOpen(int fd, boolean owner);
	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, boolean owner);		private native int nativeGet(int fd, long memoryAddr, int index);
	private native void nativeSet(int fd, long memoryAddr, int index, int value, boolean owner);		private native void nativeSet(int fd, long memoryAddr, int index, int value);
	private native int nativeSize(int fd);		private native int nativeSize(int fd);

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

core/jni/android_util_MemoryIntArray.cpp

+24 −8

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

	int protMode = (owner \|\| writable) ? (PROT_READ \| PROT_WRITE) : PROT_READ;		// IMPORTANT: Ashmem allows the caller to change its size until
			// 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 && !writable) {		if (owner) {
	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");
	@@ -121,7 +137,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, jboolean owner)		jint fd, jlong address, jint index)
	{		{
	if (fd < 0) {		if (fd < 0) {
	jniThrowException(env, "java/io/IOException", "bad file descriptor");		jniThrowException(env, "java/io/IOException", "bad file descriptor");
	@@ -138,7 +154,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, jboolean owner)		jint fd, jlong address, jint index, jint newValue)
	{		{
	if (fd < 0) {		if (fd < 0) {
	jniThrowException(env, "java/io/IOException", "bad file descriptor");		jniThrowException(env, "java/io/IOException", "bad file descriptor");
	@@ -171,10 +187,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", "(IZZ)J", (void*)android_util_MemoryIntArray_open},		{"nativeOpen", "(IZ)J", (void*)android_util_MemoryIntArray_open},
	{"nativeClose", "(IJZ)V", (void*)android_util_MemoryIntArray_close},		{"nativeClose", "(IJZ)V", (void*)android_util_MemoryIntArray_close},
	{"nativeGet", "(IJIZ)I", (void*)android_util_MemoryIntArray_get},		{"nativeGet", "(IJI)I", (void*)android_util_MemoryIntArray_get},
	{"nativeSet", "(IJIIZ)V", (void*) android_util_MemoryIntArray_set},		{"nativeSet", "(IJII)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

+4 −0

Original line number	Original line	Diff line number	Diff line
	@@ -12,6 +12,8 @@ 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
	@@ -23,3 +25,5 @@ LOCAL_PACKAGE_NAME := FrameworksUtilTests
	LOCAL_CERTIFICATE := platform		LOCAL_CERTIFICATE := platform

	include $(BUILD_PACKAGE)		include $(BUILD_PACKAGE)

			include $(call all-makefiles-under,$(LOCAL_PATH))
			No newline at end of file

core/tests/utiltests/jni/Android.mk

0 → 100644

+32 −0

Original line number	Original line	Diff line number	Diff line
			# Copyright 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..

			LOCAL_PATH := $(call my-dir)
			include $(CLEAR_VARS)

			LOCAL_MODULE := libmemoryintarraytest

			LOCAL_SRC_FILES := \
			registration.cpp \
			android_util_MemoryIntArrayTest.cpp

			LOCAL_CFLAGS += -include bionic/libc/kernel/uapi/linux/types.h

			LOCAL_SHARED_LIBRARIES := libcutils

			LOCAL_CLANG := true

			LOCAL_CPPFLAGS := -Werror

			include $(BUILD_SHARED_LIBRARY)

core/tests/utiltests/jni/android_util_MemoryIntArrayTest.cpp

0 → 100644

+66 −0

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);
			}