Guard against out of bounds accesses caused by a large base level

A texture's base level can be any positive integer, but SwiftShader stores texture levels in a array that has an implementation dependent size (IMPLEMENTATION_MAX_TEXTURE_LEVELS). To avoid accessing this array out of bounds, a class was added to make sure all accesses to the array are done within its bounds. Change-Id: I9b439018f209a47371bd2959ba847345326964dd Reviewed-on: https://swiftshader-review.googlesource.com/20488 Tested-by: Alexis Hétu <sugoi@google.com> Tested-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com>
skia-flutter-autoroll · Sep 7, 2018 · cf47cfd · cf47cfd
1 parent 3655209
commit cf47cfd
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 65 deletions.
diff --git a/src/OpenGL/libGLESv2/Texture.cpp b/src/OpenGL/libGLESv2/Texture.cpp
@@ -420,11 +420,6 @@ bool Texture::isMipmapFiltered(Sampler *sampler) const
 
 Texture2D::Texture2D(GLuint name) : Texture(name)
 {
-	for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
-	{
-		image[i] = nullptr;
-	}
-
 	mSurface = nullptr;
 
 	mColorbufferProxy = nullptr;
@@ -433,14 +428,7 @@ Texture2D::Texture2D(GLuint name) : Texture(name)
 
 Texture2D::~Texture2D()
 {
-	for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
-	{
-		if(image[i])
-		{
-			image[i]->unbind(this);
-			image[i] = nullptr;
-		}
-	}
+	image.unbind(this);
 
 	if(mSurface)
 	{
@@ -562,14 +550,7 @@ void Texture2D::setImage(GLint level, GLsizei width, GLsizei height, GLint inter
 
 void Texture2D::bindTexImage(gl::Surface *surface)
 {
-	for(int level = 0; level < IMPLEMENTATION_MAX_TEXTURE_LEVELS; level++)
-	{
-		if(image[level])
-		{
-			image[level]->release();
-			image[level] = nullptr;
-		}
-	}
+	image.release();
 
 	image[0] = surface->getRenderTarget();
 
@@ -579,14 +560,7 @@ void Texture2D::bindTexImage(gl::Surface *surface)
 
 void Texture2D::releaseTexImage()
 {
-	for(int level = 0; level < IMPLEMENTATION_MAX_TEXTURE_LEVELS; level++)
-	{
-		if(image[level])
-		{
-			image[level]->release();
-			image[level] = nullptr;
-		}
-	}
+	image.release();
 
 	if(mSurface)
 	{
@@ -902,14 +876,6 @@ Renderbuffer *Texture2DRect::getRenderbuffer(GLenum target, GLint level)
 
 TextureCubeMap::TextureCubeMap(GLuint name) : Texture(name)
 {
-	for(int f = 0; f < 6; f++)
-	{
-		for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
-		{
-			image[f][i] = nullptr;
-		}
-	}
-
 	for(int f = 0; f < 6; f++)
 	{
 		mFaceProxies[f] = nullptr;
@@ -919,20 +885,9 @@ TextureCubeMap::TextureCubeMap(GLuint name) : Texture(name)
 
 TextureCubeMap::~TextureCubeMap()
 {
-	for(int f = 0; f < 6; f++)
-	{
-		for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
-		{
-			if(image[f][i])
-			{
-				image[f][i]->unbind(this);
-				image[f][i] = nullptr;
-			}
-		}
-	}
-
 	for(int i = 0; i < 6; i++)
 	{
+		image[i].unbind(this);
 		mFaceProxies[i] = nullptr;
 	}
 }
@@ -1440,11 +1395,6 @@ bool TextureCubeMap::isShared(GLenum target, unsigned int level) const
 
 Texture3D::Texture3D(GLuint name) : Texture(name)
 {
-	for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
-	{
-		image[i] = nullptr;
-	}
-
 	mSurface = nullptr;
 
 	mColorbufferProxy = nullptr;
@@ -1453,14 +1403,7 @@ Texture3D::Texture3D(GLuint name) : Texture(name)
 
 Texture3D::~Texture3D()
 {
-	for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
-	{
-		if(image[i])
-		{
-			image[i]->unbind(this);
-			image[i] = nullptr;
-		}
-	}
+	image.unbind(this);
 
 	if(mSurface)
 	{

diff --git a/src/OpenGL/libGLESv2/Texture.h b/src/OpenGL/libGLESv2/Texture.h
@@ -45,6 +45,54 @@ enum
 	IMPLEMENTATION_MAX_RENDERBUFFER_SIZE = sw::OUTLINE_RESOLUTION,
 };
 
+class ImageLevels
+{
+public:
+	inline const egl::Image* operator[](size_t index) const
+	{
+		return (index < IMPLEMENTATION_MAX_TEXTURE_LEVELS) ? image[index] : nullptr;
+	}
+
+	inline egl::Image*& operator[](size_t index)
+	{
+		if(index < IMPLEMENTATION_MAX_TEXTURE_LEVELS)
+		{
+			return image[index];
+		}
+
+		static egl::Image* nullImage;
+		nullImage = nullptr;
+		return nullImage;
+	}
+
+	inline void release()
+	{
+		for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
+		{
+			if(image[i])
+			{
+				image[i]->release();
+				image[i] = nullptr;
+			}
+		}
+	}
+
+	inline void unbind(const egl::Texture* texture)
+	{
+		for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
+		{
+			if(image[i])
+			{
+				image[i]->unbind(texture);
+				image[i] = nullptr;
+			}
+		}
+	}
+
+private:
+	egl::Image *image[IMPLEMENTATION_MAX_TEXTURE_LEVELS] = {};
+};
+
 class Texture : public egl::Texture
 {
 public:
@@ -192,7 +240,7 @@ class Texture2D : public Texture
 
 	bool isMipmapComplete() const;
 
-	egl::Image *image[IMPLEMENTATION_MAX_TEXTURE_LEVELS];
+	ImageLevels image;
 
 	gl::Surface *mSurface;
 
@@ -265,7 +313,7 @@ class TextureCubeMap : public Texture
 	// face is one of the GL_TEXTURE_CUBE_MAP_* enumerants. Returns nullptr on failure.
 	egl::Image *getImage(GLenum face, unsigned int level);
 
-	egl::Image *image[6][IMPLEMENTATION_MAX_TEXTURE_LEVELS];
+	ImageLevels image[6];
 
 	// A specific internal reference count is kept for colorbuffer proxy references,
 	// because, as the renderbuffer acting as proxy will maintain a binding pointer
@@ -321,7 +369,7 @@ class Texture3D : public Texture
 
 	bool isMipmapComplete() const;
 
-	egl::Image *image[IMPLEMENTATION_MAX_TEXTURE_LEVELS];
+	ImageLevels image;
 
 	gl::Surface *mSurface;