Revert "Fix vulnerability in MemoryIntArray" (29139a8a) · Commits · e / os / android_frameworks_base

core/java/android/util/MemoryIntArray.java

+37 −22

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.
		* 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.
		* 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.
		* </p>
		*
		* @hide
		@@ -51,7 +51,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {

		private static final int MAX_SIZE = 1024;

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

		@@ -63,25 +64,31 @@ 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) throws IOException {
		public MemoryIntArray(int size, boolean clientWritable) throws IOException {
		if (size > MAX_SIZE) {
		throw new IllegalArgumentException("Max size is " + MAX_SIZE);
		}
		mIsOwner = true;
		mOwnerPid = Process.myPid();
		mClientWritable = clientWritable;
		final String name = UUID.randomUUID().toString();
		mFd = nativeCreate(name, size);
		mMemoryAddr = nativeOpen(mFd, mIsOwner);
		mMemoryAddr = nativeOpen(mFd, true, clientWritable);
		}

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

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

		/**
		@@ -102,7 +109,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
		public int get(int index) throws IOException {
		enforceNotClosed();
		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();
		enforceWritable();
		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
		public void close() throws IOException {
		if (!isClosed()) {
		nativeClose(mFd, mMemoryAddr, mIsOwner);
		nativeClose(mFd, mMemoryAddr, isOwner());
		mFd = -1;
		}
		}
		@@ -166,7 +173,10 @@ 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();
		}
		@@ -192,6 +202,10 @@ 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");
		@@ -213,10 +227,10 @@ public final class MemoryIntArray implements Parcelable, Closeable {
		}

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

		/**
		@@ -233,7 +247,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {
		try {
		return new MemoryIntArray(parcel);
		} 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	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 owner, jboolean writable)
		{
		if (fd < 0) {
		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;
		}

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

core/tests/utiltests/Android.mk

+0 −2

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

deleted100644 → 0

+0 −27

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

deleted100644 → 0

+0 −66

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