New issue
Advanced search Search tips

Issue 921358 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in mojo::StructTraits<viz::mojom::CopyOutputResultDataView, std::__1::unique_ptr<vi

Project Member Reported by ClusterFuzz, Jan 12

Issue description

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

Fuzzer: meacer_extension_apis
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  mojo::StructTraits<viz::mojom::CopyOutputResultDataView, std::__1::unique_ptr<vi
  mojo::internal::Serializer<viz::mojom::CopyOutputResultDataView, std::__1::uniqu
  viz::mojom::CopyOutputResultSenderProxy::SendResult
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=622254:622255

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for instructions to reproduce this bug locally.
 
Project Member

Comment 1 by ClusterFuzz, Jan 12

Components: Internals>Core
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.
Project Member

Comment 2 by ClusterFuzz, Jan 12

Labels: Test-Predator-Auto-Owner
Owner: malaykeshav@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/5b1ebc04f52bdbcf20f0165fa3d8405e0d15fd55 (Store and set stale frame content screenshot when the frame is evicted).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Cc: sadrul@chromium.org fdoray@chromium.org osh...@chromium.org sky@chromium.org samans@chromium.org
Status: Started (was: Assigned)
Will add a MACRO to only capture stale image for Chrome OS
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 14

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

commit 962a444c2612006be95d08fea01f7d03e898d5a0
Author: Malay Keshav <malaykeshav@chromium.org>
Date: Mon Jan 14 03:15:45 2019

Revert "Store and set stale frame content screenshot when the frame is evicted"

This reverts commit 5b1ebc04f52bdbcf20f0165fa3d8405e0d15fd55.

Reason for revert: Browser crash on Non CrOS platforms.


Original change's description:
> Store and set stale frame content screenshot when the frame is evicted
> 
> This patch captures the web content surface as a texture and displays
> it on the RenderWidgetHostView until a new compositor frame is submitted
> by the viz client. This allows the browser to display stale content
> instead of a white solid color during animations.
> 
> The decision to show stale content or a solid color is done by the
> WebContentsDelegate associated with the RenderWidgetHostView and its
> DelegatedFrameHost. For this patch, the default is to show a solid
> color on frame eviction. In the case of a tabbed browser, the Browser
> (which implements the WebContentsDelegate) chooses to show stale content
> if the frame being evicted is from an active tab.
> 
> Bug: 897826
> Change-Id: I3346f3dec79780d107d2c96b4382886690a69570
> Component: RenderWidgetHostViewAura, DelegatedFrameHost, ui layer
> Reviewed-on: https://chromium-review.googlesource.com/c/1369714
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#622255}

TBR=sadrul@chromium.org,sky@chromium.org,danakj@chromium.org,oshima@chromium.org,fdoray@chromium.org,samans@chromium.org,malaykeshav@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 897826, 921420,  921358 
Change-Id: I79a41a4c01c6b1335c31e26eb1f69434834eed7e
Reviewed-on: https://chromium-review.googlesource.com/c/1408224
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622377}
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/chrome/browser/ui/browser.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/chrome/browser/ui/browser.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/DEPS
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/delegated_frame_host_client_aura.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/delegated_frame_host_client_aura.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/renderer_host/render_widget_host_view_aura.h
[delete] https://crrev.com/481af38cb355bc279458dd9838e19ab1b99709d0/content/browser/renderer_host/render_widget_host_view_aura_browsertest.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/content/test/BUILD.gn
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/ui/compositor/layer.cc
[modify] https://crrev.com/962a444c2612006be95d08fea01f7d03e898d5a0/ui/compositor/layer_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 14

Labels: merge-merged-3671
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7605ca88f0b11289fbc21289d1d182d1872938e4

commit 7605ca88f0b11289fbc21289d1d182d1872938e4
Author: Malay Keshav <malaykeshav@chromium.org>
Date: Mon Jan 14 14:52:59 2019

Revert "Store and set stale frame content screenshot when the frame is evicted"

