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

Commit 6aff9050 authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix a major bug in Bundle when unparcelling from AIDL. 

There was a serious problem in the Bundle(Parcel) and readFromParcel() methods,
where it wasn't doing the copying of the Parcel that Parcel.readBundle() does
and is a basic requirement for it to work correctly.

This re-arranges the code to make all of these functions (hopefully) correct.

Also fix a problem in Parcel where we were not duping fds when copying data from
one Parcel to another.
parent 9681a5e0

core/java/android/os/Bundle.java

+58 −10
Original line number Diff line number Diff line
@@ -78,6 +78,10 @@ public final class Bundle implements Parcelable, Cloneable { 
        readFromParcel(parcelledData);

    }



    /* package */ Bundle(Parcel parcelledData, int length) {

        readFromParcelInner(parcelledData, length);

    }



    /**

     * Constructs a new, empty Bundle that uses a specific ClassLoader for

     * instantiating Parcelable and Serializable objects.
@@ -155,13 +159,14 @@ public final class Bundle implements Parcelable, Cloneable { 
            return;

        }



        mParcelledData.setDataPosition(0);

        Bundle b = mParcelledData.readBundleUnpacked(mClassLoader);

        mMap = b.mMap;



        mHasFds = mParcelledData.hasFileDescriptors();

        mFdsKnown = true;

        

        int N = mParcelledData.readInt();

        if (N < 0) {

            return;

        }

        if (mMap == null) {

            mMap = new HashMap<String, Object>();

        }

        mParcelledData.readMapInternal(mMap, N, mClassLoader);

        mParcelledData.recycle();

        mParcelledData = null;

    }
@@ -1427,7 +1432,25 @@ public final class Bundle implements Parcelable, Cloneable { 
     * @param parcel The parcel to copy this bundle to.

     */

    public void writeToParcel(Parcel parcel, int flags) {

        parcel.writeBundle(this);

        if (mParcelledData != null) {

            int length = mParcelledData.dataSize();

            parcel.writeInt(length);

            parcel.writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'

            parcel.appendFrom(mParcelledData, 0, length);

        } else {

            parcel.writeInt(-1); // dummy, will hold length

            parcel.writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'



            int oldPos = parcel.dataPosition();

            parcel.writeMapInternal(mMap);

            int newPos = parcel.dataPosition();



            // Backpatch length

            parcel.setDataPosition(oldPos - 8);

            int length = newPos - oldPos;

            parcel.writeInt(length);

            parcel.setDataPosition(newPos);

        }

    }



    /**
@@ -1436,8 +1459,33 @@ public final class Bundle implements Parcelable, Cloneable { 
     * @param parcel The parcel to overwrite this bundle from.

     */

    public void readFromParcel(Parcel parcel) {

        mParcelledData = parcel;

        mHasFds = mParcelledData.hasFileDescriptors();

        int length = parcel.readInt();

        if (length < 0) {

            throw new RuntimeException("Bad length in parcel: " + length);

        }

        readFromParcelInner(parcel, length);

    }



    void readFromParcelInner(Parcel parcel, int length) {

        int magic = parcel.readInt();

        if (magic != 0x4C444E42) {

            //noinspection ThrowableInstanceNeverThrown

            String st = Log.getStackTraceString(new RuntimeException());

            Log.e("Bundle", "readBundle: bad magic number");

            Log.e("Bundle", "readBundle: trace = " + st);

        }



        // Advance within this Parcel

        int offset = parcel.dataPosition();

        parcel.setDataPosition(offset + length);



        Parcel p = Parcel.obtain();

        p.setDataPosition(0);

        p.appendFrom(parcel, offset, length);

        p.setDataPosition(0);

        

        mParcelledData = p;

        mHasFds = p.hasFileDescriptors();

        mFdsKnown = true;

    }

core/java/android/os/Parcel.java

