Skip to content

A
angle
Commit 901b379f by Jamie Madill Committed by Commit Bot
Options

Fix use-after-free when deleting share contexts.

The pattern of gen context, share context, free context, then allocate a shared GL object in the second context would cause a use-after-free of the ContextImpl as a GLFactory. Fix this by passing the factory as a parameter to the resource manager allocation methods instead of storing the factory pointer. This allows the same ResourceManager to work with separate Context implementations, which will work with non-virtual contexts. BUG=612931 Change-Id: Ifceeb893bebd072f318963d935ff9d17181f5305 Reviewed-on: https://chromium-review.googlesource.com/347463Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org>
parent 9e54b5af
Showing with 148 additions and 50 deletions
src/libANGLE/Context.cpp
......@@ -162,7 +162,7 @@ Context::Context(rx::EGLImplFactory *implFactory,
}
else
{
mResourceManager = new ResourceManager(mImplementation.get());
mResourceManager = new ResourceManager();
}
mData.resourceManager = mResourceManager;
......@@ -411,12 +411,13 @@ GLuint Context::createBuffer()
GLuint Context::createProgram()
{
return mResourceManager->createProgram();
return mResourceManager->createProgram(mImplementation.get());
}
GLuint Context::createShader(GLenum type)
{
return mResourceManager->createShader(mImplementation->getNativeLimitations(), type);
return mResourceManager->createShader(mImplementation.get(),
mImplementation->getNativeLimitations(), type);
}
GLuint Context::createTexture()
......@@ -431,7 +432,7 @@ GLuint Context::createRenderbuffer()
GLsync Context::createFenceSync()
{
GLuint handle = mResourceManager->createFenceSync();
GLuint handle = mResourceManager->createFenceSync(mImplementation.get());
return reinterpret_cast<GLsync>(static_cast<uintptr_t>(handle));
}
......@@ -706,13 +707,13 @@ bool Context::isSampler(GLuint samplerName) const
void Context::bindArrayBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setArrayBufferBinding(buffer);
}
void Context::bindElementArrayBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.getVertexArray()->setElementArrayBuffer(buffer);
}
......@@ -726,7 +727,7 @@ void Context::bindTexture(GLenum target, GLuint handle)
}
else
{
texture = mResourceManager->checkTextureAllocation(handle, target);
texture = mResourceManager->checkTextureAllocation(mImplementation.get(), handle, target);
}
ASSERT(texture);
......@@ -747,7 +748,8 @@ void Context::bindDrawFramebuffer(GLuint framebufferHandle)
void Context::bindRenderbuffer(GLuint renderbufferHandle)
{
Renderbuffer *renderbuffer = mResourceManager->checkRenderbufferAllocation(renderbufferHandle);
Renderbuffer *renderbuffer =
mResourceManager->checkRenderbufferAllocation(mImplementation.get(), renderbufferHandle);
mState.setRenderbufferBinding(renderbuffer);
}
......@@ -760,13 +762,14 @@ void Context::bindVertexArray(GLuint vertexArrayHandle)
void Context::bindSampler(GLuint textureUnit, GLuint samplerHandle)
{
ASSERT(textureUnit < mCaps.maxCombinedTextureImageUnits);
Sampler *sampler = mResourceManager->checkSamplerAllocation(samplerHandle);
Sampler *sampler =
mResourceManager->checkSamplerAllocation(mImplementation.get(), samplerHandle);
mState.setSamplerBinding(textureUnit, sampler);
}
void Context::bindGenericUniformBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setGenericUniformBufferBinding(buffer);
}
......@@ -775,13 +778,13 @@ void Context::bindIndexedUniformBuffer(GLuint bufferHandle,
GLintptr offset,
GLsizeiptr size)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setIndexedUniformBufferBinding(index, buffer, offset, size);
}
void Context::bindGenericTransformFeedbackBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.getCurrentTransformFeedback()->bindGenericBuffer(buffer);
}
......@@ -790,31 +793,31 @@ void Context::bindIndexedTransformFeedbackBuffer(GLuint bufferHandle,
GLintptr offset,
GLsizeiptr size)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.getCurrentTransformFeedback()->bindIndexedBuffer(index, buffer, offset, size);
}
void Context::bindCopyReadBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setCopyReadBufferBinding(buffer);
}
void Context::bindCopyWriteBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setCopyWriteBufferBinding(buffer);
}
void Context::bindPixelPackBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setPixelPackBufferBinding(buffer);
}
void Context::bindPixelUnpackBuffer(GLuint bufferHandle)
{
Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setPixelUnpackBufferBinding(buffer);
}
......@@ -1910,7 +1913,7 @@ void Context::setVertexAttribDivisor(GLuint index, GLuint divisor)
void Context::samplerParameteri(GLuint sampler, GLenum pname, GLint param)
{
mResourceManager->checkSamplerAllocation(sampler);
mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
......@@ -1935,7 +1938,7 @@ void Context::samplerParameteri(GLuint sampler, GLenum pname, GLint param)
void Context::samplerParameterf(GLuint sampler, GLenum pname, GLfloat param)
{
mResourceManager->checkSamplerAllocation(sampler);
mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
......@@ -1960,7 +1963,7 @@ void Context::samplerParameterf(GLuint sampler, GLenum pname, GLfloat param)
GLint Context::getSamplerParameteri(GLuint sampler, GLenum pname)
{
mResourceManager->checkSamplerAllocation(sampler);
mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
......@@ -1985,7 +1988,7 @@ GLint Context::getSamplerParameteri(GLuint sampler, GLenum pname)
GLfloat Context::getSamplerParameterf(GLuint sampler, GLenum pname)
{
mResourceManager->checkSamplerAllocation(sampler);
mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
......
src/libANGLE/ResourceManager.cpp
......@@ -20,7 +20,7 @@
namespace gl
{
ResourceManager::ResourceManager(rx::GLImplFactory *factory) : mFactory(factory), mRefCount(1)
ResourceManager::ResourceManager() : mRefCount(1)
{
}
......@@ -86,13 +86,15 @@ GLuint ResourceManager::createBuffer()
}
// Returns an unused shader/program name
GLuint ResourceManager::createShader(const gl::Limitations &rendererLimitations, GLenum type)
GLuint ResourceManager::createShader(rx::GLImplFactory *factory,
const gl::Limitations &rendererLimitations,
GLenum type)
{
GLuint handle = mProgramShaderHandleAllocator.allocate();
if (type == GL_VERTEX_SHADER || type == GL_FRAGMENT_SHADER)
{
mShaderMap[handle] = new Shader(this, mFactory, rendererLimitations, type, handle);
mShaderMap[handle] = new Shader(this, factory, rendererLimitations, type, handle);
}
else UNREACHABLE();
......@@ -100,11 +102,11 @@ GLuint ResourceManager::createShader(const gl::Limitations &rendererLimitations,
}
// Returns an unused program/shader name
GLuint ResourceManager::createProgram()
GLuint ResourceManager::createProgram(rx::GLImplFactory *factory)
{
GLuint handle = mProgramShaderHandleAllocator.allocate();
mProgramMap[handle] = new Program(mFactory, this, handle);
mProgramMap[handle] = new Program(factory, this, handle);
return handle;
}
......@@ -140,11 +142,11 @@ GLuint ResourceManager::createSampler()
}
// Returns the next unused fence name, and allocates the fence
GLuint ResourceManager::createFenceSync()
GLuint ResourceManager::createFenceSync(rx::GLImplFactory *factory)
{
GLuint handle = mFenceSyncHandleAllocator.allocate();
FenceSync *fenceSync = new FenceSync(mFactory->createFenceSync(), handle);
FenceSync *fenceSync = new FenceSync(factory->createFenceSync(), handle);
fenceSync->addRef();
mFenceSyncMap[handle] = fenceSync;
......@@ -355,7 +357,7 @@ void ResourceManager::setRenderbuffer(GLuint handle, Renderbuffer *buffer)
mRenderbufferMap[handle] = buffer;
}
Buffer *ResourceManager::checkBufferAllocation(GLuint handle)
Buffer *ResourceManager::checkBufferAllocation(rx::GLImplFactory *factory, GLuint handle)
{
if (handle == 0)
{
......@@ -370,7 +372,7 @@ Buffer *ResourceManager::checkBufferAllocation(GLuint handle)
return bufferMapIt->second;
}
Buffer *buffer = new Buffer(mFactory->createBuffer(), handle);
Buffer *buffer = new Buffer(factory->createBuffer(), handle);
buffer->addRef();
if (handleAllocated)
......@@ -386,7 +388,9 @@ Buffer *ResourceManager::checkBufferAllocation(GLuint handle)
return buffer;
}
Texture *ResourceManager::checkTextureAllocation(GLuint handle, GLenum type)
Texture *ResourceManager::checkTextureAllocation(rx::GLImplFactory *factory,
GLuint handle,
GLenum type)
{
if (handle == 0)
{
......@@ -401,7 +405,7 @@ Texture *ResourceManager::checkTextureAllocation(GLuint handle, GLenum type)
return textureMapIt->second;
}
Texture *texture = new Texture(mFactory, handle, type);
Texture *texture = new Texture(factory, handle, type);
texture->addRef();
if (handleAllocated)
......@@ -417,7 +421,8 @@ Texture *ResourceManager::checkTextureAllocation(GLuint handle, GLenum type)
return texture;
}
Renderbuffer *ResourceManager::checkRenderbufferAllocation(GLuint handle)
Renderbuffer *ResourceManager::checkRenderbufferAllocation(rx::GLImplFactory *factory,
GLuint handle)
{
if (handle == 0)
{
......@@ -432,7 +437,7 @@ Renderbuffer *ResourceManager::checkRenderbufferAllocation(GLuint handle)
return renderbufferMapIt->second;
}
Renderbuffer *renderbuffer = new Renderbuffer(mFactory->createRenderbuffer(), handle);
Renderbuffer *renderbuffer = new Renderbuffer(factory->createRenderbuffer(), handle);
renderbuffer->addRef();
if (handleAllocated)
......@@ -448,7 +453,7 @@ Renderbuffer *ResourceManager::checkRenderbufferAllocation(GLuint handle)
return renderbuffer;
}
Sampler *ResourceManager::checkSamplerAllocation(GLuint samplerHandle)
Sampler *ResourceManager::checkSamplerAllocation(rx::GLImplFactory *factory, GLuint samplerHandle)
{
// Samplers cannot be created via Bind
if (samplerHandle == 0)
......@@ -460,7 +465,7 @@ Sampler *ResourceManager::checkSamplerAllocation(GLuint samplerHandle)
if (!sampler)
{
sampler = new Sampler(mFactory, samplerHandle);
sampler = new Sampler(factory, samplerHandle);
mSamplerMap[samplerHandle] = sampler;
sampler->addRef();
}
......
src/libANGLE/ResourceManager.h
......@@ -34,19 +34,21 @@ class Texture;
class ResourceManager : angle::NonCopyable
{
public:
explicit ResourceManager(rx::GLImplFactory *factory);
ResourceManager();
~ResourceManager();
void addRef();
void release();
GLuint createBuffer();
GLuint createShader(const gl::Limitations &rendererLimitations, GLenum type);
GLuint createProgram();
GLuint createShader(rx::GLImplFactory *factory,
const gl::Limitations &rendererLimitations,
GLenum type);
GLuint createProgram(rx::GLImplFactory *factory);
GLuint createTexture();
GLuint createRenderbuffer();
GLuint createSampler();
GLuint createFenceSync();
GLuint createFenceSync(rx::GLImplFactory *factory);
void deleteBuffer(GLuint buffer);
void deleteShader(GLuint shader);
......@@ -66,17 +68,17 @@ class ResourceManager : angle::NonCopyable
void setRenderbuffer(GLuint handle, Renderbuffer *renderbuffer);
Buffer *checkBufferAllocation(GLuint handle);
Texture *checkTextureAllocation(GLuint handle, GLenum type);
Renderbuffer *checkRenderbufferAllocation(GLuint handle);
Sampler *checkSamplerAllocation(GLuint samplerHandle);
Buffer *checkBufferAllocation(rx::GLImplFactory *factory, GLuint handle);
Texture *checkTextureAllocation(rx::GLImplFactory *factory, GLuint handle, GLenum type);
Renderbuffer *checkRenderbufferAllocation(rx::GLImplFactory *factory, GLuint handle);
Sampler *checkSamplerAllocation(rx::GLImplFactory *factory, GLuint samplerHandle);
bool isSampler(GLuint sampler);
private:
void createTextureInternal(GLuint handle);
rx::GLImplFactory *mFactory;
;
std::size_t mRefCount;
ResourceMap<Buffer> mBufferMap;
......
src/libANGLE/ResourceManager_unittest.cpp
......@@ -25,7 +25,7 @@ class ResourceManagerTest : public testing::Test
protected:
void SetUp() override
{
mResourceManager = new ResourceManager(&mMockFactory);
mResourceManager = new ResourceManager();
}
void TearDown() override
......@@ -41,7 +41,7 @@ TEST_F(ResourceManagerTest, ReallocateBoundTexture)
{
EXPECT_CALL(mMockFactory, createTexture(_)).Times(1).RetiresOnSaturation();
mResourceManager->checkTextureAllocation(1, GL_TEXTURE_2D);
mResourceManager->checkTextureAllocation(&mMockFactory, 1, GL_TEXTURE_2D);
GLuint newTexture = mResourceManager->createTexture();
EXPECT_NE(1u, newTexture);
}
......@@ -50,7 +50,7 @@ TEST_F(ResourceManagerTest, ReallocateBoundBuffer)
{
EXPECT_CALL(mMockFactory, createBuffer()).Times(1).RetiresOnSaturation();
mResourceManager->checkBufferAllocation(1);
mResourceManager->checkBufferAllocation(&mMockFactory, 1);
GLuint newBuffer = mResourceManager->createBuffer();
EXPECT_NE(1u, newBuffer);
}
......@@ -59,9 +59,9 @@ TEST_F(ResourceManagerTest, ReallocateBoundRenderbuffer)
{
EXPECT_CALL(mMockFactory, createRenderbuffer()).Times(1).RetiresOnSaturation();
mResourceManager->checkRenderbufferAllocation(1);
mResourceManager->checkRenderbufferAllocation(&mMockFactory, 1);
GLuint newRenderbuffer = mResourceManager->createRenderbuffer();
EXPECT_NE(1u, newRenderbuffer);
}
}
} // anonymous namespace
src/tests/angle_end2end_tests.gypi
......@@ -73,6 +73,7 @@
'<(angle_path)/src/tests/gl_tests/VertexAttributeTest.cpp',
'<(angle_path)/src/tests/gl_tests/ViewportTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLContextCompatibilityTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLContextSharingTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLQueryContextTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLSanityCheckTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLSurfaceTest.cpp',
......
src/tests/egl_tests/EGLContextSharingTest.cpp 0 → 100644
//
// Copyright (c) 2016 The ANGLE Project Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// EGLContextSharingTest.cpp:
// Tests relating to shared Contexts.
#include <gtest/gtest.h>
#include <EGL/egl.h>
#include <EGL/eglext.h>
#include "test_utils/ANGLETest.h"
#include "test_utils/angle_test_configs.h"
using namespace angle;
namespace
{
EGLBoolean SafeDestroyContext(EGLDisplay display, EGLContext &context)
{
EGLBoolean result = EGL_TRUE;
if (context != EGL_NO_CONTEXT)
{
result = eglDestroyContext(display, context);
context = EGL_NO_CONTEXT;
}
return result;
}
class EGLContextSharingTest : public ANGLETest
{
public:
EGLContextSharingTest() : mContexts{EGL_NO_CONTEXT, EGL_NO_CONTEXT}, mTexture(0) {}
void TearDown() override
{
glDeleteTextures(1, &mTexture);
EGLDisplay display = getEGLWindow()->getDisplay();
if (display != EGL_NO_DISPLAY)
{
for (auto &context : mContexts)
{
SafeDestroyContext(display, context);
}
}
ANGLETest::TearDown();
}
EGLContext mContexts[2];
GLuint mTexture;
};
// Tests that creating resources works after freeing the share context.
TEST_P(EGLContextSharingTest, BindTextureAfterShareContextFree)
{
EGLDisplay display = getEGLWindow()->getDisplay();
EGLConfig config = getEGLWindow()->getConfig();
EGLSurface surface = getEGLWindow()->getSurface();
const EGLint contextAttribs[] = {EGL_CONTEXT_CLIENT_VERSION,
getEGLWindow()->getClientMajorVersion(), EGL_NONE};
mContexts[0] = eglCreateContext(display, config, nullptr, contextAttribs);
ASSERT_EGL_SUCCESS();
ASSERT_TRUE(mContexts[0] != EGL_NO_CONTEXT);
mContexts[1] = eglCreateContext(display, config, mContexts[1], contextAttribs);
ASSERT_EGL_SUCCESS();
ASSERT_TRUE(mContexts[1] != EGL_NO_CONTEXT);
ASSERT_EGL_TRUE(SafeDestroyContext(display, mContexts[0]));
ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mContexts[1]));
ASSERT_EGL_SUCCESS();
glGenTextures(1, &mTexture);
glBindTexture(GL_TEXTURE_2D, mTexture);
ASSERT_GL_NO_ERROR();
}
} // anonymous namespace
ANGLE_INSTANTIATE_TEST(EGLContextSharingTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL());
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment