From f3ad324a8d3f5b5530bd1945f461faf4b0adec8c Mon Sep 17 00:00:00 2001
From: John Reck <jreck@google.com>
Date: Wed, 24 Feb 2016 15:36:35 -0800
Subject: [PATCH] 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
---
 libs/hwui/PixelBuffer.cpp | 10 ++++++++--
 libs/hwui/PixelBuffer.h   | 16 ++++++++--------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/libs/hwui/PixelBuffer.cpp b/libs/hwui/PixelBuffer.cpp
index 6df994c623f23..165c7db4c85f0 100644
--- a/libs/hwui/PixelBuffer.cpp
+++ b/libs/hwui/PixelBuffer.cpp
@@ -36,12 +36,14 @@ public:
     CpuPixelBuffer(GLenum format, uint32_t width, uint32_t height);
 
     uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) override;
-    void unmap() override;
 
     uint8_t* getMappedPointer() const override;
 
     void upload(uint32_t x, uint32_t y, uint32_t width, uint32_t height, int offset) override;
 
+protected:
+    void unmap() override;
+
 private:
     std::unique_ptr<uint8_t[]> mBuffer;
 };
@@ -81,12 +83,14 @@ public:
     ~GpuPixelBuffer();
 
     uint8_t* map(AccessMode mode = kAccessMode_ReadWrite) override;
-    void unmap() override;
 
     uint8_t* getMappedPointer() const override;
 
     void upload(uint32_t x, uint32_t y, uint32_t width, uint32_t height, int offset) override;
 
+protected:
+    void unmap() override;
+
 private:
     GLuint mBuffer;
     uint8_t* mMappedPointer;
@@ -118,6 +122,7 @@ uint8_t* GpuPixelBuffer::map(AccessMode mode) {
             LOG_ALWAYS_FATAL("Failed to map PBO");
         }
         mAccessMode = mode;
+        mCaches.pixelBufferState().unbind();
     }
 
     return mMappedPointer;
@@ -147,6 +152,7 @@ void GpuPixelBuffer::upload(uint32_t x, uint32_t y, uint32_t width, uint32_t hei
     unmap();
     glTexSubImage2D(GL_TEXTURE_2D, 0, x, y, width, height, mFormat,
             GL_UNSIGNED_BYTE, reinterpret_cast<void*>(offset));
+    mCaches.pixelBufferState().unbind();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/libs/hwui/PixelBuffer.h b/libs/hwui/PixelBuffer.h
index aac5ec4777ee0..bbef36b72e4f3 100644
--- a/libs/hwui/PixelBuffer.h
+++ b/libs/hwui/PixelBuffer.h
@@ -90,14 +90,6 @@ public:
      */
     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
      * is not mapped, this method returns kAccessMode_None.
@@ -204,6 +196,14 @@ protected:
             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;
 
     uint32_t mWidth;