Closed Bug 1125719 Opened 10 years ago Closed 10 years ago

[Bluetooth] Redial from bluetooth device cause bluetoothd and gecko crash.

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S5 (6feb)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: brsun, Assigned: tzimmermann)

Details

Attachments

(2 files, 1 obsolete file)

The scenario can refer [1] for more details. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1124760#c14
Gecko call stacks: Program received signal SIGSEGV, Segmentation fault. 0xb6e8b6ea in memmove (dst0=0xb6578efc <mozilla::CountingAllocatorBase<NesteggReporter>::sAmount>, src0=<optimized out>, length=4294967292) at bionic/libc/upstream-openbsd/lib/libc/string/bcopy.c:97 97 TLOOP(*(word *)dst = *(word *)src; src += wsize; dst += wsize); (gdb) bt #0 0xb6e8b6ea in memmove (dst0=0xb6578efc <mozilla::CountingAllocatorBase<NesteggReporter>::sAmount>, src0=<optimized out>, length=4294967292) at bionic/libc/upstream-openbsd/lib/libc/string/bcopy.c:97 #1 0xb56148c2 in nsTArray_Impl<nsRefPtr<mozilla::dom::bluetooth::BluetoothResultHandler>, nsTArrayInfallibleAllocator>::RemoveElementsAt (this=this@entry=0xb286ab20, aStart=aStart@entry=0, aCount=aCount@entry=1) at ../../dist/include/nsTArray.h:1398 #2 0xb56148f2 in RemoveElementAt (aIndex=0, this=0xb286ab20) at ../../dist/include/nsTArray.h:1403 #3 mozilla::dom::bluetooth::BluetoothDaemonInterface::OnDisconnect (this=0xb286ab00, aChannel=<optimized out>) at ../../../../mozilla-central-working/dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp:1909 #4 0xb4e8b668 in mozilla::ipc::SocketBase::NotifyDisconnect (this=this@entry=0xa8a29620) at ../../../../mozilla-central-working/ipc/unixsocket/SocketBase.cpp:221 #5 0xb4e8b0f2 in mozilla::ipc::ListenSocket::Close (this=0xa8a29620) at ../../../../mozilla-central-working/ipc/unixsocket/ListenSocket.cpp:380 #6 0xb56148d8 in mozilla::dom::bluetooth::BluetoothDaemonInterface::OnDisconnect (this=0xb286ab00, aChannel=<optimized out>) at ../../../../mozilla-central-working/dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp:1905 #7 0xb4e8b668 in mozilla::ipc::SocketBase::NotifyDisconnect (this=this@entry=0xa8036280) at ../../../../mozilla-central-working/ipc/unixsocket/SocketBase.cpp:221 #8 0xb4e8a386 in mozilla::ipc::BluetoothDaemonConnection::CloseSocket (this=0xa8036280) at ../../../../mozilla-central-working/ipc/bluetooth/BluetoothDaemonConnection.cpp:577 #9 0xb4e8a212 in mozilla::ipc::SocketIORequestClosingRunnable<mozilla::ipc::BluetoothDaemonConnectionIO>::Run (this=<optimized out>) at ../../dist/include/mozilla/ipc/SocketBase.h:438 #10 0xb4ca9646 in nsThread::ProcessNextEvent (this=0xb67025c0, aMayWait=<optimized out>, aResult=0xbed92647) at ../../../../mozilla-central-working/xpcom/threads/nsThread.cpp:855 #11 0xb4cb7028 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=aMayWait@entry=false) at /home/bruce_sun/Source/mozilla-central-working/xpcom/glue/nsThreadUtils.cpp:265 #12 0xb4daa020 in mozilla::ipc::MessagePump::Run (this=0xb6781190, aDelegate=0xb676d1a0) at ../../../../mozilla-central-working/ipc/glue/MessagePump.cpp:99 #13 0xb4da0370 in MessageLoop::RunInternal (this=this@entry=0xb676d1a0) at ../../../../mozilla-central-working/ipc/chromium/src/base/message_loop.cc:233 #14 0xb4da0424 in RunHandler (this=0xb676d1a0) at ../../../../mozilla-central-working/ipc/chromium/src/base/message_loop.cc:226 #15 MessageLoop::Run (this=0xb676d1a0) at ../../../../mozilla-central-working/ipc/chromium/src/base/message_loop.cc:200 #16 0xb5626636 in nsBaseAppShell::Run (this=0xb2a5b460) at ../../../mozilla-central-working/widget/nsBaseAppShell.cpp:164 #17 0xb58b70ea in nsAppStartup::Run (this=0xb2a61340) at ../../../../../mozilla-central-working/toolkit/components/startup/nsAppStartup.cpp:281 #18 0xb58cd070 in XREMain::XRE_mainRun (this=this@entry=0xbed927b8) at ../../../../mozilla-central-working/toolkit/xre/nsAppRunner.cpp:4160 #19 0xb58cd2a2 in XREMain::XRE_main (this=this@entry=0xbed927b8, argc=argc@entry=1, argv=argv@entry=0xb672e188, aAppData=aAppData@entry=0xb6f82838 <_ZL8sAppData>) at ../../../../mozilla-central-working/toolkit/xre/nsAppRunner.cpp:4236 #20 0xb58cd44a in XRE_main (argc=1, argv=0xb672e188, aAppData=0xb6f82838 <_ZL8sAppData>, aFlags=<optimized out>) at ../../../../mozilla-central-working/toolkit/xre/nsAppRunner.cpp:4456 #21 0xb6f67970 in do_main (argc=argc@entry=1, argv=argv@entry=0xb672e188) at ../../../../mozilla-central-working/b2g/app/nsBrowserApp.cpp:167 #22 0xb6f67abc in b2g_main (argc=1, argv=<optimized out>) at ../../../../mozilla-central-working/b2g/app/nsBrowserApp.cpp:299 #23 0xb6f67818 in RunProcesses (aReservedFds=..., argv=0xbed93a74, argc=1) at ../../../../mozilla-central-working/b2g/app/B2GLoader.cpp:225 #24 main (argc=1, argv=0xbed93a74) at ../../../../mozilla-central-working/b2g/app/B2GLoader.cpp:290
bluetoothd call stacks: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 2385.2399] mainloop () at bionic/libc/arch-arm/cortex-a15/bionic/strlen.S:100 100 ldrd r2, r3, [r1], #8 (gdb) bt #0 mainloop () at bionic/libc/arch-arm/cortex-a15/bionic/strlen.S:100 #1 0xb6faec44 in strlen (s=0x0) at bionic/libc/include/string.h:239 #2 dial_call_cmd_cb (number=0x0, bd_addr=0xb6b1601a <btif_hf_cb+2>) at system/bluetoothd/src/bt-hf-io.c:259 #3 0xb6a28922 in btif_hf_upstreams_evt (event=<optimized out>, p_param=<optimized out>) at external/bluetooth/bluedroid/main/../btif/src/btif_hf.c:614 #4 0xb6a19c8a in btif_context_switched (p_msg=0xb5b0bc68) at external/bluetooth/bluedroid/main/../btif/src/btif_core.c:181 #5 btif_task (params=<optimized out>) at external/bluetooth/bluedroid/main/../btif/src/btif_core.c:333 #6 0xb6a7254a in gki_task_entry (params=3066029332) at external/bluetooth/bluedroid/gki/./ulinux/gki_ulinux.c:263 #7 0xb6f382e4 in __pthread_start (arg=0xb6c35580, arg@entry=<error reading variable: value has been optimized out>) at bionic/libc/bionic/pthread_create.cpp:141 #8 0xb6f362d4 in __start_thread (fn=<optimized out>, arg=<optimized out>) at bionic/libc/bionic/clone.cpp:41 #9 0x00000000 in ?? ()
This basically is a crash issue of bluetoothd, but Gecko suffers from bug 1115656 so b2g crashes as well. This issue occurs on my nexus-5-l environment, but doesn't happens on my flame-kk environment.
(In reply to Bruce Sun [:brsun] from comment #3) > This issue occurs on my nexus-5-l environment, but doesn't happens on my > flame-kk environment. It turns out that I forget to cleanup |out/target/product/flame/system/build.prop| on my environment, so that |ro.moz.bluetooth.backend=bluedroid| exists in |system/build.prop| on my flame device.
(In reply to Bruce Sun [:brsun] from comment #4) > (In reply to Bruce Sun [:brsun] from comment #3) > > This issue occurs on my nexus-5-l environment, but doesn't happens on my > > flame-kk environment. > > It turns out that I forget to cleanup > |out/target/product/flame/system/build.prop| on my environment, so that > |ro.moz.bluetooth.backend=bluedroid| exists in |system/build.prop| on my > flame device. My flame crashes as well.
(In reply to Bruce Sun [:brsun] from comment #2) > bluetoothd call stacks: > > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread 2385.2399] > mainloop () at bionic/libc/arch-arm/cortex-a15/bionic/strlen.S:100 > 100 ldrd r2, r3, [r1], #8 > (gdb) bt > #0 mainloop () at bionic/libc/arch-arm/cortex-a15/bionic/strlen.S:100 > #1 0xb6faec44 in strlen (s=0x0) at bionic/libc/include/string.h:239 > #2 dial_call_cmd_cb (number=0x0, bd_addr=0xb6b1601a <btif_hf_cb+2>) at > system/bluetoothd/src/bt-hf-io.c:259 'number' is NULL. When does that happen? > #3 0xb6a28922 in btif_hf_upstreams_evt (event=<optimized out>, > p_param=<optimized out>) at > external/bluetooth/bluedroid/main/../btif/src/btif_hf.c:614 > #4 0xb6a19c8a in btif_context_switched (p_msg=0xb5b0bc68) at > external/bluetooth/bluedroid/main/../btif/src/btif_core.c:181 > #5 btif_task (params=<optimized out>) at > external/bluetooth/bluedroid/main/../btif/src/btif_core.c:333 > #6 0xb6a7254a in gki_task_entry (params=3066029332) at > external/bluetooth/bluedroid/gki/./ulinux/gki_ulinux.c:263 > #7 0xb6f382e4 in __pthread_start (arg=0xb6c35580, arg@entry=<error reading > variable: value has been optimized out>) at > bionic/libc/bionic/pthread_create.cpp:141 > #8 0xb6f362d4 in __start_thread (fn=<optimized out>, arg=<optimized out>) > at bionic/libc/bionic/clone.cpp:41 > #9 0x00000000 in ?? ()
Assignee: nobody → tzimmermann
Status: NEW → ASSIGNED
NULL means redial [1]: /** Callback for dialing an outgoing call * If number is NULL, redial */ typedef void (* bthf_dial_call_cmd_callback)(char *number, bt_bdaddr_t *bd_addr); [1] https://source.android.com/devices/halref/bt__hf_8h_source.html
This should fix the crash, but I'm not sure if Gecko interprets this as 'redial.' Bruce, can you give it a try?
Attachment #8554425 - Flags: feedback?(brsun)
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #8) > Created attachment 8554425 [details] [diff] [review] > Github pull request > > This should fix the crash, but I'm not sure if Gecko interprets this as > 'redial.' Bruce, can you give it a try? I've tried a similar patch locally, but redial function doesn't work. I am trying to digging into it.
OK, thanks! It's probably just that Gecko doesn't interpret the string correctly. I suggest to look into the Notification handlers in the Handsfree manager.
Do you want this bug?
Attachment 8554425 [details] [diff] can fix the crash issue in bluetoothd. This patch can fix the redial issue as stated on comment 9.
Attachment #8554450 - Flags: review?(tzimmermann)
Comment on attachment 8554425 [details] [diff] [review] Github pull request This patch can avoid the crash issue. :)
Attachment #8554425 - Flags: feedback?(brsun) → feedback+
Comment on attachment 8554425 [details] [diff] [review] Github pull request BTW, you can simply paste PR url as plain text attachment instead of html. Current redirection html code is not working
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #11) > Do you want this bug? Well...let's share this bug.
Comment on attachment 8554450 [details] [diff] [review] bug1125719_bluetoothd_redial_crash.patch Review of attachment 8554450 [details] [diff] [review]: ----------------------------------------------------------------- I don't understand this patch. Can you explain what is does? ::: dom/bluetooth/bluedroid/BluetoothDaemonHelpers.cpp @@ +1599,5 @@ > if (NS_WARN_IF(!str)) { > return NS_ERROR_ILLEGAL_VALUE; // end of PDU > } > > + const char* end = static_cast<char*>(memchr(str, '\0', aPDU.GetSize() + 1)); This reads beyond the end of the PDU, so we probably can't do that. @@ +1604,5 @@ > if (NS_WARN_IF(!end)) { > return NS_ERROR_ILLEGAL_VALUE; // no string terminator > } > > ptrdiff_t len = end - str; What about 'len = end - str + 1'? That would include the final '\0' in the length without reading beyond the end of the PDU.
Attachment #8554450 - Flags: feedback?(brsun)
Because there is |aPDU.Consume(1)| at the beginning of |UnpackPDU()|, |aPDU.GetSize()| becomes 1 less than the actual data length we want to parse. So |memchr()| will always fail by missing the final character by using |aPDU.GetSize()|. ex: "1234567890" contains in PDU while receiving callback: step 0: |UnpackPDU()| enters - |aPDU.GetSize()| is 11 (there is one character for the trailing |'\0'|). step 1: |aPDU.Consume(1)| - |str| points to '1'. - |aPDU.GetSize()| is 10. step 2: |memchr(str, '\0', aPDU.GetSize())| - |memchr()| never finds |'\0'| character because |'\0'| is not within the lookup range, 10, from |str|.
Flags: needinfo?(tzimmermann)
Comment on attachment 8554450 [details] [diff] [review] bug1125719_bluetoothd_redial_crash.patch Review of attachment 8554450 [details] [diff] [review]: ----------------------------------------------------------------- Oh that makes sense! Classical off-by-one. Thanks for fixing this bug.
Attachment #8554450 - Flags: review?(tzimmermann)
Attachment #8554450 - Flags: review+
Attachment #8554450 - Flags: feedback?(brsun)
Flags: needinfo?(tzimmermann)
TBPL results seem good.
blocking-b2g: --- → 2.2?
Keywords: checkin-needed
Comment on attachment 8554482 [details] [diff] [review] bug1125719_bluetoothd_redial_crash.checkin.patch NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 1091588, Bug 1087195 User impact if declined: Not able to perform redial through the Bluetooth device. Testing completed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=21bfb41d57cb Risk to taking this patch (and alternatives if risky): Low. This fixes crash issues by simply correcting misbehaved codes. String or UUID changes made by this patch: N/A
Attachment #8554482 - Flags: approval-mozilla-b2g37?
Comment on attachment 8554425 [details] [diff] [review] Github pull request Kindly help refer to comment 21 as the request comment. Thanks.
Attachment #8554425 - Flags: approval-mozilla-b2g37?
blocking-b2g: 2.2? → 2.2+
Comment on attachment 8554425 [details] [diff] [review] Github pull request Switching the approval to gaia.
Attachment #8554425 - Flags: approval-mozilla-b2g37? → approval-gaia-v2.2+
Attachment #8554482 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S5 (6feb)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: