Closed Bug 1339485 Opened 3 years ago Closed 2 years ago

[10.12] Crash in libobjc.A.dylib@0x6b5d

Categories

(Core :: Widget: Cocoa, defect, critical)

52 Branch
Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- fixed

People

(Reporter: marcia, Assigned: spohl)

References

Details

(4 keywords, Whiteboard: [adv-main57+][post-critsmash-triage])

Crash Data

This bug was filed from the Socorro interface and is 
report bp-faed78c3-dbb5-40ad-b4c0-7f03d2170209.
=============================================================

Although this signature is associated with the Mac touchbar crash, there are also an increasing set of comments that users are crashing when uploading files.

See http://bit.ly/2kPs1w0 for those crashes. All of the users are on 10.12, and some are on the latest 10.12.3 version.

We should try to get a set of STR so we can investigate this crash further, as well as figure out if this is an issue that needs a bug on Apple's side.
The symbolicated stack trace looks something like this:

#0 objc_msgSend (in libobjc.A.dylib)
#1 -[NSTextInputContext handleTSMEvent:completionHandler:] (in AppKit)
#2 -[NSTableRowData moveRowAtIndex:toIndex:] (in AppKit)
#3 -[NSKeyBindingManager(NSKeyBindingManager_MultiClients) interpretEventAsCommand:forClient:] (in AppKit)
#4 FinderKit@0xd211
#5 -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] (in AppKit)
#6 -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] (in AppKit)
#7 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (in AppKit)
#8 -[GeckoNSApplication sendEvent:]
#9 -[NSToolbarItemViewer hitTest:] (in AppKit)
#10 -[NSApplication runModalSession:] (in AppKit)
#11 -[NSWindow(NSSheets) _setSheet:] (in AppKit)
#12 -[NSSavePanel _overwriteExistingFileCheck:] (in AppKit)
#13 nsFilePicker::GetLocalFiles
#14 nsFilePicker::Show
#15 AsyncShowFilePicker::Run

(I symbolicated this using a slightly different version of AppKit (from 16D25a instead of 16D32) so the symbols may be slightly off, but the stack looks mostly reasonable.)

What seems to happen is that the user is inside the file save modal dialog, they have selected a file that already exists, tried to save over it, the file dialog shows a modal dialog asking whether to overwrite the file, and then the user pressed a key.
This is the top Mac crash on release channel, and almost all of the comments mention uploading pictures or doing a file operation while crashing which corroborates what Markus sees in Comment 1. Should we try to reproduce this issue - the crash does exist on beta as well.
Too late for firefox 52, mass-wontfix.
Sec-uaf bug, same as bug 1359901 (newer variant signature).

Goes a long way back it appears.

Likely we're not holding a strong ref for the lifetime of an OS call, and we get deleted/gc'd out from under the os
Group: core-security
Duplicate of this bug: 1359901
Crash Signature: [@ libobjc.A.dylib@0x6b5d] → [@ libobjc.A.dylib@0x6b5d] [@ libobjc.A.dylib@0x705d] [@ libobjc.A.dylib@0x701d]
Component: General → Widget: Cocoa
Group: core-security → layout-core-security
Keywords: testcase-wanted
Looks like this signature is the same or similar:

[@ objc_msgSend | -[NSKeyBindingManager interpretEventAsCommand:forClient:] ]
570 crashes in the last week

Marcus, Stephen, any ideas on how we can address this long-standing sec/stability issue?
Crash Signature: [@ libobjc.A.dylib@0x6b5d] [@ libobjc.A.dylib@0x705d] [@ libobjc.A.dylib@0x701d] → [@ libobjc.A.dylib@0x6b5d] [@ libobjc.A.dylib@0x705d] [@ libobjc.A.dylib@0x701d] [@ objc_msgSend | -[NSKeyBindingManager interpretEventAsCommand:forClient:] ]
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(mstange)
I don't have any good ideas for what to do with this as long as we can't reproduce this reliably. Auditing our code might work; reverse-engineering how Apple's code uses NSKeyBindingManagers and NSTextInputContexts might also help but would be a very involved task.
Flags: needinfo?(mstange)
Flags: needinfo?(spohl.mozilla.bugs)
The data the OS is accessing in the filepicker should be (quite?) limited, so auditing the lifetimes of those (across the filepicker call) should be the most productive approach.  Probably holding a ref to <something> higher up the callstack is all that's needed.
Long standing issue, seems unlikely we will fix this for 53. We could still take a patch for 54.
This may be a duplicate of bug 1361432 and I wouldn't be surprised if this would get fixed by bug 1324892 as well.
Depends on: 1324892
Several sec issues end up pointing at bug 1324892. I'm not sure if that will happen for the 55 release or if it's more reasonable to aim at 56. I emailed :aobreja and cc-ed him on the relevant bugs.
Bug 1324892 has now landed. It would be great to keep an eye on crash stats to see if 57 starts appearing. So far, I don't see any reports for 57 and if bug 1324892 fixed this, we should continue to not see any.
Flags: needinfo?(mozillamarcia.knous)
(In reply to Stephen A Pohl [:spohl] from comment #12)
> Bug 1324892 has now landed. It would be great to keep an eye on crash stats
> to see if 57 starts appearing. So far, I don't see any reports for 57 and if
> bug 1324892 fixed this, we should continue to not see any.

I assume we will have to revisit this since the bug in Comment 12 was backed out?
Flags: needinfo?(mozillamarcia.knous)
(In reply to Marcia Knous [:marcia - use ni] from comment #13)
> (In reply to Stephen A Pohl [:spohl] from comment #12)
> > Bug 1324892 has now landed. It would be great to keep an eye on crash stats
> > to see if 57 starts appearing. So far, I don't see any reports for 57 and if
> > bug 1324892 fixed this, we should continue to not see any.
> 
> I assume we will have to revisit this since the bug in Comment 12 was backed
> out?

Please ignore, as I guess that comment was posted to the wrong bug. I will leave the ni on myself to check crash-stats.
Flags: needinfo?(mozillamarcia.knous)
So far from what I can see in crash stats, there are no 57 crashes for all 4 of these signatures.
Flags: needinfo?(mozillamarcia.knous)
Thank you. Closing as fixed by bug 1324892.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Group: layout-core-security → core-security-release
Assignee: nobody → spohl.mozilla.bugs
Target Milestone: --- → mozilla57
Per discussion with IRC, the OSX SDK update isn't something we want to attempt to backport to an ESR branch midway through a cycle.
Whiteboard: [adv-main57+]
Flags: qe-verify-
Whiteboard: [adv-main57+] → [adv-main57+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.