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

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Marcia Knous [:marcia]]

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.

Comment 1

[Markus Stange [:mstange]]

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.

Comment 2

[Marcia Knous [:marcia]]

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.

Comment 3

[Julien Cristau [:jcristau]]

Too late for firefox 52, mass-wontfix.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

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?

Comment 7

[Markus Stange [: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.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Long standing issue, seems unlikely we will fix this for 53. We could still take a patch for 54.

Comment 10

[Stephen A Pohl [:spohl]]

This may be a duplicate of bug 1361432 and I wouldn't be surprised if this would get fixed by bug 1324892 as well.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 12

[Stephen A Pohl [:spohl]]

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.

Comment 13

[Marcia Knous [:marcia]]

(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?

Comment 14

[Marcia Knous [:marcia]]

(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.

Comment 15

[Marcia Knous [:marcia]]

So far from what I can see in crash stats, there are no 57 crashes for all 4 of these signatures.

Comment 16

[Stephen A Pohl [:spohl]]

Thank you. Closing as fixed by bug 1324892.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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.