drgreen/android_frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix vulnerability in MemoryIntArray

Browse Source 
am: 1181f448c1

Change-Id: I4217066be49bb9525e945f110c22eb864ec6c212
This commit is contained in:
Svetoslav Ganov
2016-12-09 01:43:52 +00:00
committed by android-build-merger
parent 498547ec6c 1181f448c1
commit d08cf2b071
11 changed files with 291 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
core/java/android/util/MemoryIntArray.java
View File

@ -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,24 @@ 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);
}
/**
@ -96,7 +88,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
*/
public boolean isWritable() {
enforceNotClosed();
return isOwner() || mClientWritable;
return mIsOwner;
}
/**
@ -109,7 +101,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 +117,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 +138,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 +165,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 +191,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 +212,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 +232,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");
}
}

32
core/jni/android_util_MemoryIntArray.cpp
View File

@ -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},
};

4
core/tests/utiltests/Android.mk
View File

@ -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
@ -23,3 +25,5 @@ LOCAL_PACKAGE_NAME := FrameworksUtilTests
LOCAL_CERTIFICATE := platform
include $(BUILD_PACKAGE)
include $(call all-makefiles-under,$(LOCAL_PATH))

32
core/tests/utiltests/jni/Android.mk Normal file
View File

@ -0,0 +1,32 @@
# 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)

66
core/tests/utiltests/jni/android_util_MemoryIntArrayTest.cpp Normal file
View File

@ -0,0 +1,66 @@
/*
* 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);
}

43
core/tests/utiltests/jni/registration.cpp Normal file
View File

@ -0,0 +1,43 @@
/*
* 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 <jni.h>
extern jint android_util_MemoryIntArrayTest_createAshmem(JNIEnv* env,
jobject clazz, jstring name, jint size);
extern void android_util_MemoryIntArrayTest_setAshmemSize(JNIEnv* env,
jobject clazz, jint fd, jint size);
extern "C" {
JNIEXPORT jint JNICALL Java_android_util_MemoryIntArrayTest_nativeCreateAshmem(
JNIEnv * env, jobject obj, jstring name, jint size);
JNIEXPORT void JNICALL Java_android_util_MemoryIntArrayTest_nativeSetAshmemSize(
JNIEnv * env, jobject obj, jint fd, jint size);
};
JNIEXPORT jint JNICALL Java_android_util_MemoryIntArrayTest_nativeCreateAshmem(
__attribute__((unused)) JNIEnv * env,__attribute__((unused)) jobject obj,
jstring name, jint size)
{
return android_util_MemoryIntArrayTest_createAshmem(env, obj, name, size);
}
JNIEXPORT void JNICALL Java_android_util_MemoryIntArrayTest_nativeSetAshmemSize(
__attribute__((unused)) JNIEnv * env,__attribute__((unused)) jobject obj,
jint fd, jint size)
{
android_util_MemoryIntArrayTest_setAshmemSize(env, obj, fd, size);
}

3
core/tests/utiltests/src/android/util/IRemoteMemoryIntArray.aidl
View File

@ -20,11 +20,12 @@ import android.util.MemoryIntArray;
interface IRemoteMemoryIntArray {
MemoryIntArray peekInstance();
void create(int size, boolean clientWritable);
void create(int size);
boolean isWritable();
int get(int index);
void set(int index, int value);
int size();
void close();
boolean isClosed();
void accessLastElementInRemoteProcess(in MemoryIntArray array);
}

86
core/tests/utiltests/src/android/util/MemoryIntArrayTest.java
View File

@ -28,14 +28,22 @@ import libcore.io.IoUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import java.lang.reflect.Field;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
@RunWith(AndroidJUnit4.class)
public class MemoryIntArrayTest {
static {
System.loadLibrary("cutils");
System.loadLibrary("memoryintarraytest");
}
@Test
public void testSize() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(3, false);
array = new MemoryIntArray(3);
assertEquals("size must be three", 3, array.size());
} finally {
IoUtils.closeQuietly(array);
@ -46,7 +54,7 @@ public class MemoryIntArrayTest {
public void testGetSet() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(3, false);
array = new MemoryIntArray(3);
array.set(0, 1);
array.set(1, 2);
@ -64,7 +72,7 @@ public class MemoryIntArrayTest {
public void testWritable() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(3, true);
array = new MemoryIntArray(3);
assertTrue("Must be mutable", array.isWritable());
} finally {
IoUtils.closeQuietly(array);
@ -75,7 +83,7 @@ public class MemoryIntArrayTest {
public void testClose() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(3, false);
array = new MemoryIntArray(3);
array.close();
assertTrue("Must be closed", array.isClosed());
} finally {
@ -90,7 +98,7 @@ public class MemoryIntArrayTest {
MemoryIntArray firstArray = null;
MemoryIntArray secondArray = null;
try {
firstArray = new MemoryIntArray(3, false);
firstArray = new MemoryIntArray(3);
firstArray.set(0, 1);
firstArray.set(1, 2);
@ -117,7 +125,7 @@ public class MemoryIntArrayTest {
public void testInteractOnceClosed() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(3, false);
array = new MemoryIntArray(3);
array.close();
array.close();
@ -160,7 +168,7 @@ public class MemoryIntArrayTest {
public void testInteractPutOfBounds() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(3, false);
array = new MemoryIntArray(3);
try {
array.get(-1);
@ -198,7 +206,7 @@ public class MemoryIntArrayTest {
public void testOverMaxSize() throws Exception {
MemoryIntArray array = null;
try {
array = new MemoryIntArray(MemoryIntArray.getMaxSize() + 1, false);
array = new MemoryIntArray(MemoryIntArray.getMaxSize() + 1);
fail("Cannot use over max size");
} catch (IllegalArgumentException e) {
/* expected */
@ -209,7 +217,7 @@ public class MemoryIntArrayTest {
@Test
public void testNotMutableByUnprivilegedClients() throws Exception {
RemoteIntArray remoteIntArray = new RemoteIntArray(1, false);
RemoteIntArray remoteIntArray = new RemoteIntArray(1);
try {
assertNotNull("Couldn't get remote instance", remoteIntArray);
MemoryIntArray localIntArray = remoteIntArray.peekInstance();
@ -230,4 +238,64 @@ public class MemoryIntArrayTest {
remoteIntArray.destroy();
}
}
@Test
public void testAshmemSizeMatchesMemoryIntArraySize() throws Exception {
boolean success = false;
// Get a handle to a remote process to send the fd
RemoteIntArray remoteIntArray = new RemoteIntArray(1);
try {
// Let us try 100 times
for (int i = 0; i < 100; i++) {
// Create a MemoryIntArray to muck with
MemoryIntArray array = new MemoryIntArray(1);
// Create the fd to stuff in the MemoryIntArray
final int fd = nativeCreateAshmem("foo", 1);
// Replace the fd with our ahsmem region
Field fdFiled = MemoryIntArray.class.getDeclaredField("mFd");
fdFiled.setAccessible(true);
fdFiled.set(array, fd);
CountDownLatch countDownLatch = new CountDownLatch(2);
new Thread() {
@Override
public void run() {
for (int i = 2; i < Integer.MAX_VALUE; i++) {
if (countDownLatch.getCount() == 1) {
countDownLatch.countDown();
return;
}
nativeSetAshmemSize(fd, i);
}
}
}.start();
try {
remoteIntArray.accessLastElementInRemoteProcess(array);
} catch (IllegalArgumentException e) {
success = true;
}
countDownLatch.countDown();
countDownLatch.await(1000, TimeUnit.MILLISECONDS);
if (success) {
break;
}
}
} finally {
remoteIntArray.destroy();
}
if (!success) {
fail("MemoryIntArray should catch ahshmem size changing under it");
}
}
private native int nativeCreateAshmem(String name, int size);
private native void nativeSetAshmemSize(int fd, int size);
}

12
core/tests/utiltests/src/android/util/RemoteIntArray.java
View File

@ -40,7 +40,7 @@ final class RemoteIntArray implements ServiceConnection, Closeable {
private android.util.IRemoteMemoryIntArray mRemoteInstance;
public RemoteIntArray(int size, boolean clientWritable) throws IOException, TimeoutException {
public RemoteIntArray(int size) throws IOException, TimeoutException {
mIntent.setComponent(new ComponentName(InstrumentationRegistry.getContext(),
RemoteMemoryIntArrayService.class));
synchronized (mLock) {
@ -48,7 +48,7 @@ final class RemoteIntArray implements ServiceConnection, Closeable {
bindLocked();
}
try {
mRemoteInstance.create(size, clientWritable);
mRemoteInstance.create(size);
} catch (RemoteException e) {
throw new IOException(e);
}
@ -148,6 +148,14 @@ final class RemoteIntArray implements ServiceConnection, Closeable {
}
}
public void accessLastElementInRemoteProcess(MemoryIntArray array) {
try {
mRemoteInstance.accessLastElementInRemoteProcess(array);
} catch (RemoteException e) {
throw new RuntimeException(e);
}
}
@Override
public void onServiceConnected(ComponentName name, IBinder service) {
synchronized (mLock) {

13
core/tests/utiltests/src/android/util/RemoteMemoryIntArrayService.java
View File

@ -35,10 +35,10 @@ public class RemoteMemoryIntArrayService extends Service {
return new android.util.IRemoteMemoryIntArray.Stub() {
@Override
public void create(int size, boolean clientWritable) {
public void create(int size) {
synchronized (mLock) {
try {
mArray = new MemoryIntArray(size, clientWritable);
mArray = new MemoryIntArray(size);
} catch (IOException e) {
throw new IllegalStateException(e);
}
@ -109,6 +109,15 @@ public class RemoteMemoryIntArrayService extends Service {
return mArray.isClosed();
}
}
@Override
public void accessLastElementInRemoteProcess(MemoryIntArray array) {
try {
array.get(array.size() - 1);
} catch (IOException e) {
throw new IllegalStateException(e);
}
}
};
}
}

2
packages/SettingsProvider/src/com/android/providers/settings/GenerationRegistry.java
View File

@ -119,7 +119,7 @@ final class GenerationRegistry {
// and twice max user count for system and secure.
final int size = 1 + 2 + 10 + 2 * UserManager.getMaxSupportedUsers();
try {
mBackingStore = new MemoryIntArray(size, false);
mBackingStore = new MemoryIntArray(size);
} catch (IOException e) {
Slog.e(LOG_TAG, "Error creating generation tracker", e);
}