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

Commit 0a52509e authored by Ady Abraham's avatar Ady Abraham
Browse files

ISurfaceComposer: boundary check input on CAPTURE_LAYERS 

Add a sanity check on numExcludeHandles to make sure we don't cause
an overflow.

Test: adb shell /data/nativetest64/SurfaceFlinger_test/SurfaceFlinger_test
Fixes: 146435753
Change-Id: I2c700392727e2f4e0e434fb4c1800f2973c7418b
Merged-In: I2c700392727e2f4e0e434fb4c1800f2973c7418b
parent 5fad64e0

libs/gui/ISurfaceComposer.cpp

+3 −0
Original line number Diff line number Diff line
@@ -1281,6 +1281,9 @@ status_t BnSurfaceComposer::onTransact( 


            std::unordered_set<sp<IBinder>, SpHash<IBinder>> excludeHandles;

            int numExcludeHandles = data.readInt32();

            if (numExcludeHandles >= static_cast<int>(MAX_LAYERS)) {

                return BAD_VALUE;

            }

            excludeHandles.reserve(numExcludeHandles);

            for (int i = 0; i < numExcludeHandles; i++) {

                excludeHandles.emplace(data.readStrongBinder());

libs/gui/include/gui/ISurfaceComposer.h

+2 −0
Original line number Diff line number Diff line
@@ -76,6 +76,8 @@ class ISurfaceComposer: public IInterface { 
public:

    DECLARE_META_INTERFACE(SurfaceComposer)



    static constexpr size_t MAX_LAYERS = 4096;



    // flags for setTransactionState()

    enum {

        eSynchronous = 0x01,

services/surfaceflinger/SurfaceFlinger.cpp

+3 −3
Original line number Diff line number Diff line
@@ -367,7 +367,7 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI 
    property_get("ro.sf.blurs_are_expensive", value, "0");

    mBlursAreExpensive = atoi(value);



    const size_t defaultListSize = MAX_LAYERS;

    const size_t defaultListSize = ISurfaceComposer::MAX_LAYERS;

    auto listSize = property_get_int32("debug.sf.max_igbp_list_size", int32_t(defaultListSize));

    mMaxGraphicBufferProducerListSize = (listSize > 0) ? size_t(listSize) : defaultListSize;
@@ -3066,9 +3066,9 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind 
            parent = parentLayer;

        }



        if (mNumLayers >= MAX_LAYERS) {

        if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) {

            ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),

                  MAX_LAYERS);

                  ISurfaceComposer::MAX_LAYERS);

            return NO_MEMORY;

        }

services/surfaceflinger/SurfaceFlinger.h

+1 −2
Original line number Diff line number Diff line
@@ -351,7 +351,6 @@ private: 
    // every half hour.

    enum { LOG_FRAME_STATS_PERIOD =  30*60*60 };



    static const size_t MAX_LAYERS = 4096;

    static const int MAX_TRACING_MEMORY = 100 * 1024 * 1024; // 100MB



protected:
@@ -976,7 +975,7 @@ private: 


    // Can't be unordered_set because wp<> isn't hashable

    std::set<wp<IBinder>> mGraphicBufferProducerList;

    size_t mMaxGraphicBufferProducerListSize = MAX_LAYERS;

    size_t mMaxGraphicBufferProducerListSize = ISurfaceComposer::MAX_LAYERS;



    // protected by mStateLock (but we could use another lock)

    bool mLayersRemoved = false;