1544878 - Crash in [@ _object_remove_assocations]

Status: RESOLVED WORKSFORME

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

Description

[Marcia Knous [:marcia]]

This bug is for crash report bp-6585481a-7781-46bf-b5ad-fe3b00190416.

Not sure where this belongs, but filing this low volume crash since all the signatures appear to be potential UAFs.

One comment "It is related to last pass add on." A quick spot check of the reports shows many have 4.26.0.4 version installed.

Top 10 frames of crashing thread:

0 libobjc.A.dylib _object_remove_assocations 
1 SkyLight CGSWindowInvalidateCache 
2 SkyLight CGSWindowGetShapeBounds 
3 SkyLight GetWindowBounds 
4 OpenGL cglBadApplicationNotMuxAwareLockDown 
5 OpenGL CGLUpdateContext 
6 OpenGL CGLUpdateContext 
7 AppKit -[NSOpenGLContext update] 
8 XUL -[ChildView preRender:] widget/cocoa/nsChildView.mm:3078
9 XUL nsChildView::PreRender widget/cocoa/nsChildView.mm:1833

Comment 1

[Marcia Knous [:marcia]]

It this is indeed all lastpass I guess we can move it to external software. I'll let someone in dev look at the crash stack first.

Comment 2

[Stephen A Pohl [:spohl]]

Markus, this looks like it's hitting code that was added in bug 1261483. Do you have any thoughts?

Comment 3

[Markus Stange [:mstange]]

I think the problem is that this call is happening on the compositor thread at the same time as SLSSetSurfaceProperty runs on the main thread. Here's the main thread stack:

0 	libsystem_platform.dylib 	_platform_memmove$VARIANT$Haswell 	
1 	SkyLight 	CGSPropertyListCreateSerializedBytes 	
2 	SkyLight 	CGSPropertyListPerformWithSerializedBytes 	
3 	SkyLight 	SLSSetSurfaceProperty 	
4 	AppKit 	-[NSSurface syncSurfaceWantsExtendedDynamicRange] 	
5 	AppKit 	__38-[NSSurface syncToViewUnconditionally]_block_invoke 	
6 	AppKit 	NSPerformVisuallyAtomicChange 	
7 	AppKit 	-[NSSurface syncToViewUnconditionally] 	
8 	AppKit 	-[NSSurface syncToView:] 	
9 	AppKit 	-[NSView(NSInternal) geometryInWindowDidChange] 	
10 	AppKit 	NSViewHierarchyNoteGeometryInWindowDidChange 	
11 	AppKit 	NSViewHierarchyNoteGeometryInWindowDidChange 	
12 	AppKit 	NSViewHierarchyNoteGeometryInWindowDidChange 	
13 	AppKit 	-[NSView _invalidateFocus] 	
14 	AppKit 	-[NSView setFrameSize:] 	
15 	AppKit 	-[NSWindow _oldPlaceWindow:] 	
16 	AppKit 	-[NSWindow _setFrameCommon:display:stashSize:] 	
17 	AppKit 	-[NSWindow _setFrame:display:allowImplicitAnimation:stashSize:] 	
18 	AppKit 	-[NSWindow setFrame:display:] 	
19 	XUL 	nsCocoaWindow::DoResize(double, double, double, double, bool, bool)

Once we use CoreAnimation, we will no longer have a call to the update method.
Not sure what to do in the meantime. We could try moving the update call back to the main thread.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

(In reply to Markus Stange [:mstange] from comment #3)

Not sure what to do in the meantime. We could try moving the update call back to the main thread.

Is that something we can consider pursuing? This crash is still lingering around :(

Comment 5

[Stephen A Pohl [:spohl]]

With all the work happening around CoreAnimation, I'm going to assign this bug to Markus to see how we should proceed here.

Comment 6

[Markus Stange [:mstange]]

In the CoreAnimation path, CGLUpdateContext is no longer called. And indeed, there are only two crashes with _object_remove_assocations in the top frame on the 70 channel, and both look completely unrelated to compositing / OpenGL contexts:
https://crash-stats.mozilla.org/report/index/62cb5c80-ac3c-4fb8-bc29-389a60190925
https://crash-stats.mozilla.org/report/index/7698ac29-f7ee-43c6-b846-3be990190927

The former is in native menu code, and the latter is in the native event loop during CFTimer handling:

NSInvalidArgumentException: -[__NSCFTimer scrollView:scrollWheelWithEvent:]: unrecognized selector sent to instance 0x19acd51c0

Let's wait and see what the crash numbers look like once 70 is released.

Comment 7

[Selena Deckelmann :selenamarie :selena]

Flagging this for review.

Comment 8

[Jim Mathies [:jimm]]

Hey Markus, still showing up in 70/71 nothing in 72 yet. Can you giver us a status on this? It's still sec-high and actively being tracked.

Comment 9

[Markus Stange [:mstange]]

I've looked through all the crash reports for 70 and 71 and none of them contained the frame CGLUpdateContext. So I think the crash from comment 0 is no longer happening.

The other crashes all happen in system code and are not very actionable. Moreover, _object_remove_assocations seems to be a very generic frame that is associated with use-after-frees in Objective C code in general. I think it would be a good idea to add it to the signature skip list. Then we can file individual bugs for the remaining occurrences, but we still won't really know what to do about them.

Comment 10

[Jim Mathies [:jimm]]

closing this out based on last comments about this signature no longer showing up.