Upgrade Tex{Sub}Image{2|3}D to handle ES3 unpack parameters.

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index a3ad42338965a9e9f7e5c85fb6e133d83079545d..d161fa083f26d120fbc1f0252035c12edcfecbdb 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -9545,23 +9545,26 @@ error::Error GLES2DecoderImpl::HandlePixelStorei(uint32_t immediate_data_size,
       }
       break;
     case GL_PACK_ROW_LENGTH:
-    case GL_PACK_SKIP_PIXELS:
-    case GL_PACK_SKIP_ROWS:
     case GL_UNPACK_ROW_LENGTH:
     case GL_UNPACK_IMAGE_HEIGHT:
-    case GL_UNPACK_SKIP_PIXELS:
-    case GL_UNPACK_SKIP_ROWS:
-    case GL_UNPACK_SKIP_IMAGES:
       if (param < 0) {
         LOCAL_SET_GL_ERROR(
             GL_INVALID_VALUE, "glPixelStorei", "invalid param");
         return error::kNoError;
       }
+      break;
+    case GL_PACK_SKIP_PIXELS:
+    case GL_PACK_SKIP_ROWS:
+    case GL_UNPACK_SKIP_PIXELS:
+    case GL_UNPACK_SKIP_ROWS:
+    case GL_UNPACK_SKIP_IMAGES:
+      // All SKIP parameters are handled on the client side and should never
+      // be passed to the service side.
+      NOTREACHED();

[piman, 2016/03/08 01:56:50]

nit: no NOTREACHED() since this can be reached by a malicious compromised
renderer / nacl module.

[Zhenyao Mo, 2016/03/09 18:41:05]

Done.

+      return error::kInvalidArguments;
     default:
       break;
   }
-  // For pack skip parameters, we don't apply them and handle them in command
-  // buffer.
   // For alignment parameters, we always apply them.
   // For other parameters, we don't apply them if no buffer is bound at
   // PIXEL_PACK or PIXEL_UNPACK. We will handle pack and unpack according to
