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

Commit f3ad324a authored by John Reck's avatar John Reck
Browse files

Aggressively unbind GL_PIXEL_UNPACK_BUFFER 

Bug: 27186019

Theory: It appears to be possible for FontRenderer
to not unbind its PBO prior to textures being uploaded,
resulting in trying to glSubTexImage2D with a bound
GL_PIXEL_UNPACK_BUFFER. In that scenario the void* is
the offset into the PBO which given a non-null data
will almost certainly overrun the end of the buffer. This
in turn produces a GL_INVALID_OPERATION error.

Change PixelBuffer to avoid leaking this state for now.
This will result in more calls to glBindBuffer/glUnbindBuffer
in the worst case, but the worst case is already bad so this
shouldn't be a problem. In the normal case we avoid binding
the PBO at all ever, so this doesn't impact that.

Change-Id: I05473f0d2f9a3a5da0e33d8f9ddea4731ce970e3
parent 8baf238a

libs/hwui/PixelBuffer.cpp

+8 −2
Original line number Original line Diff line number Diff line
@@ -36,12 +36,14 @@ public: 
    CpuPixelBuffer(GLenum format, uint32_t width, uint32_t height);

    CpuPixelBuffer(GLenum format, uint32_t width, uint32_t height);





    uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) override;

    uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) override;

    void unmap() override;





    uint8_t* getMappedPointer() const override;

    uint8_t* getMappedPointer() const override;





    void upload(uint32_t x, uint32_t y, uint32_t width, uint32_t height, int offset) override;

    void upload(uint32_t x, uint32_t y, uint32_t width, uint32_t height, int offset) override;





protected:

    void unmap() override;



private:

private:

    std::unique_ptr<uint8_t[]> mBuffer;

    std::unique_ptr<uint8_t[]> mBuffer;

};

};
@@ -81,12 +83,14 @@ public: 
    ~GpuPixelBuffer();

    ~GpuPixelBuffer();





    uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) override;

    uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) override;

    void unmap() override;





    uint8_t* getMappedPointer() const override;

    uint8_t* getMappedPointer() const override;





    void upload(uint32_t x, uint32_t y, uint32_t width, uint32_t height, int offset) override;

    void upload(uint32_t x, uint32_t y, uint32_t width, uint32_t height, int offset) override;





protected:

    void unmap() override;



private:

private:

    GLuint mBuffer;

    GLuint mBuffer;

    uint8_t* mMappedPointer;

    uint8_t* mMappedPointer;
@@ -118,6 +122,7 @@ uint8_t* GpuPixelBuffer::map(AccessMode mode) { 
            LOG_ALWAYS_FATAL("Failed to map PBO");

            LOG_ALWAYS_FATAL("Failed to map PBO");

        }

        }

        mAccessMode = mode;

        mAccessMode = mode;

        mCaches.pixelBufferState().unbind();

    }

    }





    return mMappedPointer;

    return mMappedPointer;
@@ -147,6 +152,7 @@ void GpuPixelBuffer::upload(uint32_t x, uint32_t y, uint32_t width, uint32_t hei 
    unmap();

    unmap();

    glTexSubImage2D(GL_TEXTURE_2D, 0, x, y, width, height, mFormat,

    glTexSubImage2D(GL_TEXTURE_2D, 0, x, y, width, height, mFormat,

            GL_UNSIGNED_BYTE, reinterpret_cast<void*>(offset));

            GL_UNSIGNED_BYTE, reinterpret_cast<void*>(offset));

    mCaches.pixelBufferState().unbind();

}

}





///////////////////////////////////////////////////////////////////////////////

///////////////////////////////////////////////////////////////////////////////

libs/hwui/PixelBuffer.h

+8 −8
Original line number Original line Diff line number Diff line
@@ -90,14 +90,6 @@ public: 
     */

     */

    virtual uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) = 0;

    virtual uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) = 0;





    /**

     * Unmaps this buffer, if needed. After the buffer is unmapped,

     * the pointer previously returned by map() becomes invalid and

     * should not be used. After calling this method, getMappedPointer()

     * will always return NULL.

     */

    virtual void unmap() = 0;



    /**

    /**

     * Returns the current access mode for this buffer. If the buffer

     * Returns the current access mode for this buffer. If the buffer

     * is not mapped, this method returns kAccessMode_None.

     * is not mapped, this method returns kAccessMode_None.
@@ -204,6 +196,14 @@ protected: 
            mFormat(format), mWidth(width), mHeight(height), mAccessMode(kAccessMode_None) {

            mFormat(format), mWidth(width), mHeight(height), mAccessMode(kAccessMode_None) {

    }

    }





    /**

     * Unmaps this buffer, if needed. After the buffer is unmapped,

     * the pointer previously returned by map() becomes invalid and

     * should not be used. After calling this method, getMappedPointer()

     * will always return NULL.

     */

    virtual void unmap() = 0;



    GLenum mFormat;

    GLenum mFormat;





    uint32_t mWidth;

    uint32_t mWidth;