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

Issue 837092 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in ash::BackdropController::UpdateBackdrop

Project Member Reported by ClusterFuzz, Apr 26 2018

Issue description

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

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_asan_chrome_chromeos
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  ash::BackdropController::UpdateBackdrop
  ash::Shell::NotifyOverviewModeEnded
  ash::WindowSelectorController::OnSelectionEnded
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=553250:553259

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

Additional requirements: Requires Gestures

Additional requirements: Requires HTTP

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Apr 26 2018

Components: UI>Shell>WindowManager
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, Apr 26 2018

Cc: minch@google.com
Labels: Test-Predator-Auto-CC
Automatically adding ccs based on suspected regression changelists:

Restore backdrop if dismissed app list. by minch@google.com - https://chromium.googlesource.com/chromium/src/+/7bf57266f44d4cf0133764abc6f3ff2a02945865

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.

Comment 3 by minch@chromium.org, Apr 26 2018

Cc: -minch@google.com osh...@chromium.org
Owner: minch@chromium.org
Status: Started (was: Untriaged)
Thanks. I am starting to take a look.

Steps to consistently repro the issue,
1. Click overview button to start overview mode.
2. Click app list button in overview mode.

Crash happened.

This is because the check in 
https://codesearch.chromium.org/chromium/src/ash/wm/workspace/backdrop_controller.cc?sq=package:chromium&l=130
The app list status is visible but not app list view in this case. So null pointer in this case.
Not sure why the app list state is visible in this case. Will do more debug.

Comment 4 by minch@chromium.org, Apr 26 2018

Labels: Merge-Request-67 M-67
This crash is the regression of https://bugs.chromium.org/p/chromium/issues/detail?id=836854&desc=2
which was merged back to M67.

Request merge back to M67 first.
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 26 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Sorry a little confused by #4.  Are we regressing a previous change, or ?  Also, is there an associated CL?  Has the change been tested?

Comment 7 by minch@chromium.org, Apr 26 2018

I think this crash is because of my previous cl here,
https://chromium-review.googlesource.com/c/chromium/src/+/1002863
which was landed into M67.
So I think we'd better also land the fix into M67.

Comment 8 by minch@chromium.org, Apr 26 2018

Sorry for the less explanation,
This should directly because of this change,
https://chromium-review.googlesource.com/c/chromium/src/+/1025215

and the change above is because of this change
https://chromium-review.googlesource.com/c/chromium/src/+/1002863

The two cls above are both in M67 now. 
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 Chrome OS.

Issue 838422 has been merged into this issue.
Project Member

Comment 11 by bugdroid1@chromium.org, May 1 2018

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

commit bf0a09236d19f7919085fd126b2aa6999dd48b40
Author: Min Chen <minch@google.com>
Date: Tue May 01 15:49:09 2018

Fixed crash happened when open launcher in overview mode.

Because of the animation delay, the app list view might still nullptr
when the TargetVisibility of app list is TRUE already. Make GetDisplayId
in AppListPresenterImpl as a public method and check the root window of
the display_id directly instead of getting the root window through app list.

Bug:  837092 
Change-Id: I3589702b278ac5fa556e1f77c7d8ec71d834547f
Reviewed-on: https://chromium-review.googlesource.com/1031747
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555047}
[modify] https://crrev.com/bf0a09236d19f7919085fd126b2aa6999dd48b40/ash/wm/workspace/backdrop_controller.cc
[modify] https://crrev.com/bf0a09236d19f7919085fd126b2aa6999dd48b40/ash/wm/workspace/workspace_layout_manager_unittest.cc

Project Member

Comment 12 by sheriffbot@chromium.org, May 1 2018

Cc: kbleicher@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 13 by bugdroid1@chromium.org, May 1 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58759603609fafa0febeb56b69a1bb8dc1c13e2b

commit 58759603609fafa0febeb56b69a1bb8dc1c13e2b
Author: Min Chen <minch@google.com>
Date: Tue May 01 17:01:06 2018

[Merge to M67]Fixed crash happened when open launcher in overview mode.

TBR=minch@chromium.org

Because of the animation delay, the app list view might still nullptr
when the TargetVisibility of app list is TRUE already. Make GetDisplayId
in AppListPresenterImpl as a public method and check the root window of
the display_id directly instead of getting the root window through app list.

(cherry picked from commit bf0a09236d19f7919085fd126b2aa6999dd48b40)

Bug:  837092 
Change-Id: I3589702b278ac5fa556e1f77c7d8ec71d834547f
Reviewed-on: https://chromium-review.googlesource.com/1031747
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555047}
Reviewed-on: https://chromium-review.googlesource.com/1037443
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#407}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/58759603609fafa0febeb56b69a1bb8dc1c13e2b/ash/wm/workspace/backdrop_controller.cc
[modify] https://crrev.com/58759603609fafa0febeb56b69a1bb8dc1c13e2b/ash/wm/workspace/workspace_layout_manager_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 15 by ClusterFuzz, May 2 2018

ClusterFuzz has detected this issue as fixed in range 555046:555052.

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

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_asan_chrome_chromeos
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  ash::BackdropController::UpdateBackdrop
  ash::Shell::NotifyOverviewModeEnded
  ash::WindowSelectorController::OnSelectionEnded
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=553250:553259
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=555046:555052

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

Additional requirements: Requires Gestures

Additional requirements: Requires HTTP

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 16 by ClusterFuzz, May 2 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 5109801739354112 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 17 by bugdroid1@chromium.org, May 2 2018

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

commit c177375eaedbf86e248c409c53f7cb2d82cc2958
Author: Min Chen <minch@google.com>
Date: Wed May 02 21:15:20 2018

Fix WorkspaceLayoutManagerBackdropTest.OpenAppListInOverviewMode test
crash in mash.

Add RunAllPendingInMessageLoop to make it happy in mash.

Bug:  838595 ,  837092 
Change-Id: Ib5fbd2857c41fc4de3254b6f31a4bb6809864d76
Reviewed-on: https://chromium-review.googlesource.com/1040533
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555538}
[modify] https://crrev.com/c177375eaedbf86e248c409c53f7cb2d82cc2958/ash/wm/workspace/workspace_layout_manager_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, May 4 2018

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

commit 6d730ee7e25d0c76ea769914442de439f32c3611
Author: Min Chen <minch@google.com>
Date: Fri May 04 18:44:38 2018

Disable WorkspaceLayoutManagerBackdropTest.OpenAppListInOverviewMode in asan.

Bug: 838756,  838822 ,  837092 
Change-Id: I9f7fcb79a8305632ca264324f69aee1ae7d2ac71
Reviewed-on: https://chromium-review.googlesource.com/1042868
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556132}
[modify] https://crrev.com/6d730ee7e25d0c76ea769914442de439f32c3611/ash/wm/workspace/workspace_layout_manager_unittest.cc

Cc: dcasta...@chromium.org
 Issue 839434  has been merged into this issue.
Issue 840042 has been merged into this issue.
literally just happened while looking up this bug again.  Report ID b0b9fea8b30cf45d

Chrome Version: 67.0.3396.26 (Official Build) beta (64-bit)
I can confirm that this is happening on 67.0.3396.26 beta. Not entirely sure where the fix was checked in.

Happened when I was trying to reproduce this issue following the steps outlined here: http://listnr/product/208/report/85413694338
I think the fix was merged into  67.0.3396.30
https://chromium-review.googlesource.com/c/chromium/src/+/1037443

Sign in to add a comment