1499105 - Assertion failure: mResponseSent, at /builds/worker/workspace/build/src/dom/filehandle/ActorsParent.cpp:2025

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect, P3)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 4a230b07f0cb.  Please note, testcase must be served via a local webserver.

Assertion failure: mResponseSent, at /builds/worker/workspace/build/src/dom/filehandle/ActorsParent.cpp:2025

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x0000000000000b40   rbx = 0x00007fde4b37aca0
rsi = 0x00007fde7e22b8b0   rdi = 0x00007fde7e22a680
rbp = 0x00007fde5d0cc4b0   rsp = 0x00007fde5d0cc4a0
r8 = 0x00007fde7e22b8b0    r9 = 0x00007fde5d0cd700
r10 = 0x0000000000000002   r11 = 0x0000000000000000
r12 = 0x00007fde4b37aa60   r13 = 0x00007fde4b37aca0
r14 = 0x00007fde5d0cc520   r15 = 0x00007fde4b61d438
rip = 0x00007fde6e133683
OS|Linux|0.0.0 Linux 4.15.0-36-generic #39-Ubuntu SMP Mon Sep 24 16:19:09 UTC 2018 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|23
23|0|libxul.so|mozilla::dom::NormalFileHandleOp::Cleanup()|hg:hg.mozilla.org/mozilla-central:dom/filehandle/ActorsParent.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|2025|0x18
23|1|libxul.so|mozilla::dom::FileHandle::RecvPBackgroundFileRequestConstructor(mozilla::dom::PBackgroundFileRequestParent*, mozilla::dom::FileRequestParams const&)|hg:hg.mozilla.org/mozilla-central:dom/filehandle/ActorsParent.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|1926|0xa
23|2|libxul.so|mozilla::dom::PBackgroundFileHandleParent::OnMessageReceived(IPC::Message const&)|s3:gecko-generated-sources:322c8a746d97062b50657d8ee8b45e2bf6130f7edb7e00c5512bc6c39c725b550b110311a71605c8c4c20d741f8094c2d6eefeebebd9650367a1de1deef6800c/ipc/ipdl/PBackgroundFileHandleParent.cpp:|243|0x3
23|3|libxul.so|mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|2248|0x6
23|4|libxul.so|mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|2175|0xb
23|5|libxul.so|mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|2012|0xb
23|6|libxul.so|mozilla::ipc::MessageChannel::MessageTask::Run()|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|2045|0xc
23|7|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|1252|0x15
23|8|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|530|0x11
23|9|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|334|0xa
23|10|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|325|0x17
23|11|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|318|0x8
23|12|libxul.so|nsThread::ThreadFunc(void*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|505|0x8
23|13|libnspr4.so|_pt_root|hg:hg.mozilla.org/mozilla-central:nsprpub/pr/src/pthreads/ptthread.c:4a230b07f0cbf48e87dcb4265ea2d00893bb1b62|201|0x7
23|14|libpthread-2.27.so||||0x76db
23|15|libc-2.27.so||||0x12188f

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/kiqiViulvd1wYWooEY4yfg/index.html

This is hit frequently by the fuzzers and can limit their effectiveness.

:jstutte, is there someone that can help get this resolved?

Comment 2

[Jens Stutte [:jstutte]]

I had a first look at the pernosco session. The creation of a MemoryOutputStream fails here due to an unreasonable big size parameter. The size parameter seems to derive directly from an IPC call. There is some parameter verification ongoing before, but this case is not handled and only covered by the later assertion from MemoryOutputStream such that we probably miss the occasion to fail more early and more gently, leaving it up to the caller to decide if we really need to crash.

However, this does not yet explain, who is making this wrong IPC call, and why (should be visible in the pernosco session, too).

:ssengupta, do you want to dig into this a bit?

Comment 3

[Jan Varga [:janv]]

Yes, IDBFileHandle::CheckStateAndArgumentsForRead needs to check the size (aSize <= UINT32_MAX). That's a first step. We should also enhance FileHandle::VerifyRequestParams to check it as well.

Anyway, this bug was originally filed for asserting on mResponseSent. That seems to be another issue. I think, this happens when we call op->Cleanup() after failed op->Init() in FileHandle::RecvPBackgroundFileRequestConstructor.
The assertion needs to be modified probably to cover the case when we fail very early. A similar pattern is used in IndexedDB, for example in TransactionBase::StartRequest and NormalTransactionOp::Cleanup has the same assertion which needs to be modified too I think.

Comment 4

[Subhamoy [:ssengupta][he/him]]

Attachment: Bug 1499105 - P1 - IDBFileHandle::Read() now throws error and returns nullptr if read size larger than 4GB r=janv

Comment 5

[Subhamoy [:ssengupta][he/him]]

Attachment: Bug 1499105 - P2 - NormalFileHandleOp::Cleanup() no longer asserts mResponseSent when called after failed initialization r=janv

Comment 6

[Subhamoy [:ssengupta][he/him]]

Attachment: Bug 1499105 - P3 - Mochitest updated to confirm IDBFileHandle::Read() throws on data larger than 4GB r=janv

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9dc74ad18069
P2 - NormalFileHandleOp::Cleanup() no longer asserts mResponseSent when called after failed initialization r=janv

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/9dc74ad18069

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/874ea520c496
P1 - IDBFileHandle::Read() now throws error and returns nullptr if read size larger than 4GB r=janv,sg

Comment 10

[Alexandru Michis [:malexandru]]

Backed out changeset 874ea520c496 for causing bustages in dom/filehandle/ActorsParent.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/721687f73354010e8fd9f850a4962081188d7247

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=598800&resultStatus=testfailed%2Cbusted%2Cexception&revision=874ea520c496c61eb4a1b289d2e7fd3d0c532667

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284667588&repo=autoland&lineNumber=30245

[task 2020-01-13T13:56:52.461Z] 13:56:52 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media'
[task 2020-01-13T13:56:52.462Z] 13:56:52 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/ipc'
[task 2020-01-13T13:56:52.463Z] 13:56:52 INFO - dom/media/ipc/RDDProcessManager.o
[task 2020-01-13T13:56:52.463Z] 13:56:52 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/ipc'
[task 2020-01-13T13:56:52.762Z] 13:56:52 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/filehandle'
[task 2020-01-13T13:56:52.763Z] 13:56:52 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_dom_filehandle0.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/filehandle -I/builds/worker/workspace/build/src/obj-firefox/dom/filehandle -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -MD -MP -MF .deps/Unified_cpp_dom_filehandle0.o.pp Unified_cpp_dom_filehandle0.cpp
[task 2020-01-13T13:56:52.763Z] 13:56:52 INFO - In file included from Unified_cpp_dom_filehandle0.cpp:2:
[task 2020-01-13T13:56:52.764Z] 13:56:52 ERROR - /builds/worker/workspace/build/src/dom/filehandle/ActorsParent.cpp:1402:36: error: result of comparison of constant 4294967295 with expression of type 'const bool' is always false [-Werror,-Wtautological-constant-out-of-range-compare]
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - if (NS_WARN_IF(params.size() > UINT32_MAX)) {
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - ~~~~~~~~~~~~~ ^ ~~~~~~~~~~
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsDebug.h:61:23: note: expanded from macro 'NS_WARN_IF'
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - NS_warn_if_impl(condition, #condition, FILE, LINE)
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - ^~~~~~~~~
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - 1 error generated.
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - /builds/worker/workspace/build/src/config/rules.mk:744: recipe for target 'Unified_cpp_dom_filehandle0.o' failed
[task 2020-01-13T13:56:52.764Z] 13:56:52 ERROR - make[4]: *** [Unified_cpp_dom_filehandle0.o] Error 1
[task 2020-01-13T13:56:52.764Z] 13:56:52 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/filehandle'
[task 2020-01-13T13:56:52.765Z] 13:56:52 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'dom/filehandle/target-objects' failed
[task 2020-01-13T13:56:52.765Z] 13:56:52 ERROR - make[3]: *** [dom/filehandle/target-objects] Error 2
[task 2020-01-13T13:56:52.765Z] 13:56:52 INFO - make[3]: *** Waiting for unfinished jobs....

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc86b21e81bd
P1 - IDBFileHandle::Read() now throws error and returns nullptr if read size larger than 4GB r=janv,sg

Comment 12

[Subhamoy [:ssengupta][he/him]]

'Have tagged a revised version for check-in.

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/dc86b21e81bd

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f54eafdb959
P3 - Mochitest updated to confirm IDBFileHandle::Read() throws on data larger than 4GB r=janv

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/8f54eafdb959