New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847725 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::ImageLayerBridge::PrepareTransferableResource

Project Member Reported by ClusterFuzz, May 30 2018

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5093861752569856

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_ubsan_vptr_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::ImageLayerBridge::PrepareTransferableResource
  cc::TextureLayer::Update
  cc::LayerTreeHost::PaintContent
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=543412:543415

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5093861752569856

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, May 30 2018

Components: Blink>WebGL Internals>Compositing
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Cc: pnangunoori@chromium.org
Labels: M-69 Test-Predator-Wrong
Owner: danakj@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using the code search for the file, “texture_layer.cc” assigning to concern owner from GIT blame.
Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/92d63896e496e23dcbedacd873fc37b8be8a5df6

@danakj -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.
Thank You.

Project Member

Comment 3 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4d21dc26bb412d4e47edd7deda3d81ad9c16179

commit b4d21dc26bb412d4e47edd7deda3d81ad9c16179
Author: danakj <danakj@chromium.org>
Date: Thu May 31 17:15:19 2018

Clear TextureLayer client before dropping reference in ImageLayerBridge

When removing its reference to the TextureLayer, the ImageLayerBridge
may in the future be destroyed, and should no longer be held as the
layer's client. If the layer is kept alive by another reference, such
as an ongoing update/commit, or elsewhere in the system, then cc
will try to use the client which could be an invalid pointer.

This was done for other sites in bd76d2ec7ac8bf8fe2186c1ab8da92 but
this case was missed.

R=chrishtr@chromium.org

Bug:  847725 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Id5874d6477a4e1869fca82ce4d21e2daafe475a4
Reviewed-on: https://chromium-review.googlesource.com/1080396
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563290}
[modify] https://crrev.com/b4d21dc26bb412d4e47edd7deda3d81ad9c16179/third_party/blink/renderer/platform/graphics/gpu/image_layer_bridge.cc

Comment 4 by danakj@chromium.org, May 31 2018

Status: Fixed (was: Assigned)
Project Member

Comment 5 by ClusterFuzz, Jun 7 2018

Labels: Needs-Feedback
ClusterFuzz testcase 5093861752569856 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.
Status: Assigned (was: Fixed)
Owner: junov@chromium.org
oh this looks like a bug in ImageLayerBridge:

https://chromium.googlesource.com/chromium/src/+/fa718bcf8146558b23914cf301974249cd4efe07/third_party/blink/renderer/platform/graphics/gpu/image_layer_bridge.cc#127

It crashes on

  // Upload to a texture if the compositor is expecting one.
  if (gpu_compositing && !image_->IsTextureBacked()) {
    image_for_compositor =
        image_->MakeAccelerated(SharedGpuContext::ContextProviderWrapper());
  } else if (!gpu_compositing && image_->IsTextureBacked()) {
    image_for_compositor = image_->MakeUnaccelerated();
  } else {
    image_for_compositor = image_;
  }
  DCHECK_EQ(image_for_compositor->IsTextureBacked(), gpu_compositing);
  if (gpu_compositing) {
    uint32_t filter =
        filter_quality_ == kNone_SkFilterQuality ? GL_NEAREST : GL_LINEAR;
    image_for_compositor->EnsureMailbox(kUnverifiedSyncToken, filter);

This last line.

image_->MakeAccelerated() can return nullptr, but this doesn't check for it.

Comment 8 by junov@chromium.org, Jun 8 2018

Status: Started (was: Assigned)
Yep. The logic here is also unnecessarily complex. I'm on it.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2790aeed7431225c8deb172cf1dde94d053a7747

commit 2790aeed7431225c8deb172cf1dde94d053a7747
Author: Justin Novosad <junov@chromium.org>
Date: Mon Jun 11 16:00:19 2018

Fix crash in ImageLayerBridge on gpu resource allocation failure

BUG= 847725 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ie762af63cd0a414a10a940344b90e6296dcb60a9
Reviewed-on: https://chromium-review.googlesource.com/1093161
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566020}
[modify] https://crrev.com/2790aeed7431225c8deb172cf1dde94d053a7747/third_party/blink/renderer/platform/graphics/accelerated_static_bitmap_image.h
[modify] https://crrev.com/2790aeed7431225c8deb172cf1dde94d053a7747/third_party/blink/renderer/platform/graphics/gpu/image_layer_bridge.cc

Project Member

Comment 10 by ClusterFuzz, Jun 12 2018

ClusterFuzz has detected this issue as fixed in range 566019:566021.

Detailed report: https://clusterfuzz.com/testcase?key=5093861752569856

Fuzzer: mbarbella_js_mutation_layout
Job Type: linux_ubsan_vptr_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::ImageLayerBridge::PrepareTransferableResource
  cc::TextureLayer::Update
  cc::LayerTreeHost::PaintContent
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=543412:543415
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=566019:566021

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5093861752569856

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, Jun 12 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 5093861752569856 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment