elements_upload_data_stream.cc(78)] Check failed: result != ERR_IO_PENDING || !reader->IsInMemory(). |
||||||
Issue descriptionVersion: ToT dad6ea0b1c74c5ebb7feef966db51970b6049fed OS: Linux What steps will reproduce the problem? (1) Build chrome with dchecks enabled. (2) Make an upload-stream.html file with this content: <!DOCTYPE html> <meta charset="utf-8"> <title>Page Title</title> <script> var blob = new Blob(['it\'s me the blob', new Float64Array(1024*1024), 'and more blob!']); fetch('resources/dummy.html', { method: 'POST', body: blob }) .then(resp => resp.text()) .then(text => { console.log('OK: ' + text); }) .catch(error => { console.log('FAIL: ' + text); }); </script> (3) Run an HTTP server that serves that file. (4) Navigate to the file. What is the expected output? No crash. What do you see instead? [2507:2535:0512/154821:FATAL:elements_upload_data_stream.cc(78)] Check failed: result != ERR_IO_PENDING || !reader->IsInMemory(). #0 0x7f8bc4613ffe base::debug::StackTrace::StackTrace() #1 0x7f8bc46346db logging::LogMessage::~LogMessage() #2 0x7f8bc3f61762 net::ElementsUploadDataStream::InitElements() #3 0x7f8bc3f7ee47 net::UploadDataStream::Init() #4 0x7f8bc40591f6 net::HttpNetworkTransaction::DoLoop() #5 0x7f8bc405abc0 net::HttpNetworkTransaction::OnStreamReady() #6 0x7f8bc4086024 net::HttpStreamFactoryImpl::Request::OnStreamReady() #7 0x7f8bc407fa34 net::HttpStreamFactoryImpl::Job::OnStreamReadyCallback() #8 0x7f8bc3f32f2c _ZN4base8internal7InvokerINS_13IndexSequenceIJLm0EEEENS0_9BindStateINS0_15RunnableAdapterIMN3net12_GLOBAL__N_115QuicChromeAlarmEFvvEEEFvPS8_EJNS_7WeakPtrIS8_EEEEENS0_12InvokeHelperILb1EvSB_EEFvvEE3RunEPNS0_13BindStateBaseE #9 0x7f8bc4615209 base::debug::TaskAnnotator::RunTask() #10 0x7f8bc463ee35 base::MessageLoop::RunTask() #11 0x7f8bc463f168 base::MessageLoop::DeferOrRunPendingTask() #12 0x7f8bc463f4fb base::MessageLoop::DoWork() #13 0x7f8bc46425f9 base::MessagePumpLibevent::Run() #14 0x7f8bc463e961 base::MessageLoop::RunHandler() #15 0x7f8bc466d610 base::RunLoop::Run() #16 0x7f8bc463da70 base::MessageLoop::Run() #17 0x7f8bc1ffdba6 content::BrowserThreadImpl::IOThreadRun() #18 0x7f8bc1ffddb1 content::BrowserThreadImpl::Run() #19 0x7f8bc46a16d3 base::Thread::ThreadMain() #20 0x7f8bc469b055 base::(anonymous namespace)::ThreadFunc() #21 0x7f8bbcabf182 start_thread #22 0x7f8bbb76247d clone This seems to require triggering async blob construction with the big blob.
,
May 12 2016
IsInMemory is basically used to check if we can read from the data synchronously. If we can, we'll try to merge the headers and post body, if it's small enough. If an UploadElementReader can't do stuff synchronously, it should just be returning false in response to IsInMemory. This is a real bug, at least if read operations on that reader also don't complete synchronously, that will result in accessing out of bounds memory in release builds.
,
May 12 2016
,
May 19 2016
Matt: What's the next step on this bug? I see "accessing out of bounds memory in release builds" and think "pri-1, security-review-needed", but you didn't do that, so I presume there's something I'm missing?
,
May 19 2016
Where do you see out of bound memory access? The crash trace in post #1 shows a CHECK failure. This is a bug in the Blob code, which I'm unfamiliar with. I'd attach a label for that, but I don't know what the right label is, so I CCed the owner of the relevant directory. A blob is claiming it's in memory, but initialization isn't completing synchronously, so it's apparently not in memory.
,
May 19 2016
Err post #0, rather.
,
May 19 2016
I was referring to your c#2: "... at least if read operations on that reader also don't complete synchronously, that will result in accessing out of bounds memory in release builds." Given the lack of response in a week, I'll escalate slightly. But I'm still wondering if we should get the security machinery involved based on your analysis (I don't know the blog code, I'm just reacting to your comment.)
,
May 19 2016
oof, sorry guys this slipped past my filters, I had another blob issue that took up my time. I'll investigate soon.
,
May 19 2016
rdsmith: Ahh, I meant the CHECK exists for a reason. Otherwise, we could just remove the CHECK, and problem solved!
,
May 19 2016
Ah, got it! Sorry. Ok, I won't worry about escalating the priority on this, but assume that dmurph@ will get to it when when they have a chance.
,
May 20 2016
So this can happen if the blob isn't constructed yet. The return value is IO_PENDING, as the blob isn't constructed yet, but since we think the blob doesn't exist we return true here: https://code.google.com/p/chromium/codesearch#chromium/src/storage/browser/blob/blob_reader.cc&sq=package:chromium&type=cs&l=230&rcl=1463699816 So idk what we want to do here, Martin do you have any preference? We could maybe return false in the reader (not sure what side effects that would have), or just don't have this dcheck, or adds the condition that it can be a blob & pending and that's fine?
,
May 20 2016
we shouldn't return false, as that's actually the case if the blob doesn't exist (we don't have blob_data_ until the async construction callback is triggered). So maybe just remove the check? there's no way for us to know if it's a blob, right?
,
May 20 2016
ooor we can have the blob reader return false if it's being built, because technically it's not in the 'browser' 's memory yet
,
May 20 2016
The method was added exactly to mean if it's in the browser process's memory. It's used to know if operations will complete synchronously - if they will, and it's a small enough upload body, we'll merge it with the headers, so we can send them in one packet. That logic doesn't support async reads.
,
May 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f4afcf1730afbeaf79d6c115d8e92d7708da931 commit 1f4afcf1730afbeaf79d6c115d8e92d7708da931 Author: dmurph <dmurph@chromium.org> Date: Fri May 20 19:51:55 2016 [BlobReader] Correct IsInMemory = false when async constructing BUG= 611317 Review-Url: https://codereview.chromium.org/2000483004 Cr-Commit-Position: refs/heads/master@{#395142} [modify] https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931/content/browser/blob_storage/blob_reader_unittest.cc [modify] https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931/net/base/upload_element_reader.h [modify] https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931/storage/browser/blob/blob_reader.cc [modify] https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931/storage/browser/blob/blob_reader.h
,
May 20 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by falken@chromium.org
, May 12 2016