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

Issue 455368 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment

UNKNOWN in blink::SQLStatementBackend::execute

Project Member Reported by ClusterFuzz, Feb 4 2015

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4615025003069440

Fuzzer: Therealholden_worker
Job Type: Linux_asan_chrome_v8_arm

Crash Type: UNKNOWN
Crash Address: 0x44c7240c
Crash State:
  blink::SQLStatementBackend::execute
  blink::SQLTransactionBackend::runCurrentStatementAndGetNextState
  blink::SQLTransactionBackend::runStatements
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_v8_arm&range=296680:296701

Minimized Testcase (1.84 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96GDuoHsgIC6QCI3mZLHbeyfbRh7cxlFA_r8DE1D5-pCenDv4_CXZaj_04cZ1LJGcIr6Zsj8q3FNrMGNyUozSRlr36_YCLQ91e_7XeVpjfs60YsmVoW16Z01xnRtA7PZO0QPAfsOqwAazfbOnXL0nThmXFoyA

Additional requirements: Requires Gestures

Additional requirements: Requires HTTP

Filer: mbarbella
 
Project Member

Comment 1 by ClusterFuzz, Feb 4 2015

Labels: M-40 Pri-1
Cc: tkent@chromium.org michaeln@chromium.org
Owner: keishi@chromium.org
Status: Assigned
keishi@ - Please investigate this stale pointer appears to be a regression from this CL: https://codereview.chromium.org/563703002

Also CC'ing michaeln@ as owner and tkent@ who also has a CL in range (but that one looks innocuous)

Cc: haraken@chromium.org
+haraken, since he has a CL in range that looks like the potential culprit as well.
Owner: tkent@chromium.org
tkent-san: Would you mind taking a look at this?

Comment 5 by tkent@chromium.org, Feb 10 2015

I couldn't reproduce this locally, and I found no suspicious code in modules/webdatabas/.

The testcase contains window.close().  I guess the Oilpan heap for the main thread is destructed before the database thread.
 Issue 450051  might be the same bug.  Its testcase contains location.reload().

When is the oilpan heap destroyed in the window.close() sequence? Can you point me to a function/method name for where the heap is destroyed so I can observe that. The database thread should be done upon return from DatabaseContext::stopDatabases(). Is the heap is destroyed prior to that?

Comment 7 by tkent@chromium.org, Feb 10 2015

In window.close() case, the oilpan heap for the Blink main thread is terminated at:

   RenderThreadImpl::Shutdown -> blink::shutdown (WebKit.cpp) -> ThreadState::detachMainThread()

and DatabaseContext::stopDatabases() is not called at all.  In blink::shutdown, all of worker threads and the HTML parser thread are terminated explicitly.  Probably we need to terminate database threads here.

On the other hand, location.reload() seems to call stopDatabases().   Issue 450051  is a different bug.

Comment 8 by tkent@chromium.org, Feb 10 2015

Labels: -Cr-Blink-Workers Cr-Blink-Storage
Status: Started
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 12 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=190021

------------------------------------------------------------------
r190021 | tkent@chromium.org | 2015-02-12T04:28:48.618712Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/InitModules.h?r1=190021&r2=190020&pathrev=190021
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/webdatabase/DatabaseManager.cpp?r1=190021&r2=190020&pathrev=190021
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/webdatabase/DatabaseManager.h?r1=190021&r2=190020&pathrev=190021
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebKit.cpp?r1=190021&r2=190020&pathrev=190021
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/InitModules.cpp?r1=190021&r2=190020&pathrev=190021

Web SQL: Termiante database thread before finalizing the Oilpan heap for the Blink main thread.

Oilpan heap for the database thread should be finalized before finalizing the
main thread heap. Usually it's done in ExecutionContext::stopActiveDOMObjects().
When blink::shutdown() is called, stopActiveDOMObjects() was not called and
objects owned by the main thread is finalized though they were referred by the
database thread.  blink::shutdown() should terminate the database thread
explicitly.

BUG= 455368 , 455789 

Review URL: https://codereview.chromium.org/892343003
-----------------------------------------------------------------
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 12 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=190035

------------------------------------------------------------------
r190035 | tkent@chromium.org | 2015-02-12T09:54:34.322010Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/webdatabase/DatabaseManager.cpp?r1=190035&r2=190034&pathrev=190035
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/webdatabase/DatabaseManager.h?r1=190035&r2=190034&pathrev=190035

DatabaseManager::terminateDatabaseThread should handle multiple DatabaseContexts.

This is a followup of Blink r190021 [1].

[1] http://src.chromium.org/viewvc/blink?view=rev&rev=190021

BUG= 455368 , 455789 

Review URL: https://codereview.chromium.org/913413002
-----------------------------------------------------------------

Comment 11 by tkent@chromium.org, Feb 13 2015

Labels: M-41
Status: Fixed
Project Member

Comment 12 by ClusterFuzz, Feb 13 2015

Labels: -Restrict-View-SecurityTeam Merge-Triage Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz

Comment 13 by tkent@chromium.org, Feb 16 2015

Labels: -Merge-Triage Merge-Requested
Labels: -Merge-Requested Merge-Review Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M40), manual review required.
Labels: Merge-Approved Hotlist-Merge-Approved
Approved for M41 (branch: 2272)
Project Member

