crash in mozilla::dom::bluetooth::BluetoothHfpManager::ReceiveSocketData

RESOLVED FIXED in Firefox 21

Status

--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: m1, Assigned: echou)

Tracking

({crash})

unspecified
B2G C4 (2jan on)
ARM
Gonk (Firefox OS)
crash
Dependency tree / graph
Bug Flags:
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 fixed, b2g18-v1.0.1 fixed)

Details

(Whiteboard: [b2g-crash][BTG-1070] QARegressExclude, crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

Crash seen during stability test on AU 185.

Test Steps:
1. Run of Reboot script continuously with a connection of WIFI and BT Car kit.


Top frames
----------
Crash reason:  SIGSEGV
Crash address: 0x4a100000

Thread 0 (crashed)
 0  libc.so!strlen [strlen.c : 62 + 0x5c]
     r4 = 0x4a0ffff0    r5 = 0xffffffff    r6 = 0xbe945628    r7 = 0x0000000a
     r8 = 0xbe94575f    r9 = 0x40307c0c   r10 = 0x00000000    fp = 0x00000000
     sp = 0xbe945448    lr = 0x4118ef6f    pc = 0x40078b4c
    Found by: given as instruction pointer in context
 1  libxul.so!nsACString_internal::Assign [nsCharTraits.h : 590 + 0x5]
     r4 = 0x4a0ffff0    r5 = 0xffffffff    r6 = 0xbe945628    r7 = 0x0000000a
     r8 = 0xbe94575f    r9 = 0x40307c0c   r10 = 0x00000000    fp = 0x00000000
     sp = 0xbe945450    pc = 0x4118ef6f
    Found by: call frame info
 2  libxul.so!nsACString_internal::Assign [nsTSubstring.cpp : 290 + 0x5]
     r4 = 0x488fa0b0    r5 = 0x00000000    r6 = 0xbe945628    r7 = 0x0000000a
     r8 = 0xbe94575f    r9 = 0x40307c0c   r10 = 0x00000000    fp = 0x00000000
     sp = 0xbe945478    pc = 0x4118efe5
    Found by: call frame info
 3  libxul.so!mozilla::dom::bluetooth::BluetoothHfpManager::ReceiveSocketData [nsTString.h : 463 + 0x3]
     r4 = 0x488fa0b0    r5 = 0x00000000    r6 = 0xbe945628    r7 = 0x0000000a
     r8 = 0xbe94575f    r9 = 0x40307c0c   r10 = 0x00000000    fp = 0x00000000
     sp = 0xbe945490    pc = 0x40defe37
    Found by: call frame info

Updated

6 years ago
Severity: normal → critical
Crash Signature: [@ strlen | nsACString_internal::Assign ]
Keywords: crash
Whiteboard: [BTG-1070] → [b2g-crash][BTG-1070]
(Reporter)

Comment 1

6 years ago
Created attachment 707220 [details]
decoded minidump of crash
(Reporter)

Updated

6 years ago
Assignee: nobody → echou
(Assignee)

Comment 2

6 years ago
According to the minidump, this may be caused by the following line in BluetoothHfpManager::ReceiveSocketData():

  nsAutoCString msg((const char*)aMessage->mData.get());

This would be dangerous if aMessage->mData is not terminated by '\0'. If that happens, then it may be possible getting into crash when trying to get the string length in ctor of nsAutoCString.

I don't know exactly when would this case happen, but it seems to be possible (and looks like it /did/ happen). I'll provide a patch asap.
(Assignee)

Comment 3

6 years ago
Created attachment 707497 [details] [diff] [review]
patch 1: v1: assign length of the message with aMessage->mSize

Hi Greg,

Please apply this patch to b2g18 branch and see if this would still happen. Thank you.

Eric
Flags: needinfo?(mvines)
blocking-b2g: tef? → tef+
(Assignee)

Comment 4

6 years ago
(In reply to Eric Chou [:ericchou] [:echou] from comment #3)
> Created attachment 707497 [details] [diff] [review]
> patch 1: v1: assign length of the message with aMessage->mSize
> 
> Hi Greg,
> 

Sorry, I thought this was filed by Greg.

Michael, could you help with this?
(Reporter)

Comment 5

6 years ago
Patch makes sense to me, although are you confident that stripping the CR won't cause a regression? 

Suggest we just land this so it'll get picked up in the next stability run in a couple days or so.   If it's seen again we can reopen the bug.
Flags: needinfo?(mvines)
Blocks: 829396
(Assignee)

Comment 6

6 years ago
(In reply to Michael Vines [:m1] [:evilmachines] from comment #5)
> Patch makes sense to me, although are you confident that stripping the CR
> won't cause a regression? 
>

Yes, it should be fine.

> Suggest we just land this so it'll get picked up in the next stability run
> in a couple days or so.   If it's seen again we can reopen the bug.

Sure. Thanks.
(Assignee)

Comment 7

6 years ago
Created attachment 707970 [details] [diff] [review]
patch 1: v2: assign length of the message with aMessage->mSize

* We don't need to strip '\r' by ourselves because msg.StripWhitespaces() has done that. The only thing we need to do is making sure the string length is right.
Attachment #707497 - Attachment is obsolete: true
Attachment #707970 - Flags: review?(gyeh)
Comment on attachment 707970 [details] [diff] [review]
patch 1: v2: assign length of the message with aMessage->mSize

Review of attachment 707970 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for spending time on this, r=me :)
Attachment #707970 - Flags: review?(gyeh) → review+
(Assignee)

Comment 10

6 years ago
Created attachment 708006 [details] [diff] [review]
patch for b2g18

Due to this is a tef+ bug, the patch should be checked into b2g18. However this cannot be directly applied or there will be a conflict. Therefore, please use this patch for checking into b2g18. The logic is totally the same as the reviewed patch.
https://hg.mozilla.org/mozilla-central/rev/f8cdfa25ad9f
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ed7302e4801c
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/361d9359f4f3
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → fixed
status-firefox19: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → fixed
Target Milestone: --- → B2G C4 (2jan on)
status-b2g18-v1.0.1: --- → fixed

Comment 13

6 years ago
does not make sense to create a regression issue.
Flags: in-moztrap-

Updated

6 years ago
Whiteboard: [b2g-crash][BTG-1070] → [b2g-crash][BTG-1070] QARegressExclude

Comment 14

6 years ago
Cannot verify, need steps to blackbox test this issue.
You need to log in before you can comment on or make changes to this bug.