+5 −63
Original line number Diff line number Diff line
@@ -457,7 +457,7 @@ public final class Parcel { 
     * Flatten a Map into the parcel at the current dataPosition(),

     * growing dataCapacity() if needed.  The Map keys must be String objects.

     */

    private void writeMapInternal(Map<String,Object> val) {

    /* package */ void writeMapInternal(Map<String,Object> val) {

        if (val == null) {

            writeInt(-1);

            return;
@@ -480,23 +480,7 @@ public final class Parcel { 
            return;

        }



        if (val.mParcelledData != null) {

            int length = val.mParcelledData.dataSize();

            appendFrom(val.mParcelledData, 0, length);

        } else {

            writeInt(-1); // dummy, will hold length

            int oldPos = dataPosition();

            writeInt(0x4C444E42); // 'B' 'N' 'D' 'L'



            writeMapInternal(val.mMap);

            int newPos = dataPosition();



            // Backpatch length

            setDataPosition(oldPos - 4);

            int length = newPos - oldPos;

            writeInt(length);

            setDataPosition(newPos);

        }

        val.writeToParcel(this, 0);

    }



    /**
@@ -1352,60 +1336,18 @@ public final class Parcel { 
     * Returns null if the previously written Bundle object was null.

     */

    public final Bundle readBundle(ClassLoader loader) {

        int offset = dataPosition();

        int length = readInt();

        if (length < 0) {

            return null;

        }

        int magic = readInt();

        if (magic != 0x4C444E42) {

            //noinspection ThrowableInstanceNeverThrown

            String st = Log.getStackTraceString(new RuntimeException());

            Log.e("Bundle", "readBundle: bad magic number");

            Log.e("Bundle", "readBundle: trace = " + st);

        }



        // Advance within this Parcel

        setDataPosition(offset + length + 4);

        

        Parcel p = new Parcel(0);

        p.setDataPosition(0);

        p.appendFrom(this, offset, length + 4);

        p.setDataPosition(0);

        final Bundle bundle = new Bundle(p);

        final Bundle bundle = new Bundle(this, length);

        if (loader != null) {

            bundle.setClassLoader(loader);

        }

        return bundle;

    }



    /**

     * Read and return a new Bundle object from the parcel at the current

     * dataPosition().  Returns null if the previously written Bundle object was

     * null.  The returned bundle will have its contents fully unpacked using

     * the given ClassLoader.

     */

    /* package */ Bundle readBundleUnpacked(ClassLoader loader) {

        int length = readInt();

        if (length == -1) {

            return null;

        }

        int magic = readInt();

        if (magic != 0x4C444E42) {

            //noinspection ThrowableInstanceNeverThrown

            String st = Log.getStackTraceString(new RuntimeException());

            Log.e("Bundle", "readBundleUnpacked: bad magic number");

            Log.e("Bundle", "readBundleUnpacked: trace = " + st);

        }

        Bundle m = new Bundle(loader);

        int N = readInt();

        if (N < 0) {

            return null;

        }

        readMapInternal(m.mMap, N, loader);

        return m;

    }



    /**

     * Read and return a byte[] object from the parcel.

     */
@@ -1998,7 +1940,7 @@ public final class Parcel { 
    private native void init(int obj);

    private native void destroy();



    private void readMapInternal(Map outVal, int N,

    /* package */ void readMapInternal(Map outVal, int N,

        ClassLoader loader) {

        while (N > 0) {

            Object key = readValue(loader);

libs/utils/Parcel.cpp

+6 −2
Original line number Diff line number Diff line
@@ -409,12 +409,16 @@ status_t Parcel::appendFrom(Parcel *parcel, size_t offset, size_t len) 
            mObjects[idx++] = off;

            mObjectsSize++;



            const flat_binder_object* flat

            flat_binder_object* flat

                = reinterpret_cast<flat_binder_object*>(mData + off);

            acquire_object(proc, *flat, this);



            // take note if the object is a file descriptor

            if (flat->type == BINDER_TYPE_FD) {

                // If this is a file descriptor, we need to dup it so the

                // new Parcel now owns its own fd, and can declare that we

                // officially know we have fds.

                flat->handle = dup(flat->handle);

                flat->cookie = (void*)1;

                mHasFds = mFdsKnown = true;

            }

        }