Bug 248293

Summary:	Uninitialized memory read when opening web inspector
Product:	WebKit	Reporter:	Michael Catanzaro <mcatanzaro>
Component:	Web Inspector	Assignee:	Michael Catanzaro <mcatanzaro>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	inspector-bugzilla-changes, mcatanzaro, webkit-bug-importer
Priority:	P2	Keywords:	InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
See Also:	https://bugs.webkit.org/show_bug.cgi?id=209468

Description Michael Catanzaro 2022-11-23 16:01:22 PST

This looks identical to bug #209468. Seems it was not fixed after all. Happens when opening the web inspector. Pretty sure the bug is that WebPageInspectorController::m_enabledBrowserAgent is not initially initialized to anything, so it's invalid to call InspectorBrowserAgent::enabled before InspectorBrowserAgent::enable or InspectorBrowserAgent::disable has been called. Let's hope that's the only problem here....

==573540== Conditional jump or move depends on uninitialised value(s)
==573540==    at 0x6D452DB: non-virtual thunk to WebKit::InspectorBrowserAgent::enable() (InspectorBrowserAgent.h:50)
==573540==    by 0xADBBDD0: Inspector::BrowserBackendDispatcher::enable(long, WTF::RefPtr<WTF::JSONImpl::Object, WTF::RawPtrTraits<WTF::JSONImpl::Object>, WTF::DefaultRefDerefTraits<WTF::JSONImpl::Object> >&&) (InspectorBackendDispatchers.cpp:566)
==573540==    by 0xADBC127: Inspector::BrowserBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<WTF::JSONImpl::Object, WTF::RawPtrTraits<WTF::JSONImpl::Object> >&&) (InspectorBackendDispatchers.cpp:546)
==573540==    by 0xADEBFF5: Inspector::BackendDispatcher::dispatch(WTF::String const&) (InspectorBackendDispatcher.cpp:183)
==573540==    by 0x6954B05: operator()<WTF::String> (HandleMessage.h:133)
==573540==    by 0x6954B05: __invoke_impl<void, IPC::callMemberFunction<WebKit::WebInspectorUIProxy, WebKit::WebInspectorUIProxy, void(const WTF::String&), std::tuple<WTF::String> >(WebKit::WebInspectorUIProxy*, void (WebKit::WebInspectorUIProxy::*)(const WTF::String&), std::tuple<WTF::String>&&)::<lambda(auto:60&& ...)>, WTF::String> (invoke.h:61)
==573540==    by 0x6954B05: __invoke<IPC::callMemberFunction<WebKit::WebInspectorUIProxy, WebKit::WebInspectorUIProxy, void(const WTF::String&), std::tuple<WTF::String> >(WebKit::WebInspectorUIProxy*, void (WebKit::WebInspectorUIProxy::*)(const WTF::String&), std::tuple<WTF::String>&&)::<lambda(auto:60&& ...)>, WTF::String> (invoke.h:96)
==573540==    by 0x6954B05: __apply_impl<IPC::callMemberFunction<WebKit::WebInspectorUIProxy, WebKit::WebInspectorUIProxy, void(const WTF::String&), std::tuple<WTF::String> >(WebKit::WebInspectorUIProxy*, void (WebKit::WebInspectorUIProxy::*)(const WTF::String&), std::tuple<WTF::String>&&)::<lambda(auto:60&& ...)>, std::tuple<WTF::String>, 0> (tuple:1852)
==573540==    by 0x6954B05: apply<IPC::callMemberFunction<WebKit::WebInspectorUIProxy, WebKit::WebInspectorUIProxy, void(const WTF::String&), std::tuple<WTF::String> >(WebKit::WebInspectorUIProxy*, void (WebKit::WebInspectorUIProxy::*)(const WTF::String&), std::tuple<WTF::String>&&)::<lambda(auto:60&& ...)>, std::tuple<WTF::String> > (tuple:1863)
==573540==    by 0x6954B05: callMemberFunction<WebKit::WebInspectorUIProxy, WebKit::WebInspectorUIProxy, void(const WTF::String&), std::tuple<WTF::String> > (HandleMessage.h:131)
==573540==    by 0x6954B05: handleMessage<Messages::WebInspectorUIProxy::SendMessageToBackend, WebKit::WebInspectorUIProxy, WebKit::WebInspectorUIProxy, void(const WTF::String&)> (HandleMessage.h:213)
==573540==    by 0x6954B05: WebKit::WebInspectorUIProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) (WebInspectorUIProxyMessageReceiver.cpp:57)
==573540==    by 0x6B571B7: IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) (MessageReceiverMap.cpp:129)
==573540==    by 0x6C028B1: WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) (WebProcessProxy.cpp:883)
==573540==    by 0x6B5011C: IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) (Connection.cpp:1242)
==573540==    by 0x6B51BEB: IPC::Connection::dispatchIncomingMessages() [clone .part.0] (Connection.cpp:1358)
==573540==    by 0xB68A75D: operator() (Function.h:82)
==573540==    by 0xB68A75D: WTF::RunLoop::performWork() (RunLoop.cpp:146)
==573540==    by 0xB6E61B8: WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) (RunLoopGLib.cpp:80)
==573540==    by 0xB6E6B0E: operator() (RunLoopGLib.cpp:53)
==573540==    by 0xB6E6B0E: WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)#1}::_FUN(_GSource*, int (*)(void*), void*) (RunLoopGLib.cpp:56)
==573540==  Uninitialised value was created by a heap allocation
==573540==    at 0x484286F: malloc (vg_replace_malloc.c:393)
==573540==    by 0xB6F26DF: pas_debug_heap_allocate (pas_debug_heap.h:101)
==573540==    by 0xB6F26DF: pas_try_allocate_intrinsic_impl_casual_case (pas_try_allocate_intrinsic.h:104)
==573540==    by 0xB6F26DF: bmalloc_allocate_impl_casual_case.constprop.0 (bmalloc_heap_inlines.h:70)
==573540==    by 0xB6F2998: bmalloc_allocate_casual (bmalloc_heap.c:64)
==573540==    by 0x6C53452: operator new (WebPageInspectorController.h:49)
==573540==    by 0x6C53452: make_unique<WebKit::WebPageInspectorController, WebKit::WebPageProxy&> (unique_ptr.h:1065)
==573540==    by 0x6C53452: makeUnique<WebKit::WebPageInspectorController, WebKit::WebPageProxy&> (StdLibExtras.h:570)
==573540==    by 0x6C53452: WebKit::WebPageProxy::WebPageProxy(WebKit::PageClient&, WebKit::WebProcessProxy&, WTF::Ref<API::PageConfiguration, WTF::RawPtrTraits<API::PageConfiguration> >&&) (WebPageProxy.cpp:525)
==573540==    by 0x6C63DD1: create (WebPageProxy.cpp:473)
==573540==    by 0x6C63DD1: createWebPage (WebProcessProxy.cpp:609)
==573540==    by 0x6C63DD1: WebKit::WebProcessPool::createWebPage(WebKit::PageClient&, WTF::Ref<API::PageConfiguration, WTF::RawPtrTraits<API::PageConfiguration> >&&) (WebProcessPool.cpp:1107)
==573540==    by 0x6D141B1: webkitWebViewBaseCreateWebPage(_WebKitWebViewBase*, WTF::Ref<API::PageConfiguration, WTF::RawPtrTraits<API::PageConfiguration> >&&) (WebKitWebViewBase.cpp:2333)
==573540==    by 0x6CEE7C8: webkitWebContextCreatePageForWebView(_WebKitWebContext*, _WebKitWebView*, _WebKitUserContentManager*, _WebKitWebView*, _WebKitWebsitePolicies*) (WebKitWebContext.cpp:1994)
==573540==    by 0x6CFABC6: webkitWebViewConstructed(_GObject*) (WebKitWebView.cpp:747)
==573540==    by 0x4938BA9: ephy_web_view_constructed (ephy-web-view.c:3944)
==573540==    by 0x4A1249C: g_object_new_internal (gobject.c:2279)
==573540==    by 0x4A1324C: g_object_new_valist (gobject.c:2567)
==573540==    by 0x4A11D86: g_object_new (gobject.c:2040)

Comment 1 Michael Catanzaro 2022-11-23 16:06:38 PST

(In reply to Michael Catanzaro from comment #0)
> Let's hope that's the only
> problem here....

It is. Errors are gone.

Comment 2 Michael Catanzaro 2022-11-23 16:12:20 PST

Pull request: https://github.com/WebKit/WebKit/pull/6775

Comment 3 Michael Catanzaro 2022-11-23 16:12:26 PST

(In reply to Michael Catanzaro from comment #0)
> so it's invalid to call
> InspectorBrowserAgent::enabled before InspectorBrowserAgent::enable or
> InspectorBrowserAgent::disable has been called.

Well, not quite, because both these functions themselves call InspectorBrowserAgent::enabled. So InspectorBrowserAgent::enable and InspectorBrowserAgent::disable may fail spuriously.

Comment 4 EWS 2022-11-24 07:46:07 PST

Committed 256990@main (138c1e2a317b): <https://commits.webkit.org/256990@main>

Reviewed commits have been landed. Closing PR #6775 and removing active labels.

Comment 5 Radar WebKit Bug Importer 2022-11-24 07:47:17 PST

<rdar://problem/102648412>