Fix potential OOB problem with validating command decoder
Bug: 1392715
Change-Id: If51b10cc08e5b3ca4b6012b97261347a5e4c134e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4049230
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Reviewed-by: Srinivas Sista <srinivassista@chromium.org>
Owners-Override: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/5434@{#3}
Cr-Branched-From: 2c5178f0e7f0c1caca91c57f4d782cc0e9d5e6fa-refs/heads/main@{#1074591}
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 7ac1fc8..036abc5f9 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -8558,10 +8558,18 @@
service_id = texture_ref->service_id();
}
+ bool valid_target = false;
+ if (texture_ref) {
+ valid_target = texture_manager()->ValidForTextureTarget(
+ texture_ref->texture(), level, 0, 0, 1);
+ } else {
+ valid_target = texture_manager()->ValidForTarget(textarget, level, 0, 0, 1);
+ }
+
if ((level > 0 && !feature_info_->IsWebGL2OrES3Context() &&
!(fbo_render_mipmap_explicitly_enabled_ &&
feature_info_->feature_flags().oes_fbo_render_mipmap)) ||
- !texture_manager()->ValidForTarget(textarget, level, 0, 0, 1)) {
+ !valid_target) {
LOCAL_SET_GL_ERROR(
GL_INVALID_VALUE,
name, "level out of range");
@@ -8633,8 +8641,8 @@
"texture is neither TEXTURE_3D nor TEXTURE_2D_ARRAY");
return;
}
- if (!texture_manager()->ValidForTarget(texture_target, level,
- 0, 0, layer)) {
+ if (!texture_manager()->ValidForTextureTarget(texture_ref->texture(), level,
+ 0, 0, layer)) {
LOCAL_SET_GL_ERROR(
GL_INVALID_VALUE, function_name, "invalid level or layer");
return;
@@ -14686,11 +14694,6 @@
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "imageSize < 0");
return error::kNoError;
}
- if (!texture_manager()->ValidForTarget(target, level, width, height, depth) ||
- border != 0) {
- LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "dimensions out of range");
- return error::kNoError;
- }
TextureRef* texture_ref = texture_manager()->GetTextureInfoForTarget(
&state_, target);
if (!texture_ref) {
@@ -14699,6 +14702,12 @@
return error::kNoError;
}
Texture* texture = texture_ref->texture();
+ if (!texture_manager()->ValidForTextureTarget(texture, level, width, height,
+ depth) ||
+ border != 0) {
+ LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "dimensions out of range");
+ return error::kNoError;
+ }
if (texture->IsImmutable()) {
LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name, "texture is immutable");
return error::kNoError;
@@ -15068,10 +15077,6 @@
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "imageSize < 0");
return error::kNoError;
}
- if (!texture_manager()->ValidForTarget(target, level, width, height, depth)) {
- LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "dimensions out of range");
- return error::kNoError;
- }
TextureRef* texture_ref = texture_manager()->GetTextureInfoForTarget(
&state_, target);
if (!texture_ref) {
@@ -15079,7 +15084,14 @@
GL_INVALID_OPERATION, func_name, "no texture bound at target");
return error::kNoError;
}
+
Texture* texture = texture_ref->texture();
+ if (!texture_manager()->ValidForTextureTarget(texture, level, width, height,
+ depth)) {
+ LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, func_name, "dimensions out of range");
+ return error::kNoError;
+ }
+
GLenum type = 0;
GLenum internal_format = 0;
if (!texture->GetLevelType(target, level, &type, &internal_format)) {
@@ -15204,7 +15216,8 @@
GL_INVALID_OPERATION, func_name, "texture is immutable");
return;
}
- if (!texture_manager()->ValidForTarget(target, level, width, height, 1) ||
+ if (!texture_manager()->ValidForTextureTarget(texture, level, width, height,
+ 1) ||
border != 0) {
LOCAL_SET_GL_ERROR(
GL_INVALID_VALUE, func_name, "dimensions out of range");
@@ -17778,8 +17791,8 @@
}
// Check that this type of texture is allowed.
- if (!texture_manager()->ValidForTarget(source_target, source_level,
- source_width, source_height, 1)) {
+ if (!texture_manager()->ValidForTextureTarget(
+ source_texture, source_level, source_width, source_height, 1)) {
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, kFunctionName, "Bad dimensions");
return;
}
@@ -17946,8 +17959,8 @@
}
// Check that this type of texture is allowed.
- if (!texture_manager()->ValidForTarget(source_target, source_level,
- source_width, source_height, 1)) {
+ if (!texture_manager()->ValidForTextureTarget(
+ source_texture, source_level, source_width, source_height, 1)) {
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name,
"source texture bad dimensions");
return;
@@ -18187,11 +18200,20 @@
return;
}
}
+ TextureRef* texture_ref =
+ texture_manager()->GetTextureInfoForTarget(&state_, target);
+ if (!texture_ref) {
+ LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
+ "unknown texture for target");
+ return;
+ }
+ Texture* texture = texture_ref->texture();
// The glTexStorage entry points require width, height, and depth to be
// at least 1, but the other texture entry points (those which use
- // ValidForTarget) do not. So we have to add an extra check here.
+ // ValidForTextureTarget) do not. So we have to add an extra check here.
bool is_invalid_texstorage_size = width < 1 || height < 1 || depth < 1;
- if (!texture_manager()->ValidForTarget(target, 0, width, height, depth) ||
+ if (!texture_manager()->ValidForTextureTarget(texture, 0, width, height,
+ depth) ||
is_invalid_texstorage_size) {
LOCAL_SET_GL_ERROR(
GL_INVALID_VALUE, function_name, "dimensions out of range");
@@ -18204,14 +18226,6 @@
LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name, "too many levels");
return;
}
- TextureRef* texture_ref = texture_manager()->GetTextureInfoForTarget(
- &state_, target);
- if (!texture_ref) {
- LOCAL_SET_GL_ERROR(
- GL_INVALID_OPERATION, function_name, "unknown texture for target");
- return;
- }
- Texture* texture = texture_ref->texture();
if (texture->IsAttachedToFramebuffer()) {
framebuffer_state_.clear_state_dirty = true;
}
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 48325b8..ba0ebe2 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -1642,7 +1642,7 @@
return;
if (face_infos_.empty() ||
- static_cast<size_t>(base_level_) >= face_infos_[0].level_infos.size()) {
+ static_cast<size_t>(base_level_) >= MaxValidMipLevel()) {
texture_complete_ = false;
cube_complete_ = false;
return;
@@ -2025,8 +2025,7 @@
// the time.
if (face_infos_.size() == 6 && !cube_complete())
return false;
- DCHECK(level >= 0 &&
- level < static_cast<GLint>(face_infos_[0].level_infos.size()));
+ DCHECK(level >= 0 && level < static_cast<GLint>(MaxValidMipLevel()));
if (level > base_level_ && !texture_complete()) {
return false;
}
@@ -2061,7 +2060,7 @@
void Texture::ApplyFormatWorkarounds(const FeatureInfo* feature_info) {
if (feature_info->gl_version_info().NeedsLuminanceAlphaEmulation()) {
- if (static_cast<size_t>(base_level_) >= face_infos_[0].level_infos.size())
+ if (static_cast<size_t>(base_level_) >= MaxValidMipLevel())
return;
const Texture::LevelInfo& info = face_infos_[0].level_infos[base_level_];
SetCompatibilitySwizzle(GetCompatibilitySwizzleInternal(info.format));
@@ -2296,8 +2295,11 @@
return default_texture;
}
-bool TextureManager::ValidForTarget(
- GLenum target, GLint level, GLsizei width, GLsizei height, GLsizei depth) {
+bool TextureManager::ValidForTarget(GLenum target,
+ GLint level,
+ GLsizei width,
+ GLsizei height,
+ GLsizei depth) {
if (level < 0 || level >= MaxLevelsForTarget(target))
return false;
GLsizei max_size = MaxSizeForTarget(target) >> level;
@@ -2317,6 +2319,18 @@
(target != GL_TEXTURE_2D || (depth == 1));
}
+bool TextureManager::ValidForTextureTarget(const Texture* texture,
+ GLint level,
+ GLsizei width,
+ GLsizei height,
+ GLsizei depth) {
+ if (texture->target() == 0)
+ return false;
+ if (level < 0 || static_cast<size_t>(level) >= texture->MaxValidMipLevel())
+ return false;
+ return ValidForTarget(texture->target(), level, width, height, depth);
+}
+
void TextureManager::SetTarget(TextureRef* ref, GLenum target) {
DCHECK(ref);
ref->texture()->SetTarget(target, MaxLevelsForTarget(target));
@@ -2800,14 +2814,6 @@
args.internal_format, args.level)) {
return false;
}
- if (!ValidForTarget(args.target, args.level,
- args.width, args.height, args.depth) ||
- args.border != 0) {
- ERRORSTATE_SET_GL_ERROR(
- error_state, GL_INVALID_VALUE, function_name,
- "dimensions out of range");
- return false;
- }
if ((GLES2Util::GetChannelsForFormat(args.format) &
(GLES2Util::kDepth | GLES2Util::kStencil)) != 0 && args.pixels
&& !feature_info_->IsWebGL2OrES3Context()) {
@@ -2830,7 +2836,13 @@
"texture is immutable");
return false;
}
-
+ if (!ValidForTextureTarget(local_texture_ref->texture(), args.level,
+ args.width, args.height, args.depth) ||
+ args.border != 0) {
+ ERRORSTATE_SET_GL_ERROR(error_state, GL_INVALID_VALUE, function_name,
+ "dimensions out of range");
+ return false;
+ }
Buffer* buffer = state->bound_pixel_unpack_buffer.get();
if (buffer) {
if (buffer->GetMappedRange()) {
diff --git a/gpu/command_buffer/service/texture_manager.h b/gpu/command_buffer/service/texture_manager.h
index 341adcd..bf92c1b 100644
--- a/gpu/command_buffer/service/texture_manager.h
+++ b/gpu/command_buffer/service/texture_manager.h
@@ -470,6 +470,11 @@
sampler_state_.min_filter != GL_LINEAR;
}
+ size_t MaxValidMipLevel() const {
+ DCHECK(!face_infos_.empty());
+ return face_infos_[0].level_infos.size();
+ }
+
private:
friend class MailboxManagerTest;
friend class TextureManager;
@@ -932,6 +937,11 @@
bool ValidForTarget(
GLenum target, GLint level,
GLsizei width, GLsizei height, GLsizei depth);
+ bool ValidForTextureTarget(const Texture* texture,
+ GLint level,
+ GLsizei width,
+ GLsizei height,
+ GLsizei depth);
// True if this texture meets all the GLES2 criteria for rendering.
// See section 3.8.2 of the GLES2 spec.