This reverts commit 5b1ebc04f52bdbcf20f0165fa3d8405e0d15fd55.

Reason for revert: Browser crash on Non CrOS platforms.


Original change's description:
> Store and set stale frame content screenshot when the frame is evicted
> 
> This patch captures the web content surface as a texture and displays
> it on the RenderWidgetHostView until a new compositor frame is submitted
> by the viz client. This allows the browser to display stale content
> instead of a white solid color during animations.
> 
> The decision to show stale content or a solid color is done by the
> WebContentsDelegate associated with the RenderWidgetHostView and its
> DelegatedFrameHost. For this patch, the default is to show a solid
> color on frame eviction. In the case of a tabbed browser, the Browser
> (which implements the WebContentsDelegate) chooses to show stale content
> if the frame being evicted is from an active tab.
> 
> Bug: 897826
> Change-Id: I3346f3dec79780d107d2c96b4382886690a69570
> Component: RenderWidgetHostViewAura, DelegatedFrameHost, ui layer
> Reviewed-on: https://chromium-review.googlesource.com/c/1369714
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#622255}

TBR=sadrul@chromium.org,sky@chromium.org,danakj@chromium.org,oshima@chromium.org,fdoray@chromium.org,samans@chromium.org,malaykeshav@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 897826, 921420,  921358 
Change-Id: I79a41a4c01c6b1335c31e26eb1f69434834eed7e
Reviewed-on: https://chromium-review.googlesource.com/c/1408224
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#622377}(cherry picked from commit 962a444c2612006be95d08fea01f7d03e898d5a0)
Reviewed-on: https://chromium-review.googlesource.com/c/1409482
Reviewed-by: Abdul Syed <abdulsyed@google.com>
Cr-Commit-Position: refs/branch-heads/3671@{#3}
Cr-Branched-From: f0600c9f0ef2c10cefa6b5ee49365b255176192c-refs/heads/master@{#622373}
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/chrome/browser/ui/browser.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/chrome/browser/ui/browser.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/DEPS
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/delegated_frame_host_client_aura.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/delegated_frame_host_client_aura.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/renderer_host/render_widget_host_view_aura.h
[delete] https://crrev.com/e10bff5c0385c7c942a9185997b022dafd1027de/content/browser/renderer_host/render_widget_host_view_aura_browsertest.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/content/test/BUILD.gn
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/ui/compositor/layer.cc
[modify] https://crrev.com/7605ca88f0b11289fbc21289d1d182d1872938e4/ui/compositor/layer_unittest.cc

Project Member

Comment 6 by ClusterFuzz, Jan 14

ClusterFuzz has detected this issue as fixed in range 622376:622377.

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

Fuzzer: meacer_extension_apis
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  mojo::StructTraits<viz::mojom::CopyOutputResultDataView, std::__1::unique_ptr<vi
  mojo::internal::Serializer<viz::mojom::CopyOutputResultDataView, std::__1::uniqu
  viz::mojom::CopyOutputResultSenderProxy::SendResult
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=622254:622255
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=622376:622377

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

See https://github.com/google/clusterfuzz-tools for instructions to reproduce this bug locally.

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

Comment 7 by ClusterFuzz, Jan 14

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

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

Comment 8 by bugdroid1@chromium.org, Jan 15

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

commit cf4c1d4408c27394a7d088c81b5436c561fb7557
Author: Saman Sami <samans@chromium.org>
Date: Tue Jan 15 01:53:28 2019

Fix null dereference in CopyOutputResult's struct traits

The CopyOutputResult's struct traits does not properly handle empty
CopyOutputResults when the format is RGBA_TEXTURE.

Bug:  921358 
Change-Id: I103fbcf06682a5eda421ab90e9e9e4b756ab2407
Reviewed-on: https://chromium-review.googlesource.com/c/1409962
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622676}
[modify] https://crrev.com/cf4c1d4408c27394a7d088c81b5436c561fb7557/services/viz/public/cpp/compositing/copy_output_result_struct_traits.cc

Sign in to add a comment