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

core/java/android/util/MemoryIntArray.java

+22 −37

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

		private static final int MAX_SIZE = 1024;

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

		@@ -64,31 +63,25 @@ public final class MemoryIntArray implements Parcelable, Closeable {
		* @param clientWritable Whether other processes can write to the array.
		* @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) {
		throw new IllegalArgumentException("Max size is " + MAX_SIZE);
		}
		mOwnerPid = Process.myPid();
		mClientWritable = clientWritable;
		mIsOwner = true;
		final String name = UUID.randomUUID().toString();
		mFd = nativeCreate(name, size);
		mMemoryAddr = nativeOpen(mFd, true, clientWritable);
		mMemoryAddr = nativeOpen(mFd, mIsOwner);
		}

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

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

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

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

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

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

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

		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 int nativeGet(int fd, long memoryAddr, int index, boolean owner);
		private native void nativeSet(int fd, long memoryAddr, int index, int value, boolean owner);
		private native int nativeGet(int fd, long memoryAddr, int index);
		private native void nativeSet(int fd, long memoryAddr, int index, int value);
		private native int nativeSize(int fd);

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

core/jni/android_util_MemoryIntArray.cpp

+24 −8

Original line number	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,
		jboolean owner, jboolean writable)
		jboolean owner)
		{
		if (fd < 0) {
		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;
		}

		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);
		if (ashmemAddr == MAP_FAILED) {
		jniThrowException(env, "java/io/IOException", "cannot mmap ashmem");
		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) {
		int size = ashmemSize / sizeof(std::atomic_int);
		new (ashmemAddr) std::atomic_int[size];
		}

		if (owner && !writable) {
		if (owner) {
		int setProtResult = ashmem_set_prot_region(fd, PROT_READ);
		if (setProtResult < 0) {
		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,
		jint fd, jlong address, jint index, jboolean owner)
		jint fd, jlong address, jint index)
		{
		if (fd < 0) {
		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,
		jint fd, jlong address, jint index, jint newValue, jboolean owner)
		jint fd, jlong address, jint index, jint newValue)
		{
		if (fd < 0) {
		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[] = {
		{"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},
		{"nativeGet", "(IJIZ)I", (void*)android_util_MemoryIntArray_get},
		{"nativeSet", "(IJIIZ)V", (void*) android_util_MemoryIntArray_set},
		{"nativeGet", "(IJI)I", (void*)android_util_MemoryIntArray_get},
		{"nativeSet", "(IJII)V", (void*) android_util_MemoryIntArray_set},
		{"nativeSize", "(I)I", (void*) android_util_MemoryIntArray_size},
		};

core/tests/utiltests/Android.mk

+2 −0

Original line number	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 += src/android/util/IRemoteMemoryIntArray.aidl

		LOCAL_JNI_SHARED_LIBRARIES := libmemoryintarraytest libcutils libc++

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

core/tests/utiltests/jni/Android.bp

0 → 100644

+27 −0

Original line number	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

0 → 100644

+66 −0

Original line number	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);
		}