Bug 129864 – Crash shortly after loading certain web pages

• Component: New Bugs
• Status: RESOLVED FIXED

[Aaron Golden]

On "TOT" (I'm on commit c165e52), if I run

Tools/Scripts/build-webkit --debug

then

Tools/Scripts/debug-safari --target-web-process

and start loading web pages, pretty quickly I get the following crash.  I've reproduced the crash loading google.com, stackoverflow.com, penny-arcade.com, and qwantz.com, even bugs.webkit.org.  Oddly, I did not get the crash when I loaded www.webkit.org.

The "didReceiveMessageFromInjectedBundle" call in the stack made me think it might be some extension issue, but I've uninstalled my Safari extensions and the crash persists.  Previously I had installed AdBlock.

Process:         SafariForWebKitDevelopment [42652]
Path:            /Applications/Safari.app/Contents/MacOS/SafariForWebKitDevelopment
Identifier:      com.apple.Safari
Version:         6.1 (8537.71)
Build Info:      WebBrowser-7537071000000000~2
Code Type:       X86-64 (Native)
Parent Process:  WebProcess [42649]
User ID:         501

Date/Time:       2014-03-06 22:03:07.362 -0800
OS Version:      Mac OS X 10.8.5 (12F45)
Report Version:  10

Interval Since Last Report:          280533 sec
Crashes Since Last Report:           51
Per-App Interval Since Last Report:  270765 sec
Per-App Crashes Since Last Report:   23
Anonymous UUID:                      23B030E1-A6C7-1FCB-07F6-BAEC7758EAE4

Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000000

Application Specific Information:
Process Model:
Single Web Process
 
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   ???                           	000000000000000000 0 + 0
1   SecItemShim.dylib             	0x0000000100005dcc WebKit::shimSecItemCopyMatching(__CFDictionary const*, void const**) + 44
2   com.apple.Safari.framework    	0x000000010034217b getPasswordAndItem + 227
3   com.apple.Safari.framework    	0x0000000100342265 WBSGetKeychainPassword + 20
4   com.apple.Safari.framework    	0x000000010019de47 encryptionKey(signed char) + 85
5   com.apple.Safari.framework    	0x000000010019dde4 -[FormDataController verifyKeychainAccessWithContentViewController:] + 80
6   com.apple.Safari.framework    	0x000000010019bf5b -[FormDataController preFillActiveOrFirstLoginFormInViewController:metadataProvider:] + 347
7   com.apple.Safari.framework    	0x000000010007fb51 Safari::BrowserContentViewController::didCollectFormMetadataForPreFillingForm(Safari::WK::Array const&, Safari::WK::Array const&) + 165
8   com.apple.Safari.framework    	0x0000000100094b54 Safari::BrowserContentViewController::handleMessage(Safari::WK::String const&, Safari::WK::Type const&) + 872
9   com.apple.Safari.framework    	0x000000010009c114 Safari::BrowserContextInjectedBundleClient::dispatchMessage(Safari::WK::String const&, Safari::WK::Type const&) + 48
10  com.apple.Safari.framework    	0x0000000100115bc2 Safari::WK::didReceiveMessageFromInjectedBundle(OpaqueWKContext const*, OpaqueWKString const*, void const*, void const*) + 90
11  com.apple.WebKit2             	0x0000000102f7075b WebKit::WebContextInjectedBundleClient::didReceiveMessageFromInjectedBundle(WebKit::WebContext*, WTF::String const&, API::Object*) + 155 (WebContextInjectedBundleClient.cpp:41)
12  com.apple.WebKit2             	0x0000000102f4e39d WebKit::WebContext::didReceiveMessageFromInjectedBundle(WTF::String const&, API::Object*) + 61 (WebContext.cpp:840)
13  com.apple.WebKit2             	0x0000000102f4ed40 WebKit::WebContext::didReceiveMessage(IPC::Connection*, IPC::MessageDecoder&) + 480 (WebContext.cpp:1043)
14  com.apple.WebKit2             	0x0000000102f4edd7 non-virtual thunk to WebKit::WebContext::didReceiveMessage(IPC::Connection*, IPC::MessageDecoder&) + 55 (WebContext.cpp:1048)
15  com.apple.WebKit2             	0x0000000102ced4a8 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection*, IPC::MessageDecoder&) + 216 (MessageReceiverMap.cpp:83)
16  com.apple.WebKit2             	0x0000000102f4eaf7 WebKit::WebContext::dispatchMessage(IPC::Connection*, IPC::MessageDecoder&) + 55 (WebContext.cpp:1018)
17  com.apple.WebKit2             	0x00000001031eddde WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection*, IPC::MessageDecoder&) + 110 (WebProcessProxy.cpp:355)
18  com.apple.WebKit2             	0x00000001031edec7 non-virtual thunk to WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection*, IPC::MessageDecoder&) + 55 (WebProcessProxy.cpp:364)
19  com.apple.WebKit2             	0x0000000102b35333 IPC::Connection::dispatchMessage(IPC::MessageDecoder&) + 51 (Connection.cpp:777)
20  com.apple.WebKit2             	0x0000000102b2d470 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 368 (Connection.cpp:797)
21  com.apple.WebKit2             	0x0000000102b350c1 IPC::Connection::dispatchOneMessage() + 1377 (Connection.cpp:823)
22  com.apple.WebKit2             	0x0000000102b432c2 WTF::FunctionWrapper<void (IPC::Connection::*)()>::operator()(IPC::Connection*) + 114 (Functional.h:218)
23  com.apple.WebKit2             	0x0000000102b43245 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (IPC::Connection::*)()>, void (IPC::Connection*)>::operator()() + 53 (Functional.h:496)
24  com.apple.WebKit2             	0x0000000102b4afd2 WTF::Function<void ()>::operator()() const + 114 (Functional.h:704)
25  com.apple.WebKit2             	0x0000000102b4af4c std::__1::__function::__func<WTF::Function<void ()>, std::__1::allocator<WTF::Function<void ()> >, void ()()>::operator()() + 60 (functional:1059)
26  com.apple.JavaScriptCore      	0x000000010110d26a std::__1::function<void ()>::operator()() const + 26 (functional:1435)
27  com.apple.JavaScriptCore      	0x000000010111c834 WTF::RunLoop::performWork() + 276 (RunLoop.cpp:106)
28  com.apple.JavaScriptCore      	0x000000010111dcd4 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38)
29  com.apple.CoreFoundation      	0x00007fff97b3bb31 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
30  com.apple.CoreFoundation      	0x00007fff97b3b455 __CFRunLoopDoSources0 + 245
31  com.apple.CoreFoundation      	0x00007fff97b5e7f5 __CFRunLoopRun + 789
32  com.apple.CoreFoundation      	0x00007fff97b5e0e2 CFRunLoopRunSpecific + 290
33  com.apple.HIToolbox           	0x00007fff930bbeb4 RunCurrentEventLoopInMode + 209
34  com.apple.HIToolbox           	0x00007fff930bbc52 ReceiveNextEventCommon + 356
35  com.apple.HIToolbox           	0x00007fff930bbae3 BlockUntilNextEventMatchingListInMode + 62
36  com.apple.AppKit              	0x00007fff934e2533 _DPSNextEvent + 685
37  com.apple.AppKit              	0x00007fff934e1df2 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128
38  com.apple.Safari.framework    	0x00000001000664cf -[BrowserApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 161
39  com.apple.AppKit              	0x00007fff934d91a3 -[NSApplication run] + 517
40  com.apple.AppKit              	0x00007fff9347dbd6 NSApplicationMain + 869
41  com.apple.Safari.framework    	0x00000001002294fc SafariMain + 266
42  libdyld.dylib                 	0x00007fff929337e1 start + 1

[Aaron Golden]

Created attachment 226086 [details]
Patch prevents shimSecItem{CopyMatching,Add,Update,Delete} from trying to use NULL implementations

Patch prevents the crash by guarding various methods against calling implementations from the secItemShimCallbacks struct when those implementations are NULL.  I'm actually not sure what the most appropriate OSStatus return value is when the callback is NULL.  errSecUnimplemented seemed pretty good, but maybe there's a better/more standard one.

[Aaron Golden]

Created attachment 226088 [details]
Same as the other patch, but remembered to include a new ChangeLog entry.

[Anders Carlsson]

I don't think this is correct. A null shim function indicates a serious problem (like the initialization function never being called).

[Aaron Golden]

Is it weird that shimSecItemCopyMatching is being called from the SafariForWebKitDevelopment process itself?  I see WebKitSecItemShimInitialize called from within the child WebProcess, but not from within the SafariForWebKitDevelopment process.

[Aaron Golden]

The bug disappeared when I rolled back my local patch.  I wonder if when I got past the crash the first time (with the r- patch) some state was saved that prevents the troublesome code path from executing on subsequent launches.

[Aaron Golden]

I was able to reproduce the bug again by running Tools/Scripts/debug-safari --target-web-process instead of Tools/Scripts/run-safari --debug.  The WebProcess is hitting the shim initializer (WebKitSecItemShimInitialize) with non-NULL callbacks, but the SafariForWebkitDevelopment process *itself* is calling shimSecItemCopyMatching, and the shim struct used by the SafariForWebkitDevelopment process is empty (all callbacks NULL).

[Aaron Golden]

Created attachment 226189 [details]
Prevents processes spawned by WebProcess from trying to insert SecItem shim.

The trouble here is that SafariForWebkitDevelopment (spawned by WebProcess) is *itself* trying to call functions in SecItemShimLibrary.mm, but the secItemShimCallbacks struct is initialized only in the WebProcess.  The spawned processes shouldn't be in this code at all.

[WebKit Commit Bot]

Comment on attachment 226189 [details]
Prevents processes spawned by WebProcess from trying to insert SecItem shim.

Clearing flags on attachment: 226189

Committed r165329: <http://trac.webkit.org/changeset/165329>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.