New issue
Advanced search Search tips

Issue 922061 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 16
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Creation and immediate-ish deletion of a CommandBufferGLTestContext crashes

Project Member Reported by robertph...@google.com, Jan 15

Issue description

Skia uses the CommandBuffer directly to test rendering behavior. This appears to have been broken since mid-June.

Here are the Skia-side bugs that show the crash:

https://bugs.chromium.org/p/skia/issues/detail?id=6951 - original bug from June
https://bugs.chromium.org/p/skia/issues/detail?id=8681 - new bug

The causal Chrome-side CL appears to be:

https://chromium-review.googlesource.com/c/chromium/src/+/1105466
 
Cc: piman@chromium.org
Looks like there's a bug in the egl emulation layer, here: https://cs.chromium.org/chromium/src/gpu/gles2_conform_support/egl/context.cc?dr=CSs&sq=package:chromium&g=0&l=358
It destroys the service side (decoder_) before the client side (gles2_cmd_helper_), which causes UAF if there are pending flushes at destruction time (which James's change might have exposed - but shouldn't be a problem).

Moving the decoder_ destruction to the end of the function should do it.

Comment 3 by rmis...@google.com, Jan 16 (6 days ago)

Labels: -Pri-3 Pri-2
Since it looks like the fix is described in https://bugs.chromium.org/p/chromium/issues/detail?id=922061#c2
Who should make the change? (this is currently blocking one of our automated processes)
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit 84497a910c681c722cd280a7a70d6bbbb6a932b5
Author: Robert Phillips <robertphillips@google.com>
Date: Wed Jan 16 18:42:49 2019

Move destruction of GLES2Decoder to end of Context::DestroyService to avoid order issue

Bug:  922061 
Change-Id: Ic2359837cb0502cbfbf2e8a6d918e2ce288afb35
Reviewed-on: https://chromium-review.googlesource.com/c/1414760
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Cr-Commit-Position: refs/heads/master@{#623310}
[modify] https://crrev.com/84497a910c681c722cd280a7a70d6bbbb6a932b5/gpu/gles2_conform_support/egl/context.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/1951aecfd3d7064906c0e964c189e59317aadbef

commit 1951aecfd3d7064906c0e964c189e59317aadbef
Author: Robert Phillips <robertphillips@google.com>
Date: Wed Jan 16 21:06:05 2019

Manually roll Chrome for CommandBuffer

TBR=borenet@google.com
Bug:  922061 
Change-Id: Iaaec2c3c961d6050ddedc4d6fd3a10f5970ca53d
Reviewed-on: https://skia-review.googlesource.com/c/184063
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Eric Boren <borenet@google.com>

[modify] https://crrev.com/1951aecfd3d7064906c0e964c189e59317aadbef/DEPS

Comment 6 by robertph...@google.com, Jan 16 (6 days ago)

Status: Fixed (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit 87271ef3f7b7ab3688440de86aa415fb64ea0ba6
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Wed Jan 16 23:40:57 2019

Roll src/third_party/skia 1374c85fbf53..a30c755f95a3 (8 commits)

https://skia.googlesource.com/skia.git/+log/1374c85fbf53..a30c755f95a3


git log 1374c85fbf53..a30c755f95a3 --date=short --no-merges --format='%ad %ae %s'
2019-01-16 rmistry@google.com Reland "Remove android framework compile bots from CQ"
2019-01-16 robertphillips@google.com Manually roll Chrome for CommandBuffer
2019-01-16 herb@google.com Rename GrGlyphCache -> GrStrikeCache
2019-01-16 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/angle2 ab2bfa814cee..dd34b3b9b707 (2 commits)
2019-01-16 recipe-roller@chromium.org Roll recipe dependencies (trivial).
2019-01-16 reed@google.com Revert "remove SK_SUPPORT_LEGACY_CANVAS_DRAW_TEXT flag"
2019-01-16 herb@google.com Rename SkGlyphCache -> SkStrike
2019-01-16 michaelludwig@google.com Implement generic draw-as-clear fallback for color and stencil


Created with:
  gclient setdep -r src/third_party/skia@a30c755f95a3

The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:922061 
TBR=halcanary@chromium.org

Change-Id: Ied2ba614657bba099bdcb89b1193b309eaec2bf8
Reviewed-on: https://chromium-review.googlesource.com/c/1416350
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#623453}
[modify] https://crrev.com/87271ef3f7b7ab3688440de86aa415fb64ea0ba6/DEPS

Sign in to add a comment