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

Issue 715974 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

Crash in net::HttpCache::Transaction::~Transaction

Project Member Reported by ClusterFuzz, Apr 27 2017

Issue description

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

Fuzzer: inferno_webbot
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: UNKNOWN READ
Crash Address: 0x000000000008
Crash State:
  net::HttpCache::Transaction::~Transaction
  net::HttpCache::Transaction::~Transaction
  net::URLRequestHttpJob::Kill
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=467403:467472

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


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: msrchandra@chromium.org
Components: Internals>Network
Labels: M-60 Test-Predator-Correct-CLs
Owner: shivanisha@chromium.org
Status: Assigned (was: Untriaged)
Assigning to the concern owner from Predator results -- 
The result is a list of CLs that change the crashed files. 

Author: shivanisha
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1e2e347f957ef889aaee527bb757849f76e8a808
Time: Wed Apr 26 20:09:12 2017
Lines 202-205 of file http_cache_transaction.cc which potentially caused crash are changed in this cl (frame #6, "net::HttpCache::Transaction::~Transaction"). 

Lines 929 of file http_cache.cc which potentially caused crash are changed in this cl (frame #5, "net::HttpCache::DoneReadingFromEntry").
Minimum distance from crash line to modified line: 0. (file: http_cache.cc, crashed on: 929, modified: 929).

@shivanisha -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
This CL has been reverted. I am looking into the issue. Thanks
Reverting CL: https://codereview.chromium.org/2847653002/
Project Member

Comment 3 by ClusterFuzz, Apr 28 2017

ClusterFuzz has detected this issue as fixed in range 467546:467557.

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

Fuzzer: inferno_webbot
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: UNKNOWN READ
Crash Address: 0x000000000008
Crash State:
  net::HttpCache::Transaction::~Transaction
  net::HttpCache::Transaction::~Transaction
  net::URLRequestHttpJob::Kill
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=467403:467472
Fixed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=467546:467557

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


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs 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 4 by ClusterFuzz, Apr 28 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
CL https://codereview.chromium.org/2721933002/ "Patch 35: Fixed partial transactions handling in ActiveEntry" also fixes the clusterfuzz reported issue:
Here are the details:

Tested the clusterfuzz test reported in this bug and consistently the crash is coming locally on the reverted CL with the URL: https://aeginaportal.gr/
command: ./out/asan/chrome --no-sandbox | tools/valgrind/asan/asan_symbolize.py
gn args:
enable_ipc_fuzzer = true
ffmpeg_branding = "Chrome"
is_component_build = false
proprietary_codecs = true
sanitizer_coverage_flags = "edge"
v8_enable_verify_heap = true
is_asan = true
is_lsan = true
enable_nacl = false
is_debug = false

The same command , gn args and the same URL do not crash on the latest patch.

The crash reason was that DoneReadingFromEntry was being invoked for a transaction incorrectly. It should have been found in done_headers_queue/headers_transaction/writer but it incorrectly did not, thus assuming it should be a reader.

"Patch 35: Fixed partial transactions handling in ActiveEntry" fixed this issue.
Before the fix the state transition in DoneWithResponseHeaders looked like this:
   if (transaction->mode() & Transaction::WRITE) {
     DCHECK(entry->done_headers_queue.empty());
     DCHECK(!entry->writer);
     entry->writer = transaction;
Since dchecks were not getting hit in the asan build that clusterfizz was using , instead of failing at DCHECK(!entry->writer), it was actually over-writing writer transaction with another transaction and thus losing the pointer to the original writer.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 13 2017

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

commit 8061c420676998bda77caa74581ea8061860f438
Author: shivanisha <shivanisha@chromium.org>
Date: Tue Jun 13 23:35:52 2017

This CL is a precursor to allowing shared writing to fix cache lock.

This CL allows transactions to continue to their validation phase even when another
transaction is the active reader/writer. After the validation phase, if its a match
the transaction might wait till the response is written to the cache by the active
writer. If its not a match the transaction will doom the entry and go to the
network. In a subsequent CL, the not matching case will create a new entry as well.

BUG= 472740 ,  715913 ,  715974 ,  715920 ,  715911 ,  713348 

Review-Url: https://codereview.chromium.org/2721933002
Cr-Original-Commit-Position: refs/heads/master@{#467426}
Committed: https://chromium.googlesource.com/chromium/src/+/1e2e347f957ef889aaee527bb757849f76e8a808
Review-Url: https://codereview.chromium.org/2721933002
Cr-Commit-Position: refs/heads/master@{#479204}

[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache.h
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache_transaction.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache_transaction.h
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_cache_unittest.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_transaction.h
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_transaction_test_util.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/http_transaction_test_util.h
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/mock_http_cache.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/http/mock_http_cache.h
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/url_request/url_request_http_job_unittest.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/url_request/url_request_quic_unittest.cc
[modify] https://crrev.com/8061c420676998bda77caa74581ea8061860f438/net/url_request/url_request_unittest.cc

Sign in to add a comment