Bug 191616 – WebKit.GeolocationTransitionToLowAccuracy API crashes when enabling PSON

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

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

Exception Type:        EXC_BAD_ACCESS (SIGBUS)
Exception Codes:       KERN_PROTECTION_FAILURE at 0x00007f85a2d7f090
Exception Note:        EXC_CORPSE_NOTIFY

Termination Signal:    Bus error: 10
Termination Reason:    Namespace SIGNAL, Code 0xa
Terminating Process:   exc handler [37316]

VM Regions Near 0x7f85a2d7f090:
    MALLOC_TINY            00007f85a2c00000-00007f85a2d00000 [ 1024K] rw-/rwx SM=PRV  
--> MALLOC_TINY            00007f85a2d00000-00007f85a2e00000 [ 1024K] rw-/rwx SM=ALI  
    MALLOC_TINY            00007f85a2e00000-00007f85a3000000 [ 2048K] rw-/rwx SM=PRV  

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   ???                           	0x00007f85a2d7f090 0 + 140211939438736
1   com.apple.WebKit              	0x00000001097aa7fe WebKit::WebGeolocationProvider::stopUpdating(WebKit::WebGeolocationManagerProxy&) + 78 (WebGeolocationProvider.cpp:53)
2   com.apple.WebKit              	0x00000001097a9c98 WebKit::WebGeolocationManagerProxy::removeRequester(IPC::Connection::Client const*) + 168 (WebGeolocationManagerProxy.cpp:136)
3   com.apple.WebKit              	0x00000001097a9bde WebKit::WebGeolocationManagerProxy::processDidClose(WebKit::WebProcessProxy*) + 78 (WebGeolocationManagerProxy.cpp:77)
4   com.apple.WebKit              	0x0000000109867057 WebKit::WebProcessPool::disconnectProcess(WebKit::WebProcessProxy*) + 583 (WebProcessPool.cpp:1041)
5   com.apple.WebKit              	0x0000000109920137 WebKit::WebProcessProxy::shutDown() + 1175 (WebProcessProxy.cpp:275)
6   com.apple.WebKit              	0x0000000109921eb7 WebKit::WebProcessProxy::maybeShutDown() + 71 (WebProcessProxy.cpp:864)
7   com.apple.WebKit              	0x0000000109921cf0 WebKit::WebProcessProxy::removeWebPage(WebKit::WebPageProxy&, unsigned long long) + 304 (WebProcessProxy.cpp:467)
8   com.apple.WebKit              	0x00000001097d3a70 WebKit::WebPageProxy::close() + 8128 (WebPageProxy.cpp:947)
9   com.apple.WebKit              	0x000000010971dc77 WebKit::WebViewImpl::~WebViewImpl() + 2727 (WebViewImpl.mm:1456)
10  com.apple.WebKit              	0x000000010971e375 WebKit::WebViewImpl::~WebViewImpl() + 21 (WebViewImpl.mm:1460)
11  com.apple.WebKit              	0x0000000109611ac8 -[WKView dealloc] + 744 (memory:2285)
12  libobjc.A.dylib               	0x00007fff7b554c8c (anonymous namespace)::AutoreleasePoolPage::pop(void*) + 726
13  com.apple.CoreFoundation      	0x00007fff4eccec06 _CFAutoreleasePoolPop + 22
14  com.apple.Foundation          	0x00007fff5113dc6e -[NSAutoreleasePool drain] + 144
15  TestWebKitAPI                 	0x00000001034c5e37 main + 487 (mainMac.mm:54)
16  libdyld.dylib                 	0x00007fff7c62e0a5 start + 1

[Ryosuke Niwa]

The issue here is that WKView is getting inserted into auto-release pool and its destructor is calling back stopUpdatingCallback after GeolocationTransitionToLowAccuracyStateTracker had been destroyed.

[Ryosuke Niwa]

Created attachment 354764 [details]
Fixes the test

[EWS Watchlist]

Attachment 354764 [details] did not pass style-queue:


ERROR: Tools/ChangeLog:10:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: use-after-free  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 354764 [details]
Fixes the test

View in context: https://bugs.webkit.org/attachment.cgi?id=354764&action=review

r=me with simplification.

> Tools/TestWebKitAPI/Tests/WebKit/Geolocation.cpp:126
> +    WKGeolocationManagerSetProvider(WKContextGetGeolocationManager(context), &providerCallback.base);

Presumably, we can simply call:
WKGeolocationManagerSetProvider(WKContextGetGeolocationManager(context), nullptr);

Looking at the implementation, I believe it would do the right thing.

[Ryosuke Niwa]

(In reply to Chris Dumez from comment #4)
> Comment on attachment 354764 [details]
> Fixes the test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354764&action=review
> 
> r=me with simplification.
> 
> > Tools/TestWebKitAPI/Tests/WebKit/Geolocation.cpp:126
> > +    WKGeolocationManagerSetProvider(WKContextGetGeolocationManager(context), &providerCallback.base);
> 
> Presumably, we can simply call:
> WKGeolocationManagerSetProvider(WKContextGetGeolocationManager(context),
> nullptr);
> 
> Looking at the implementation, I believe it would do the right thing.

That's a good point. Will fix before landing. Thanks for the review!

[Ryosuke Niwa]

Committed r238165: <https://trac.webkit.org/changeset/238165>

[Radar WebKit Bug Importer]

<rdar://problem/46053459>