@@ -9571,14 +9574,8 @@ error::Error GLES2DecoderImpl::HandlePixelStorei(uint32_t immediate_data_size,
       if (state_.bound_pixel_pack_buffer.get())
         glPixelStorei(pname, param);
       break;
-    case GL_PACK_SKIP_PIXELS:
-    case GL_PACK_SKIP_ROWS:
-      break;
     case GL_UNPACK_ROW_LENGTH:
     case GL_UNPACK_IMAGE_HEIGHT:
-    case GL_UNPACK_SKIP_PIXELS:
-    case GL_UNPACK_SKIP_ROWS:
-    case GL_UNPACK_SKIP_IMAGES:
       if (state_.bound_pixel_unpack_buffer.get())
         glPixelStorei(pname, param);
       break;
@@ -9593,12 +9590,6 @@ error::Error GLES2DecoderImpl::HandlePixelStorei(uint32_t immediate_data_size,
     case GL_PACK_ROW_LENGTH:
       state_.pack_row_length = param;
       break;
-    case GL_PACK_SKIP_PIXELS:
-      state_.pack_skip_pixels = param;
-      break;
-    case GL_PACK_SKIP_ROWS:
-      state_.pack_skip_rows = param;
-      break;
     case GL_UNPACK_ALIGNMENT:
       state_.unpack_alignment = param;
       break;
@@ -9608,15 +9599,6 @@ error::Error GLES2DecoderImpl::HandlePixelStorei(uint32_t immediate_data_size,
     case GL_UNPACK_IMAGE_HEIGHT:
       state_.unpack_image_height = param;
       break;
-    case GL_UNPACK_SKIP_PIXELS:
-      state_.unpack_skip_pixels = param;
-      break;
-    case GL_UNPACK_SKIP_ROWS:
-      state_.unpack_skip_rows = param;
-      break;
-    case GL_UNPACK_SKIP_IMAGES:
-      state_.unpack_skip_images = param;
-      break;
     default:
       // Validation should have prevented us from getting here.
       NOTREACHED();
@@ -10266,11 +10248,21 @@ bool GLES2DecoderImpl::ClearLevel3D(Texture* texture,
 
   uint32_t size;
   uint32_t padded_row_size;
+  uint32_t padding;
   // Here we use unpack buffer to upload zeros into the texture, one layer
-  // at a time. We haven't applied unpack parameters to GL except alignment.
-  if (!GLES2Util::ComputeImageDataSizes(
-          width, height, depth, format, type, state_.unpack_alignment, &size,
-          nullptr, &padded_row_size)) {
+  // at a time.
+  // We only take into consideration UNPACK_ALIGNMENT, and clear other unpack
+  // parameters if necessary before TexSubImage3D calls.
+  PixelStoreParams params;
+  params.alignment = state_.unpack_alignment;
+  if (!GLES2Util::ComputeImageDataSizesES3(width, height, depth,
+                                           format, type,
+                                           params,
+                                           &size,
+                                           nullptr,
+                                           &padded_row_size,
+                                           nullptr,
+                                           &padding)) {
     return false;
   }
   const uint32_t kMaxZeroSize = 1024 * 1024 * 2;
@@ -10332,6 +10324,16 @@ bool GLES2DecoderImpl::ClearLevel3D(Texture* texture,
   glGenBuffersARB(1, &buffer_id);
   glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer_id);
   {
+    // Include padding as some drivers incorrectly requires padding for the
+    // last row.
+    buffer_size += padding;
+    // Buffer size needs to be a multiple of type bytes.
+    size_t type_bytes = GLES2Util::GetGLTypeSizeForTextures(type);
+    size_t residual = buffer_size % type_bytes;
+    if (residual != 0) {
+      buffer_size += type_bytes - residual;
+    }
+    DCHECK(buffer_size % type_bytes == 0);
     scoped_ptr<char[]> zero(new char[buffer_size]);
     memset(zero.get(), 0, buffer_size);
     // TODO(zmo): Consider glMapBufferRange instead.
@@ -10339,6 +10341,20 @@ bool GLES2DecoderImpl::ClearLevel3D(Texture* texture,
         GL_PIXEL_UNPACK_BUFFER, buffer_size, zero.get(), GL_STATIC_DRAW);
   }
 
+  Buffer* bound_buffer = buffer_manager()->GetBufferInfoForTarget(
+      &state_, GL_PIXEL_UNPACK_BUFFER);
+  if (bound_buffer) {
+    // If an unpack buffer is bound, we need to clear unpack parameters
+    // because they have been applied to the driver.
+    if (state_.unpack_row_length > 0)
+      glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);
+    if (state_.unpack_image_height > 0)
+      glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, 0);
+    DCHECK_EQ(0, state_.unpack_skip_pixels);
+    DCHECK_EQ(0, state_.unpack_skip_rows);
+    DCHECK_EQ(0, state_.unpack_skip_images);
+  }
+
   glBindTexture(texture->target(), texture->service_id());
 
   for (size_t ii = 0; ii < subs.size(); ++ii) {
@@ -10347,8 +10363,13 @@ bool GLES2DecoderImpl::ClearLevel3D(Texture* texture,
                     subs[ii].depth, format, type, nullptr);
   }
 
-  Buffer* bound_buffer = buffer_manager()->GetBufferInfoForTarget(
-      &state_, GL_PIXEL_UNPACK_BUFFER);
+  if (bound_buffer) {
+    if (state_.unpack_row_length > 0)
+      glPixelStorei(GL_UNPACK_ROW_LENGTH, state_.unpack_row_length);
+    if (state_.unpack_image_height > 0)
+      glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, state_.unpack_image_height);
+  }
+
   glBindBuffer(GL_PIXEL_UNPACK_BUFFER,
                bound_buffer ? bound_buffer->service_id() : 0);
   glDeleteBuffersARB(1, &buffer_id);
@@ -11229,19 +11250,53 @@ error::Error GLES2DecoderImpl::HandleTexImage2D(uint32_t immediate_data_size,
   GLenum type = static_cast<GLenum>(c.type);
   uint32_t pixels_shm_id = static_cast<uint32_t>(c.pixels_shm_id);
   uint32_t pixels_shm_offset = static_cast<uint32_t>(c.pixels_shm_offset);
-  uint32_t pixels_size;
-  if (!GLES2Util::ComputeImageDataSizes(
-      width, height, 1, format, type, state_.unpack_alignment, &pixels_size,
-      NULL, NULL)) {
+
+  if (width < 0 || height < 0) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glTexImage2D", "dimensions < 0");
+    return error::kNoError;
+  }
+  uint32_t pixels_size = 0;
+  uint32_t skip_size = 0;
+  bool no_pixels = false;
+  PixelStoreParams params;
+  if (state_.bound_pixel_unpack_buffer.get()) {
+    DCHECK_EQ(0u, pixels_shm_id);

[piman, 2016/03/08 01:56:50]

Is this DCHECK valid? We can't trust the client. We probably need to return
kInvalidArguments if this happens.

[Zhenyao Mo, 2016/03/09 18:41:05]

Done.

+    params = state_.GetUnpackParams(ContextState::k2D);
+  } else {
+    if (pixels_shm_id) {
+      // When reading from client buffer, the command buffer client side took
+      // the responsibility to take the pixels from the client buffer and
+      // unpack them according to the full ES3 pack parameters as source, all
+      // parameters for 0 (except for alignment) as destination mem for the
+      // service side.
+      params.alignment = state_.unpack_alignment;
+    } else {
+      DCHECK_EQ(0u, pixels_shm_offset);

[piman, 2016/03/08 01:56:50]

Same here.

[Zhenyao Mo, 2016/03/09 18:41:05]

Done.

+      no_pixels = true;
+    }
+  }
+  if (!no_pixels &&
+      !GLES2Util::ComputeImageDataSizesES3(width, height, 1,
+                                           format, type,
+                                           params,
+                                           &pixels_size,
+                                           nullptr,
+                                           nullptr,
+                                           &skip_size,
+                                           nullptr)) {
     return error::kOutOfBounds;
   }
-  const void* pixels = NULL;
-  if (pixels_shm_id != 0 || pixels_shm_offset != 0) {
+  DCHECK_EQ(0u, skip_size);
+
+  const void* pixels = nullptr;
+  if (pixels_shm_id) {
     pixels = GetSharedMemoryAs<const void*>(
         pixels_shm_id, pixels_shm_offset, pixels_size);
     if (!pixels) {
       return error::kOutOfBounds;
     }
+  } else {
+    pixels = reinterpret_cast<const void*>(pixels_shm_offset);
   }
 
   // For testing only. Allows us to stress the ability to respond to OOM errors.
@@ -11287,19 +11342,54 @@ error::Error GLES2DecoderImpl::HandleTexImage3D(uint32_t immediate_data_size,
   GLenum type = static_cast<GLenum>(c.type);
   uint32_t pixels_shm_id = static_cast<uint32_t>(c.pixels_shm_id);
   uint32_t pixels_shm_offset = static_cast<uint32_t>(c.pixels_shm_offset);
-  uint32_t pixels_size;
-  if (!GLES2Util::ComputeImageDataSizes(
-      width, height, depth, format, type, state_.unpack_alignment, &pixels_size,
-      NULL, NULL)) {
+
+  if (width < 0 || height < 0 || depth < 0) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glTexImage3D", "dimensions < 0");
+    return error::kNoError;
+  }
+
+  uint32_t pixels_size = 0;
+  uint32_t skip_size = 0;
+  bool no_pixels = false;
+  PixelStoreParams params;
+  if (state_.bound_pixel_unpack_buffer.get()) {
+    DCHECK_EQ(0u, pixels_shm_id);

[piman, 2016/03/08 01:56:50]

Same as above.

[Zhenyao Mo, 2016/03/09 18:41:05]

Done.

+    params = state_.GetUnpackParams(ContextState::k3D);
+  } else {
+    if (pixels_shm_id) {
+      // When reading from client buffer, the command buffer client side took
+      // the responsibility to take the pixels from the client buffer and
+      // unpack them according to the full ES3 pack parameters as source, all
+      // parameters for 0 (except for alignment) as destination mem for the
+      // service side.
+      params.alignment = state_.unpack_alignment;
+    } else {
+      DCHECK_EQ(0u, pixels_shm_offset);

[piman, 2016/03/08 01:56:50]

And here.

[Zhenyao Mo, 2016/03/09 18:41:05]

Done.

+      no_pixels = true;
+    }
+  }
+  if (!no_pixels &&
+      !GLES2Util::ComputeImageDataSizesES3(width, height, depth,
+                                           format, type,
+                                           params,
+                                           &pixels_size,
+                                           nullptr,
+                                           nullptr,
+                                           &skip_size,
+                                           nullptr)) {
     return error::kOutOfBounds;
   }
-  const void* pixels = NULL;
-  if (pixels_shm_id != 0 || pixels_shm_offset != 0) {
+  DCHECK_EQ(0u, skip_size);
+
+  const void* pixels = nullptr;
+  if (pixels_shm_id) {
     pixels = GetSharedMemoryAs<const void*>(
         pixels_shm_id, pixels_shm_offset, pixels_size);
     if (!pixels) {
       return error::kOutOfBounds;
     }
+  } else {
+    pixels = reinterpret_cast<const void*>(pixels_shm_offset);
   }
 
   // For testing only. Allows us to stress the ability to respond to OOM errors.
@@ -11324,15 +11414,8 @@ error::Error GLES2DecoderImpl::HandleTexImage3D(uint32_t immediate_data_size,
 }
 
 void GLES2DecoderImpl::DoCompressedTexSubImage2D(
-  GLenum target,
-  GLint level,
-  GLint xoffset,
-  GLint yoffset,
-  GLsizei width,
-  GLsizei height,
-  GLenum format,
-  GLsizei image_size,
-  const void * data) {
+    GLenum target, GLint level, GLint xoffset, GLint yoffset, GLsizei width,
+    GLsizei height, GLenum format, GLsizei image_size, const void * data) {
   TextureRef* texture_ref = texture_manager()->GetTextureInfoForTarget(
       &state_, target);
   if (!texture_ref) {
@@ -11679,21 +11762,59 @@ error::Error GLES2DecoderImpl::HandleTexSubImage2D(uint32_t immediate_data_size,
   GLsizei height = static_cast<GLsizei>(c.height);
   GLenum format = static_cast<GLenum>(c.format);
   GLenum type = static_cast<GLenum>(c.type);
-  uint32_t data_size;
-  if (!GLES2Util::ComputeImageDataSizes(
-      width, height, 1, format, type, state_.unpack_alignment, &data_size,
-      NULL, NULL)) {
+  uint32_t pixels_shm_id = static_cast<uint32_t>(c.pixels_shm_id);
+  uint32_t pixels_shm_offset = static_cast<uint32_t>(c.pixels_shm_offset);
+
+  if (width < 0 || height < 0) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glTexSubImage2D", "dimensions < 0");
+    return error::kNoError;
+  }
+  PixelStoreParams params;
+  if (state_.bound_pixel_unpack_buffer.get()) {
+    params = state_.GetUnpackParams(ContextState::k2D);
+  } else {
+    // When reading from client buffer, the command buffer client side took
+    // the responsibility to take the pixels from the client buffer and
+    // unpack them according to the full ES3 pack parameters as source, all
+    // parameters for 0 (except for alignment) as destination mem for the
+    // service side.
+    params.alignment = state_.unpack_alignment;
+  }
+  uint32_t pixels_size;
+  uint32_t skip_size;
+  if (!GLES2Util::ComputeImageDataSizesES3(width, height, 1,
+                                           format, type,
+                                           params,
+                                           &pixels_size,
+                                           nullptr,
+                                           nullptr,
+                                           &skip_size,
+                                           nullptr)) {
     return error::kOutOfBounds;
   }
+  DCHECK_EQ(0u, skip_size);
 
-  const void* pixels = GetSharedMemoryAs<const void*>(
-      c.pixels_shm_id, c.pixels_shm_offset, data_size);
-  if (!pixels)
-    return error::kOutOfBounds;
+  const void* pixels;
+  if (state_.bound_pixel_unpack_buffer.get()) {
+    if (pixels_shm_id != 0)
+      return error::kInvalidArguments;
+    pixels = reinterpret_cast<const void*>(pixels_shm_offset);
+  } else {
+    if (pixels_size > 0) {
+      if (pixels_shm_id == 0)
+        return error::kInvalidArguments;
+      pixels = GetSharedMemoryAs<const void*>(
+          pixels_shm_id, pixels_shm_offset, pixels_size);
+      if (!pixels)
+        return error::kOutOfBounds;
+    } else {
+      pixels = nullptr;
+    }
+  }
 
   TextureManager::DoTexSubImageArguments args = {
       target, level, xoffset, yoffset, 0, width, height, 1,
-      format, type, pixels, data_size,
+      format, type, pixels, pixels_size,
       TextureManager::DoTexSubImageArguments::kTexSubImage2D};
   texture_manager()->ValidateAndDoTexSubImage(this, &texture_state_, &state_,
                                               &framebuffer_state_,
@@ -11728,21 +11849,59 @@ error::Error GLES2DecoderImpl::HandleTexSubImage3D(uint32_t immediate_data_size,
   GLsizei depth = static_cast<GLsizei>(c.depth);
   GLenum format = static_cast<GLenum>(c.format);
   GLenum type = static_cast<GLenum>(c.type);
-  uint32_t data_size;
-  if (!GLES2Util::ComputeImageDataSizes(
-      width, height, depth, format, type, state_.unpack_alignment, &data_size,
-      NULL, NULL)) {
+  uint32_t pixels_shm_id = static_cast<uint32_t>(c.pixels_shm_id);
+  uint32_t pixels_shm_offset = static_cast<uint32_t>(c.pixels_shm_offset);
+
+  if (width < 0 || height < 0 || depth < 0) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glTexSubImage3D", "dimensions < 0");
+    return error::kNoError;
+  }
+  PixelStoreParams params;
+  if (state_.bound_pixel_unpack_buffer.get()) {
+    params = state_.GetUnpackParams(ContextState::k3D);
+  } else {
+    // When reading from client buffer, the command buffer client side took
+    // the responsibility to take the pixels from the client buffer and
+    // unpack them according to the full ES3 pack parameters as source, all
+    // parameters for 0 (except for alignment) as destination mem for the
+    // service side.
+    params.alignment = state_.unpack_alignment;
+  }
+  uint32_t pixels_size;
+  uint32_t skip_size;
+  if (!GLES2Util::ComputeImageDataSizesES3(width, height, depth,
+                                           format, type,
+                                           params,
+                                           &pixels_size,
+                                           nullptr,
+                                           nullptr,
+                                           &skip_size,
+                                           nullptr)) {
     return error::kOutOfBounds;
   }
+  DCHECK_EQ(0u, skip_size);
 
-  const void* pixels = GetSharedMemoryAs<const void*>(
-      c.pixels_shm_id, c.pixels_shm_offset, data_size);
-  if (!pixels)
-    return error::kOutOfBounds;
+  const void* pixels;
+  if (state_.bound_pixel_unpack_buffer.get()) {
+    if (pixels_shm_id != 0)
+      return error::kInvalidArguments;
+    pixels = reinterpret_cast<const void*>(pixels_shm_offset);
+  } else {
+    if (pixels_size > 0) {
+      if (pixels_shm_id == 0)
+        return error::kInvalidArguments;
+      pixels = GetSharedMemoryAs<const void*>(
+          pixels_shm_id, pixels_shm_offset, pixels_size);
+      if (!pixels)
+        return error::kOutOfBounds;
+    } else {
+      pixels = nullptr;
+    }
+  }
 
   TextureManager::DoTexSubImageArguments args = {
       target, level, xoffset, yoffset, zoffset, width, height, depth,
-      format, type, pixels, data_size,
+      format, type, pixels, pixels_size,
       TextureManager::DoTexSubImageArguments::kTexSubImage3D};
   texture_manager()->ValidateAndDoTexSubImage(this, &texture_state_, &state_,
                                               &framebuffer_state_,