EGL: Add early exit when hitting invalid attribute.
According to the EGL spec, we stop checking attributes when we
encounter an invalid enum. That means it's valid for an application to
pass in a list of attributes without EGL_NONE as long as one of them
is invalid. To handle this, we add lazy attribute validation to the
AttributeMap class, that gets triggered in the validation calls.
We only implement the early exit validation for the config attributes
to fix an EGL test that would access out of bounds memory.
Bug: angleproject:6660
Change-Id: I264d0f98b4ddd9e74187846e9e668270a6fbaee1
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3262478
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jonah Ryan-Davis <jonahr@google.com>
diff --git a/src/libANGLE/AttributeMap.cpp b/src/libANGLE/AttributeMap.cpp
index 8519731..8bbcb73 100644
--- a/src/libANGLE/AttributeMap.cpp
+++ b/src/libANGLE/AttributeMap.cpp
@@ -21,25 +21,25 @@
void AttributeMap::insert(EGLAttrib key, EGLAttrib value)
{
- mAttributes[key] = value;
+ mValidatedAttributes[key] = value;
}
bool AttributeMap::contains(EGLAttrib key) const
{
- return (mAttributes.find(key) != mAttributes.end());
+ return (attribs().find(key) != attribs().end());
}
EGLAttrib AttributeMap::get(EGLAttrib key) const
{
- auto iter = mAttributes.find(key);
- ASSERT(iter != mAttributes.end());
+ auto iter = attribs().find(key);
+ ASSERT(iter != attribs().end());
return iter->second;
}
EGLAttrib AttributeMap::get(EGLAttrib key, EGLAttrib defaultValue) const
{
- auto iter = mAttributes.find(key);
- return (iter != mAttributes.end()) ? iter->second : defaultValue;
+ auto iter = attribs().find(key);
+ return (iter != attribs().end()) ? iter->second : defaultValue;
}
EGLint AttributeMap::getAsInt(EGLAttrib key) const
@@ -54,13 +54,13 @@
bool AttributeMap::isEmpty() const
{
- return mAttributes.empty();
+ return attribs().empty();
}
std::vector<EGLint> AttributeMap::toIntVector() const
{
std::vector<EGLint> ret;
- for (const auto &pair : mAttributes)
+ for (const auto &pair : attribs())
{
ret.push_back(static_cast<EGLint>(pair.first));
ret.push_back(static_cast<EGLint>(pair.second));
@@ -72,25 +72,65 @@
AttributeMap::const_iterator AttributeMap::begin() const
{
- return mAttributes.begin();
+ return attribs().begin();
}
AttributeMap::const_iterator AttributeMap::end() const
{
- return mAttributes.end();
+ return attribs().end();
+}
+
+bool AttributeMap::validate(
+ const ValidationContext *val,
+ const egl::Display *display,
+ std::function<bool(const ValidationContext *, const egl::Display *, EGLAttrib)> validationFunc)
+ const
+{
+ if (mIntPointer)
+ {
+ for (const EGLint *curAttrib = mIntPointer; curAttrib[0] != EGL_NONE; curAttrib += 2)
+ {
+ if (!validationFunc(val, display, curAttrib[0]))
+ {
+ return false;
+ }
+
+ mValidatedAttributes[static_cast<EGLAttrib>(curAttrib[0])] =
+ static_cast<EGLAttrib>(curAttrib[1]);
+ }
+ mIntPointer = nullptr;
+ }
+
+ if (mAttribPointer)
+ {
+ for (const EGLAttrib *curAttrib = mAttribPointer; curAttrib[0] != EGL_NONE; curAttrib += 2)
+ {
+ if (!validationFunc(val, display, curAttrib[0]))
+ {
+ return false;
+ }
+
+ mValidatedAttributes[curAttrib[0]] = curAttrib[1];
+ }
+ mAttribPointer = nullptr;
+ }
+
+ return true;
+}
+
+void AttributeMap::initializeWithoutValidation() const
+{
+ auto alwaysTrue = [](const ValidationContext *, const egl::Display *, EGLAttrib) {
+ return true;
+ };
+ validate(nullptr, nullptr, alwaysTrue);
}
// static
AttributeMap AttributeMap::CreateFromIntArray(const EGLint *attributes)
{
AttributeMap map;
- if (attributes)
- {
- for (const EGLint *curAttrib = attributes; curAttrib[0] != EGL_NONE; curAttrib += 2)
- {
- map.insert(static_cast<EGLAttrib>(curAttrib[0]), static_cast<EGLAttrib>(curAttrib[1]));
- }
- }
+ map.mIntPointer = attributes;
return map;
}
@@ -98,13 +138,12 @@
AttributeMap AttributeMap::CreateFromAttribArray(const EGLAttrib *attributes)
{
AttributeMap map;
- if (attributes)
- {
- for (const EGLAttrib *curAttrib = attributes; curAttrib[0] != EGL_NONE; curAttrib += 2)
- {
- map.insert(curAttrib[0], curAttrib[1]);
- }
- }
+ map.mAttribPointer = attributes;
return map;
}
+
+bool AttributeMap::isValidated() const
+{
+ return mIntPointer == nullptr && mAttribPointer == nullptr;
+}
} // namespace egl
diff --git a/src/libANGLE/AttributeMap.h b/src/libANGLE/AttributeMap.h
index 4034189..e516181 100644
--- a/src/libANGLE/AttributeMap.h
+++ b/src/libANGLE/AttributeMap.h
@@ -11,11 +11,14 @@
#include <EGL/egl.h>
+#include <functional>
#include <map>
#include <vector>
namespace egl
{
+class Display;
+struct ValidationContext;
class AttributeMap final
{
@@ -42,8 +45,8 @@
template <typename PackedEnumT>
PackedEnumT getAsPackedEnum(EGLAttrib key, PackedEnumT defaultValue) const
{
- auto iter = mAttributes.find(key);
- return (mAttributes.find(key) != mAttributes.end())
+ auto iter = attribs().find(key);
+ return (attribs().find(key) != attribs().end())
? FromEGLenum<PackedEnumT>(static_cast<EGLenum>(iter->second))
: defaultValue;
}
@@ -56,11 +59,35 @@
const_iterator begin() const;
const_iterator end() const;
+ bool validate(const ValidationContext *val,
+ const egl::Display *display,
+ std::function<bool(const ValidationContext *, const Display *, EGLAttrib)>
+ validationFunc) const;
+
+ // TODO: remove this and validate at every call site. http://anglebug.com/6671
+ void initializeWithoutValidation() const;
+
static AttributeMap CreateFromIntArray(const EGLint *attributes);
static AttributeMap CreateFromAttribArray(const EGLAttrib *attributes);
private:
- std::map<EGLAttrib, EGLAttrib> mAttributes;
+ bool isValidated() const;
+
+ const std::map<EGLAttrib, EGLAttrib> &attribs() const
+ {
+ ASSERT(isValidated());
+ return mValidatedAttributes;
+ }
+
+ std::map<EGLAttrib, EGLAttrib> &attribs()
+ {
+ ASSERT(isValidated());
+ return mValidatedAttributes;
+ }
+
+ mutable const EGLint *mIntPointer = nullptr;
+ mutable const EGLAttrib *mAttribPointer = nullptr;
+ mutable std::map<EGLAttrib, EGLAttrib> mValidatedAttributes;
};
} // namespace egl
diff --git a/src/libANGLE/validationEGL.cpp b/src/libANGLE/validationEGL.cpp
index 6741fad..affc262 100644
--- a/src/libANGLE/validationEGL.cpp
+++ b/src/libANGLE/validationEGL.cpp
@@ -400,11 +400,13 @@
const Display *display,
const AttributeMap &attributes)
{
+ ANGLE_VALIDATION_TRY(attributes.validate(val, display, ValidateConfigAttribute));
+
for (const auto &attrib : attributes)
{
- ANGLE_VALIDATION_TRY(ValidateConfigAttribute(val, display, attrib.first));
- ANGLE_VALIDATION_TRY(
- ValidateConfigAttributeValue(val, display, attrib.first, attrib.second));
+ EGLAttrib pname = attrib.first;
+ EGLAttrib value = attrib.second;
+ ANGLE_VALIDATION_TRY(ValidateConfigAttributeValue(val, display, pname, value));
}
return true;
@@ -553,6 +555,8 @@
return false;
}
+ attribMap.initializeWithoutValidation();
+
if (platform == EGL_PLATFORM_ANGLE_ANGLE)
{
EGLAttrib platformType = EGL_PLATFORM_ANGLE_TYPE_DEFAULT_ANGLE;
@@ -1223,6 +1227,8 @@
{
ANGLE_VALIDATION_TRY(ValidateDisplay(val, display));
+ attribs.initializeWithoutValidation();
+
gl::Context *currentContext = val->eglThread->getContext();
egl::Display *currentDisplay = currentContext ? currentContext->getDisplay() : nullptr;
@@ -1649,6 +1655,8 @@
}
}
+ attributes.initializeWithoutValidation();
+
// Get the requested client version (default is 1) and check it is 2 or 3.
EGLAttrib clientMajorVersion = 1;
EGLAttrib clientMinorVersion = 0;
@@ -2146,6 +2154,8 @@
const DisplayExtensions &displayExtensions = display->getExtensions();
+ attributes.initializeWithoutValidation();
+
for (const auto &attributeIter : attributes)
{
EGLAttrib attribute = attributeIter.first;
@@ -2319,6 +2329,8 @@
const DisplayExtensions &displayExtensions = display->getExtensions();
+ attributes.initializeWithoutValidation();
+
for (const auto &attributeIter : attributes)
{
EGLAttrib attribute = attributeIter.first;
@@ -2464,6 +2476,8 @@
const DisplayExtensions &displayExtensions = display->getExtensions();
+ attributes.initializeWithoutValidation();
+
switch (buftype)
{
case EGL_D3D_TEXTURE_2D_SHARE_HANDLE_ANGLE:
@@ -2770,11 +2784,12 @@
const DisplayExtensions &displayExtensions = display->getExtensions();
- for (AttributeMap::const_iterator attributeIter = attributes.begin();
- attributeIter != attributes.end(); attributeIter++)
+ attributes.initializeWithoutValidation();
+
+ for (const auto &attributePair : attributes)
{
- EGLAttrib attribute = attributeIter->first;
- EGLAttrib value = attributeIter->second;
+ EGLAttrib attribute = attributePair.first;
+ EGLAttrib value = attributePair.second;
switch (attribute)
{
@@ -2968,6 +2983,8 @@
ANGLE_VALIDATION_TRY(ValidateDisplay(val, display));
+ attributes.initializeWithoutValidation();
+
const DisplayExtensions &displayExtensions = display->getExtensions();
// TODO(geofflang): Complete validation from EGL_KHR_image_base:
@@ -3842,6 +3859,8 @@
return false;
}
+ attributes.initializeWithoutValidation();
+
for (const auto &attributeIter : attributes)
{
EGLAttrib attribute = attributeIter.first;
@@ -4122,6 +4141,9 @@
{
plane[i] = -1;
}
+
+ attribs.initializeWithoutValidation();
+
for (const auto &attributeIter : attribs)
{
EGLAttrib attribute = attributeIter.first;
@@ -4262,6 +4284,8 @@
ANGLE_VALIDATION_TRY(ValidateStream(val, display, stream));
+ attribs.initializeWithoutValidation();
+
if (!attribs.isEmpty())
{
val->setError(EGL_BAD_ATTRIBUTE, "Invalid attribute");
@@ -4317,6 +4341,8 @@
ANGLE_VALIDATION_TRY(ValidateStream(val, display, stream));
+ attribs.initializeWithoutValidation();
+
for (auto &attributeIter : attribs)
{
EGLAttrib attribute = attributeIter.first;
@@ -5307,6 +5333,8 @@
return false;
}
+ attribs.initializeWithoutValidation();
+
for (const auto &attrib : attribs)
{
switch (attrib.first)
@@ -5644,6 +5672,8 @@
bool ValidateCreateNativeClientBufferANDROID(const ValidationContext *val,
const egl::AttributeMap &attribMap)
{
+ attribMap.initializeWithoutValidation();
+
if (attribMap.isEmpty() || attribMap.begin()->second == EGL_NONE)
{
val->setError(EGL_BAD_PARAMETER, "invalid attribute list.");
diff --git a/src/tests/deqp_support/deqp_egl_test_expectations.txt b/src/tests/deqp_support/deqp_egl_test_expectations.txt
index 798b4a9..8b8b275 100644
--- a/src/tests/deqp_support/deqp_egl_test_expectations.txt
+++ b/src/tests/deqp_support/deqp_egl_test_expectations.txt
@@ -68,7 +68,6 @@
1340 WIN : dEQP-EGL.functional.render.multi_context.gles3.rgba5551_pbuffer = FAIL
1340 WIN : dEQP-EGL.functional.render.multi_context.gles2_gles3.rgba5551_window = FAIL
1340 WIN : dEQP-EGL.functional.render.multi_context.gles2_gles3.rgba5551_pbuffer = FAIL
-1340 WIN : dEQP-EGL.functional.negative_api.choose_config = SKIP
1340 WIN : dEQP-EGL.functional.negative_api.surface_attrib = SKIP
1340 WIN : dEQP-EGL.functional.negative_api.swap_interval = FAIL
2382 WIN : dEQP-EGL.functional.native_color_mapping.native_window.* = SKIP
@@ -91,7 +90,6 @@
2546 LINUX : dEQP-EGL.functional.thread_cleanup.* = SKIP
2546 LINUX : dEQP-EGL.functional.native_color_mapping.native_window.* = FAIL
2546 LINUX : dEQP-EGL.functional.native_coord_mapping.native_window.* = FAIL
-2546 LINUX : dEQP-EGL.functional.negative_api.choose_config = FAIL
2546 LINUX : dEQP-EGL.functional.negative_api.swap_interval = FAIL
2546 LINUX : dEQP-EGL.functional.query_surface.simple.pbuffer.rgba8888_depth_stencil = FAIL
2546 LINUX : dEQP-EGL.functional.query_surface.simple.pbuffer.rgba8888_no_depth_no_stencil = FAIL
@@ -103,7 +101,6 @@
// Mac failures
2546 MAC : dEQP-EGL.functional.native_color_mapping.native_window.* = FAIL
2546 MAC : dEQP-EGL.functional.native_coord_mapping.native_window.* = FAIL
-2546 MAC : dEQP-EGL.functional.negative_api.choose_config = FAIL
2546 MAC : dEQP-EGL.functional.negative_api.copy_buffers = FAIL
2546 MAC : dEQP-EGL.functional.negative_api.swap_interval = FAIL
2546 MAC : dEQP-EGL.functional.query_surface.simple.pbuffer.rgba8888_depth_stencil = FAIL
