Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 04df738b authored by Svetoslav Ganov's avatar Svetoslav Ganov Committed by Svet Ganov
Browse files

Make settings cahches generation mechanism robust. 

Settings is using a MemoryIntArray to communicate the settings table
version enabling apps to have up-to-date local caches. However, ashmem
allows an arbitrary process with a handle to the fd (even in read only
mode) to unpin the memory which can then be garbage collected. Here we
make this mechanism fault tolerant against bad apps unpinning the ashmem
region. First, we no longer unpin the ashmem on the client side and if
the ashmem region is purged and cannot be pinned we recreate it and
hook up again with the local app caches. The change also adds a test
that clients can only read while owner can read/write.

bug:28764789

Change-Id: I1ef79b4b21e976124b268c9126a55d614157059b
parent ba83e90e

core/java/android/provider/Settings.java

+32 −2
Original line number Diff line number Diff line
@@ -1462,12 +1462,15 @@ public final class Settings { 


    private static final class GenerationTracker {

        private final MemoryIntArray mArray;

        private final Runnable mErrorHandler;

        private final int mIndex;

        private int mCurrentGeneration;



        public GenerationTracker(@NonNull MemoryIntArray array, int index) {

        public GenerationTracker(@NonNull MemoryIntArray array, int index,

                Runnable errorHandler) {

            mArray = array;

            mIndex = index;

            mErrorHandler = errorHandler;

            mCurrentGeneration = readCurrentGeneration();

        }
@@ -1487,9 +1490,23 @@ public final class Settings { 
                return mArray.get(mIndex);

            } catch (IOException e) {

                Log.e(TAG, "Error getting current generation", e);

                if (mErrorHandler != null) {

                    mErrorHandler.run();

                }

            }

            return -1;

        }



        public void destroy() {

            try {

                mArray.close();

            } catch (IOException e) {

                Log.e(TAG, "Error closing backing array", e);

                if (mErrorHandler != null) {

                    mErrorHandler.run();

                }

            }

        }

    }



    // Thread-safe.
@@ -1616,7 +1633,20 @@ public final class Settings { 
                                                    + cr.getPackageName() + " and user:"

                                                    + userHandle + " with index:" + index);

                                        }

                                        mGenerationTracker = new GenerationTracker(array, index);

                                        mGenerationTracker = new GenerationTracker(array, index,

                                                () -> {

                                            synchronized (this) {

                                                Log.e(TAG, "Error accessing generation"

                                                        + " tracker - removing");

                                                if (mGenerationTracker != null) {

                                                    GenerationTracker generationTracker =

                                                            mGenerationTracker;

                                                    mGenerationTracker = null;

                                                    generationTracker.destroy();

                                                    mValues.clear();

                                                }

                                            }

                                        });

                                    }

                                }

                                mValues.put(name, value);

core/java/android/util/MemoryIntArray.java

+8 −4
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.os.Parcel; 
import android.os.ParcelFileDescriptor;

import android.os.Parcelable;

import android.os.Process;

import libcore.io.IoUtils;



import java.io.Closeable;

import java.io.IOException;
@@ -46,6 +47,8 @@ import java.util.UUID; 
 * @hide

 */

public final class MemoryIntArray implements Parcelable, Closeable {

    private static final String TAG = "MemoryIntArray";



    private static final int MAX_SIZE = 1024;



    private final int mOwnerPid;
@@ -142,8 +145,9 @@ public final class MemoryIntArray implements Parcelable, Closeable { 
    @Override

    public void close() throws IOException {

        if (!isClosed()) {

            nativeClose(mFd.getFd(), mMemoryAddr, isOwner());

            ParcelFileDescriptor pfd = mFd;

            mFd = null;

            nativeClose(pfd.getFd(), mMemoryAddr, isOwner());

        }

    }
@@ -156,7 +160,7 @@ public final class MemoryIntArray implements Parcelable, Closeable { 


    @Override

    protected void finalize() throws Throwable {

        close();

        IoUtils.closeQuietly(this);

        super.finalize();

    }
@@ -230,7 +234,6 @@ public final class MemoryIntArray implements Parcelable, Closeable { 
    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);

    private native static int nativeGetMemoryPageSize();



    /**

     * @return The max array size.
@@ -246,7 +249,8 @@ public final class MemoryIntArray implements Parcelable, Closeable { 
            try {

                return new MemoryIntArray(parcel);

            } catch (IOException ioe) {

                throw new RuntimeException(ioe);

                Log.e(TAG, "Error unparceling MemoryIntArray");

                return null;

            }

        }

core/jni/android_util_MemoryIntArray.cpp

+8 −33
Original line number Diff line number Diff line
@@ -14,9 +14,9 @@ 
 * limitations under the License.

 */





#include "core_jni_helpers.h"

#include <cutils/ashmem.h>

#include <linux/ashmem.h>

#include <sys/mman.h>



namespace android {
@@ -44,11 +44,6 @@ static jint android_util_MemoryIntArray_create(JNIEnv* env, jobject clazz, jstri 
        return -1;

    }



    if (ashmem_pin_region(fd, 0, 0) == ASHMEM_WAS_PURGED) {

        jniThrowException(env, "java/io/IOException", "ashmem was purged");

        return -1;

    }



    int setProtResult = ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE);

    if (setProtResult < 0) {

        jniThrowException(env, "java/io/IOException", "cannot set ashmem prot mode");
@@ -133,24 +128,13 @@ static jint android_util_MemoryIntArray_get(JNIEnv* env, jobject clazz, 
        return -1;

    }



    bool unpin = false;



    if (!owner) {

    if (ashmem_pin_region(fd, 0, 0) == ASHMEM_WAS_PURGED) {

        jniThrowException(env, "java/io/IOException", "ashmem region was purged");

        return -1;

    }

        unpin = true;

    }



    std::atomic_int* value = reinterpret_cast<std::atomic_int*>(address) + index;

    const int result = value->load(std::memory_order_relaxed);



    if (unpin) {

        ashmem_unpin_region(fd, 0, 0);

    }



    return result;

    return value->load(std::memory_order_relaxed);

}



static void android_util_MemoryIntArray_set(JNIEnv* env, jobject clazz,
@@ -161,22 +145,13 @@ static void android_util_MemoryIntArray_set(JNIEnv* env, jobject clazz, 
        return;

    }



    bool unpin = false;



    if (!owner) {

    if (ashmem_pin_region(fd, 0, 0) == ASHMEM_WAS_PURGED) {

        jniThrowException(env, "java/io/IOException", "ashmem region was purged");

        return;

    }

        unpin = true;

    }



    std::atomic_int* value = reinterpret_cast<std::atomic_int*>(address) + index;

    value->store(newValue, std::memory_order_relaxed);



    if (unpin) {

        ashmem_unpin_region(fd, 0, 0);

    }

}



static jint android_util_MemoryIntArray_size(JNIEnv* env, jobject clazz, jint fd) {

core/tests/utiltests/Android.mk

+1 −0
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ LOCAL_MODULE_TAGS := tests 


# Include all test java files.

LOCAL_SRC_FILES := $(call all-java-files-under, src)

LOCAL_SRC_FILES += src/android/util/IRemoteMemoryIntArray.aidl



LOCAL_STATIC_JAVA_LIBRARIES := \

    android-support-test \

core/tests/utiltests/AndroidManifest.xml

+5 −0
Original line number Diff line number Diff line
@@ -43,6 +43,11 @@ 


    <application>

        <uses-library android:name="android.test.runner" />



        <service android:name="android.util.RemoteMemoryIntArrayService"

                android:process=":remote">

        </service>



    </application>



    <instrumentation