Comment 16 by bugdroid1@chromium.org, Feb 16 2015

Labels: -Merge-Approved merge-merged-2272
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=190225

------------------------------------------------------------------
r190225 | tkent@chromium.org | 2015-02-16T07:00:58.213939Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/web/WebKit.cpp?r1=190225&r2=190224&pathrev=190225
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/modules/InitModules.cpp?r1=190225&r2=190224&pathrev=190225
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/modules/InitModules.h?r1=190225&r2=190224&pathrev=190225
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/modules/webdatabase/DatabaseManager.cpp?r1=190225&r2=190224&pathrev=190225
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/modules/webdatabase/DatabaseManager.h?r1=190225&r2=190224&pathrev=190225

Merge 190021 "Web SQL: Termiante database thread before finalizi..."

> Web SQL: Termiante database thread before finalizing the Oilpan heap for the Blink main thread.
> 
> Oilpan heap for the database thread should be finalized before finalizing the
> main thread heap. Usually it's done in ExecutionContext::stopActiveDOMObjects().
> When blink::shutdown() is called, stopActiveDOMObjects() was not called and
> objects owned by the main thread is finalized though they were referred by the
> database thread.  blink::shutdown() should terminate the database thread
> explicitly.
> 
> BUG= 455368 , 455789 
> 
> Review URL: https://codereview.chromium.org/892343003

TBR=tkent@chromium.org

Review URL: https://codereview.chromium.org/930793002
-----------------------------------------------------------------
Project Member

Comment 17 by bugdroid1@chromium.org, Feb 16 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=190226

------------------------------------------------------------------
r190226 | tkent@chromium.org | 2015-02-16T07:03:35.588775Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/modules/webdatabase/DatabaseManager.cpp?r1=190226&r2=190225&pathrev=190226
   M http://src.chromium.org/viewvc/blink/branches/chromium/2272/Source/modules/webdatabase/DatabaseManager.h?r1=190226&r2=190225&pathrev=190226

Merge 190035 "DatabaseManager::terminateDatabaseThread should ha..."

> DatabaseManager::terminateDatabaseThread should handle multiple DatabaseContexts.
> 
> This is a followup of Blink r190021 [1].
> 
> [1] http://src.chromium.org/viewvc/blink?view=rev&rev=190021
> 
> BUG= 455368 , 455789 
> 
> Review URL: https://codereview.chromium.org/913413002

TBR=tkent@chromium.org

Review URL: https://codereview.chromium.org/925263004
-----------------------------------------------------------------
Labels: -M-40 -Merge-Review -Hotlist-Merge-Review -Hotlist-Merge-Approved
Following discussions with Tim Willis, no more security fixes are going to M40.  M41 hits stable in 2 weeks.
Labels: Release-0-M41
Labels: reward-unpaid reward-2500 CVE-2015-1221
Congrats - $2500 for this report.

Notes from reward panel: $2000 for the bug + $500 ClusterFuzz bonus.
Labels: -reward-topanel -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Processing via our e-payment system can take up to six weeks, but the reward should be on its way to you. Thanks again for your help!
Project Member

Comment 23 by ClusterFuzz, May 22 2015

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member

Comment 24 by sheriffbot@chromium.org, Oct 1 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

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

Comment 25 by sheriffbot@chromium.org, Oct 2 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment