Bug 219786 - Potential null dereference of m_frame under DocumentLoader::stopLoading()
Summary: Potential null dereference of m_frame under DocumentLoader::stopLoading()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-11 09:47 PST by Chris Dumez
Modified: 2020-12-11 16:32 PST (History)
7 users (show)

See Also:


Attachments
Patch (10.68 KB, patch)
2020-12-11 10:11 PST, Chris Dumez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (10.69 KB, patch)
2020-12-11 10:35 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (10.61 KB, patch)
2020-12-11 11:17 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2020-12-11 09:47:31 PST
Potential null dereference of m_frame under DocumentLoader::stopLoading():

Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000008)
[  0] 0x0000000192459570 WebCore`WebCore::DocumentLoader::stopLoading() [inlined] WebCore::Frame::WeakValueType* WTF::WeakPtrImpl<WTF::EmptyCounter>::get<WebCore::Frame>() at WeakPtr.h:57:56

     0x0000000192459560:      cbz w8, 0x167861c        ; <+392> at DocumentLoader.cpp
     0x0000000192459564:      ldr x8, [x19, #0x8]
     0x0000000192459568:      mov w23, #0x0
     0x000000019245956c:      mov w20, #0x0
 ->  0x0000000192459570:      ldr x8, [x8, #0x8]
     0x0000000192459574:      ldr x9, [x8, #0xc0]
     0x0000000192459578:     ldrb w9, [x9, #0x95d]
     0x000000019245957c:      cbz w9, 0x1678590        ; <+252> [inlined] WTF::HashTable<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> >, WTF::KeyValuePair<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> >, unsigned long long>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> >, unsigned long long> >, WTF::DefaultHash<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> > >, WTF::HashMap<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> >, unsigned long long, WTF::DefaultHash<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> > >, WTF::HashTraits<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> > >, WTF::HashTraits<unsigned long long> >::KeyValuePairTraits, WTF::HashTraits<std::__1::unique_ptr<WebCore::IconLoader, std::__1::default_delete<WebCore::IconLoader> > > >::keyCount() const at HashTable.h:451
     0x0000000192459580:      ldr x0, [x8, #0xa0]

[  0] 0x0000000192459570 WebCore`WebCore::DocumentLoader::stopLoading() [inlined] WTF::WeakPtr<WebCore::Frame, WTF::EmptyCounter>::get() const + 12 at WeakPtr.h:95
[  0] 0x0000000192459564 WebCore`WebCore::DocumentLoader::stopLoading() [inlined] WTF::WeakPtr<WebCore::Frame, WTF::EmptyCounter>::operator->() const at WeakPtr.h:108
[  0] 0x0000000192459564 WebCore`WebCore::DocumentLoader::stopLoading() + 208 at DocumentLoader.cpp:297
       293 	
       294 	    if (m_committed) {
       295 	        // Attempt to stop the frame if the document loader is loading, or if it is done loading but
       296 	        // still  parsing. Failure to do so can cause a world leak.
    -> 297 	        Document* doc = m_frame->document();
       298 	        
       299 	        if (loading || doc->parsing())
       300 	            m_frame->loader().stopLoading(UnloadEventPolicyNone);
       301 	    }
    
[  1] 0x000000019248a8bb WebCore`WebCore::FrameLoader::stopAllLoaders(WebCore::ClearProvisionalItemPolicy, WebCore::StopLoadingPolicy) + 371 at FrameLoader.cpp:1824:27
       1820	        child->loader().stopAllLoaders(clearProvisionalItemPolicy);
       1821	    if (m_provisionalDocumentLoader)
       1822	        m_provisionalDocumentLoader->stopLoading();
       1823	    if (m_documentLoader)
    -> 1824	        m_documentLoader->stopLoading();
       1825	    if (m_frame.page() && !m_frame.page()->chrome().client().isSVGImageChromeClient())
       1826	        platformStrategies()->loaderStrategy()->browsingContextRemoved(frame());
       1827	
       1828	    FRAMELOADER_RELEASE_LOG_IF_ALLOWED(ResourceLoading, "stopAllLoaders: Clearing provisional document loader (m_provisionalDocumentLoader=%p)", m_provisionalDocumentLoader.get());
    
[  2] 0x000000019248a8bb WebCore`WebCore::FrameLoader::stopAllLoaders(WebCore::ClearProvisionalItemPolicy, WebCore::StopLoadingPolicy) + 371 at FrameLoader.cpp:1824:27
       1820	        child->loader().stopAllLoaders(clearProvisionalItemPolicy);
       1821	    if (m_provisionalDocumentLoader)
       1822	        m_provisionalDocumentLoader->stopLoading();
       1823	    if (m_documentLoader)
    -> 1824	        m_documentLoader->stopLoading();
       1825	    if (m_frame.page() && !m_frame.page()->chrome().client().isSVGImageChromeClient())
       1826	        platformStrategies()->loaderStrategy()->browsingContextRemoved(frame());
       1827	
       1828	    FRAMELOADER_RELEASE_LOG_IF_ALLOWED(ResourceLoading, "stopAllLoaders: Clearing provisional document loader (m_provisionalDocumentLoader=%p)", m_provisionalDocumentLoader.get());
    
[  3] 0x000000019248dfbf WebCore`WebCore::FrameLoader::detachFromParent() + 103 at FrameLoader.cpp:2827:9
       2823	    if (m_frame.document()->backForwardCacheState() != Document::InBackForwardCache) {
       2824	        // stopAllLoaders() needs to be called after detachChildren() if the document is not in the back/forward cache,
       2825	        // because detachedChildren() will trigger the unload event handlers of any child frames, and those event
       2826	        // handlers might start a new subresource load in this frame.
    -> 2827	        stopAllLoaders(ShouldClearProvisionalItem, StopLoadingPolicy::AlwaysStopLoading);
       2828	    }
       2829	
       2830	    InspectorInstrumentation::frameDetachedFromParent(m_frame);
       2831	
    
[  4] 0x0000000190c5738f WebKit`WebKit::WebPage::close() + 1003 at WebPage.cpp:1496:40
       1492	    m_fullScreenClient.initialize(0);
       1493	#endif
       1494	
       1495	    m_printContext = nullptr;
    -> 1496	    m_mainFrame->coreFrame()->loader().detachFromParent();
       1497	    m_drawingArea = nullptr;
       1498	
       1499	    DeferredPageDestructor::createDeferredPageDestructor(WTFMove(m_page), this);
       1500
Comment 1 Chris Dumez 2020-12-11 09:47:44 PST
<rdar://71945402>
Comment 2 Chris Dumez 2020-12-11 10:11:23 PST
Created attachment 416005 [details]
Patch
Comment 3 Chris Dumez 2020-12-11 10:35:57 PST
Created attachment 416007 [details]
Patch
Comment 4 Geoffrey Garen 2020-12-11 10:38:10 PST
Comment on attachment 416007 [details]
Patch

r=me
Comment 5 Chris Dumez 2020-12-11 11:17:39 PST
Created attachment 416015 [details]
Patch
Comment 6 EWS 2020-12-11 16:32:33 PST
Committed r270718: <https://trac.webkit.org/changeset/270718>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416015 [details].