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)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)
People
(Reporter: brsun, Assigned: tzimmermann)
Details
Attachments
(2 files, 1 obsolete file)
421 bytes,
patch
|
shawnjohnjr
:
review+
brsun
:
feedback+
bajaj
:
approval-gaia-v2.2+
|
Details | Diff | Splinter Review |
1.46 KB,
patch
|
brsun
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
The scenario can refer [1] for more details.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1124760#c14
Reporter | ||
Comment 1•10 years ago
|
||
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
Reporter | ||
Comment 2•10 years ago
|
||
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 ?? ()
Reporter | ||
Comment 3•10 years ago
|
||
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.
Reporter | ||
Comment 4•10 years ago
|
||
(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.
Reporter | ||
Comment 5•10 years ago
|
||
(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.
Assignee | ||
Comment 6•10 years ago
|
||
(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 | ||
Updated•10 years ago
|
Assignee: nobody → tzimmermann
Status: NEW → ASSIGNED
Reporter | ||
Comment 7•10 years ago
|
||
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
Assignee | ||
Comment 8•10 years ago
|
||
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)
Reporter | ||
Comment 9•10 years ago
|
||
(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.
Assignee | ||
Comment 10•10 years ago
|
||
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.
Assignee | ||
Comment 11•10 years ago
|
||
Do you want this bug?
Reporter | ||
Comment 12•10 years ago
|
||
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)
Attachment #8554425 -
Flags: review+
Reporter | ||
Comment 13•10 years ago
|
||
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
Reporter | ||
Comment 15•10 years ago
|
||
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #11)
> Do you want this bug?
Well...let's share this bug.
Assignee | ||
Comment 16•10 years ago
|
||
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)
Reporter | ||
Comment 17•10 years ago
|
||
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|.
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(tzimmermann)
Assignee | ||
Comment 18•10 years ago
|
||
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)
Reporter | ||
Comment 19•10 years ago
|
||
TBPL is under going:
- https://tbpl.mozilla.org/?tree=Try&rev=21bfb41d57cb
- https://treeherder.mozilla.org/#/jobs?repo=try&revision=21bfb41d57cb
Attachment #8554450 -
Attachment is obsolete: true
Attachment #8554482 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(tzimmermann)
Reporter | ||
Comment 20•10 years ago
|
||
TBPL results seem good.
blocking-b2g: --- → 2.2?
Keywords: checkin-needed
Reporter | ||
Comment 21•10 years ago
|
||
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?
Reporter | ||
Comment 22•10 years ago
|
||
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?
Comment 23•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/88f85a8099ca
Master: https://github.com/mozilla-b2g/platform_system_bluetoothd/commit/a3a900615b89cbd67acfb8b750baa43b321ef68b
Keywords: checkin-needed
Updated•10 years ago
|
blocking-b2g: 2.2? → 2.2+
Comment 24•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #8554482 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S5 (6feb)
Comment 26•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/d658922315f3
v2.2: https://github.com/mozilla-b2g/platform_system_bluetoothd/commit/7716272f3e0aa86cd2edbb1b15fea35318badfbb
status-b2g-v2.2:
--- → fixed
status-b2g-master:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•