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

Issue 594129 link

Starred by 1 user
Status: Fixed
Owner:
█ sigbjo...@opera.com
Email to this user bounced
Closed: Mar 2016
Cc:
█ kbr@chromium.org
█ sigbjo...@opera.com
haraken@chromium.org
zmo@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Use-after-poison in willDestroyContext

Reported by chamal.d...@gmail.com, Mar 11 2016 
VULNERABILITY DETAILS
Attached test case will crash with Use-after-poison in willDestroyContext.

VERSION
Chrome Version: [51.0.2674.0 (64-bit)] + [TOT]
                [50.0.2661.26 (64-bit)] + [beta]
Operating System: [Ubuntu Linux 14.04]

REPRODUCTION CASE
Your graphics driver should support WEBGL_lose_context extension.

1. Run chrome built with Address Sanitizer with this flag.
   --js-flags="--expose-gc"
2. Open test.html.
3. Wait for about 5 seconds.
4. Tab will crash.

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: [tab]
Crash State: [Address Sanitizer output]

==6874==ERROR: AddressSanitizer: use-after-poison on address 0x7e897f4886c0 at pc 0x55bd65a95d4a bp 0x7fff58817d90 sp 0x7fff58817d88
READ of size 1 at 0x7e897f4886c0 thread T0 (chrome)
    #0 0x55bd65a95d49 in willDestroyContext third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:213:30
    #1 0x55bd65a9f6f5 in ~WebGLRenderingContextBase third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp:1093:5
    #2 0x55bd63f72e86 in finalize third_party/WebKit/Source/platform/heap/HeapPage.cpp:104:9
    #3 0x55bd63f795cd in sweep third_party/WebKit/Source/platform/heap/HeapPage.cpp:1150:13
    #4 0x55bd63f74f70 in sweepUnsweptPage third_party/WebKit/Source/platform/heap/HeapPage.cpp:318:9
    #5 0x55bd63f74f70 in completeSweep third_party/WebKit/Source/platform/heap/HeapPage.cpp:359:0
    #6 0x55bd63f8f8b1 in eagerSweep third_party/WebKit/Source/platform/heap/ThreadState.cpp:1079:5
    #7 0x55bd63f8f0f7 in preSweep third_party/WebKit/Source/platform/heap/ThreadState.cpp:1027:5
    #8 0x55bd63f6f3f0 in ~SafePointScope third_party/WebKit/Source/platform/heap/SafePoint.h:29:13
    #9 0x55bd63f6f3f0 in collectGarbage third_party/WebKit/Source/platform/heap/Heap.cpp:438:0
    #10 0x55bd63f856df in performIdleGC third_party/WebKit/Source/platform/heap/ThreadState.cpp:756:5
    #11 0x55bd6b6331a6 in runIdleTask components/scheduler/child/web_scheduler_impl.cc:44:3
    #12 0x55bd6b633de7 in Run<std::__1::unique_ptr<blink::WebThread::IdleTask, std::__1::default_delete<blink::WebThread::IdleTask> >, base::TimeTicks> base/bind_internal.h:159:12
    #13 0x55bd6b633de7 in MakeItSo<std::__1::unique_ptr<blink::WebThread::IdleTask, std::__1::default_delete<blink::WebThread::IdleTask> >, base::TimeTicks> base/bind_internal.h:301:0
    #14 0x55bd6b633de7 in Run base/bind_internal.h:352:0
    #15 0x55bd6b6319b3 in Run base/callback.h:397:12
    #16 0x55bd6b6319b3 in RunTask components/scheduler/child/single_thread_idle_task_runner.cc:75:0
    #17 0x55bd6b632621 in Run<const base::Callback<void (base::TimeTicks), base::internal::CopyMode::Copyable> &> base/bind_internal.h:181:12
    #18 0x55bd6b632621 in MakeItSo<base::WeakPtr<scheduler::SingleThreadIdleTaskRunner>, const base::Callback<void (base::TimeTicks), base::internal::CopyMode::Copyable> &> base/bind_internal.h:314:0
    #19 0x55bd6b632621 in Run base/bind_internal.h:352:0
    #20 0x55bd61b21814 in Run base/callback.h:397:12
    #21 0x55bd61b21814 in RunTask base/debug/task_annotator.cc:51:0
    #22 0x55bd6b63abff in ProcessTaskFromWorkQueue components/scheduler/base/task_queue_manager.cc:288:3
    #23 0x55bd6b6378dd in DoWork components/scheduler/base/task_queue_manager.cc:200:13
    #24 0x55bd6b63c7d4 in Run<const base::TimeTicks &, const bool &> base/bind_internal.h:181:12
    #25 0x55bd6b63c7d4 in MakeItSo<base::WeakPtr<scheduler::TaskQueueManager>, const base::TimeTicks &, const bool &> base/bind_internal.h:314:0
    #26 0x55bd6b63c7d4 in Run base/bind_internal.h:352:0
    #27 0x55bd61b21814 in Run base/callback.h:397:12
    #28 0x55bd61b21814 in RunTask base/debug/task_annotator.cc:51:0
    #29 0x55bd61a2abd9 in RunTask base/message_loop/message_loop.cc:476:3
    #30 0x55bd61a2b695 in DeferOrRunPendingTask base/message_loop/message_loop.cc:485:5
    #31 0x55bd61a2bfec in DoWork base/message_loop/message_loop.cc:597:13
    #32 0x55bd61a32c30 in Run base/message_loop/message_pump_default.cc:33:21
    #33 0x55bd61a72b95 in Run base/run_loop.cc:35:3
    #34 0x55bd61a2931e in ?? base/message_loop/message_loop.cc:293:3
    #35 0x55bd6b785e02 in RendererMain content/renderer/renderer_main.cc:225:7
    #36 0x55bd61937fae in RunZygote content/app/content_main_runner.cc:316:14
    #37 0x55bd6193aa6d in Run content/app/content_main_runner.cc:766:12
    #38 0x55bd619372ba in ContentMain content/app/content_main.cc:19:15
    #39 0x55bd60d55a85 in ChromeMain chrome/app/chrome_main.cc:84:12
    #40 0x7fa1e2ce6ec4 in __libc_start_main /build/eglibc-3GlaMS/eglibc-2.19/csu/libc-start.c:287:0
test.html
362 bytes View Download

Comment 1 by infe...@chromium.org, Mar 11 2016

Cc: kbr@chromium.org
Labels: Security_Severity-Medium Security_Impact-Stable OS-All Pri-1
Owner: zmo@chromium.org
Status: Assigned (was: Unconfirmed)

Comment 2 by infe...@chromium.org, Mar 11 2016

Components: Blink>WebGL
Labels: M-50

Comment 3 by kbr@chromium.org, Mar 14 2016

Cc: zmo@chromium.org
Owner: kbr@chromium.org

Comment 4 by kbr@chromium.org, Mar 15 2016

Cc: haraken@chromium.org sigbjo...@opera.com
Components: Blink>MemoryAllocator>GarbageCollection
Status: Started (was: Assigned)
The problem is that WebGLRenderingContext::~WebGLRenderingContext indirectly violates the Oilpan invariant that the destructors of finalized GC'd objects may not touch other on-heap objects.

Multiple WebGLRenderingContext instances are being collected in the same GC cycle, but WebGLRenderingContextBase::willDestroyContext (called from ~WebGLRenderingContext) is iterating down the list of forcibly evicted contexts, which includes other contexts that will be collected in this GC cycle.

I'm not 100% sure how to resolve this. Plausibly the restoration of forcibly evicted contexts that willDestroyContext does could be delayed via postTask, so that all of the dead contexts would be cleared out of the activeContexts and forciblyEvictedContexts static lists. haraken@, sigbjornf@: does that sound like a viable approach?

Comment 5 by sigbjo...@opera.com, Mar 15 2016 

I don't understand the failure condition yet to be able to answer well. forciblyEvictedContexts() holds weak references to the context objects, so none of the WebGLRenderingContextBases that are finalized in the same GC cycle should be in that map when willDestroyContext() runs.

Comment 6 by kbr@chromium.org, Mar 15 2016

Owner: sigbjo...@opera.com
forciblyEvictedContexts is a WillBePersistentHeapHashMap<RawPtrWillBeWeakMember<WebGLRenderingContextBase>, int> :

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp&q=WebGLRenderingContextBase.cpp&sq=package:chromium&type=cs&l=115

These maps are maintained manually via calls to activateContext and deactivateContext.

Appreciate some help and guidance here. Sigbjorn, can I assign this to you briefly? I will be traveling for the next two days, so won't be able to make progress on this.

Comment 7 by sigbjo...@opera.com, Mar 15 2016 

Maybe there's something unexpected happening with weak keys (rather than values), but I can investigate a bit. Travelling for the next 24 hours also.

Comment 8 by sigbjo...@opera.com, Mar 16 2016 

Now I understand - this is a false positive from our ASan poisoning of the eager heap -- disallowing any kind of access to that heap while finalizing objects within it.

Will address, but the failure here is limited to ASan and not present in production builds.
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 16 2016 

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

commit 0e0025da360e1f9793712c9c772fb76e1b4c2160
Author: sigbjornf <sigbjornf@opera.com>
Date: Wed Mar 16 21:41:30 2016

(Only) poison unmarked heap objects prior to sweeping.

Drop the unnecessary restriction that eagerly finalized objects aren't
allowed to touch access other eagerly finalized, but live, objects during
finalization. They're allowed to access live objects in other heaps/arenas,
so the same-heap restriction makes little sense.

Simplify the HeapPage poisoning methods as a result.

R=haraken
BUG= 594129 

Review URL: https://codereview.chromium.org/1805343004

Cr-Commit-Position: refs/heads/master@{#381554}

[modify] https://crrev.com/0e0025da360e1f9793712c9c772fb76e1b4c2160/third_party/WebKit/Source/platform/heap/BlinkGC.h
[modify] https://crrev.com/0e0025da360e1f9793712c9c772fb76e1b4c2160/third_party/WebKit/Source/platform/heap/HeapPage.cpp
[modify] https://crrev.com/0e0025da360e1f9793712c9c772fb76e1b4c2160/third_party/WebKit/Source/platform/heap/HeapPage.h
[modify] https://crrev.com/0e0025da360e1f9793712c9c772fb76e1b4c2160/third_party/WebKit/Source/platform/heap/ThreadState.cpp
[modify] https://crrev.com/0e0025da360e1f9793712c9c772fb76e1b4c2160/third_party/WebKit/Source/platform/heap/ThreadState.h

Comment 10 by sigbjo...@opera.com, Mar 17 2016

Status: Fixed (was: Started)

Comment 11 by sigbjo...@opera.com, Mar 17 2016

Labels: -Type-Bug-Security Type-Bug
Changing type from Bug-Security to Bug; ASan only.

Comment 12 by kbr@chromium.org, Mar 21 2016 

Sigbjorn, thanks very much for taking this.

Anyone reviewing this afterward: please note also that https://codereview.chromium.org/1815513002/ was a cleanup related to this fix.

Comment 13 by mbarbe...@chromium.org, Apr 12 2016

Labels: -Restrict-View-SecurityTeam -Security_Impact-Stable -Security_Severity-Medium
►

Sign in to add a comment