RESOLVED FIXED 283053
[GLIB] Invalid write reported by Valgrind on GSocketMonitor::sourceSocketCallback
https://bugs.webkit.org/show_bug.cgi?id=283053
Summary [GLIB] Invalid write reported by Valgrind on GSocketMonitor::sourceSocketCall...
Lauro Moura
Reported Wednesday, November 13, 2024 3:49:52 PM UTC
bug282910 originally reported an invalid memory access in GSocketMonitor::sourceSocketCallback. Originally I thought it was related to the SessionHost destructor being called prematurely, but even after ensuring we'd delete the SessionHost after it returns from the socket message handlers in https://github.com/WebKit/WebKit/pull/36533, the error continues. From the trace, somehow erasing `monitor->m_callback` (added in 285339@main) is triggering the destruction of the monitor immediately while still inside its socketSourceCallback. Does not seem to bring issues in release builds, but running under valgrind shows an invalid write while updating m_shouldDestroyCallback Maybe it's a matter of ensuring cleaning the callback is the last action in the sourceSocket callback. ``` ** (WPEWebProcess:2182124): WARNING **: 23:33:30.451: RemoteInspector failed to connect to inspector server at: 127.0.0.1:40907: GDBus.Error:org.freedesktop.portal.Error.NotAllowed: This call is not available inside the sandbox ==2181500== Invalid write of size 1 ==2181500== at 0x451DC8: WTF::GSocketMonitor::socketSourceCallback(_GSocket*, GIOCondition, WTF::GSocketMonitor*) (GSocketMonitor.cpp:51) ==2181500== by 0x4BCDBB0: socket_source_dispatch (gsocket.c:4267) ==2181500== by 0x4DC148D: g_main_dispatch.lto_priv.0 (gmain.c:3344) ==2181500== by 0x4E20716: UnknownInlinedFun (gmain.c:4152) ==2181500== by 0x4E20716: g_main_context_iterate_unlocked.isra.0 (gmain.c:4217) ==2181500== by 0x4DC1F76: g_main_loop_run (gmain.c:4419) ==2181500== by 0x453161: WTF::RunLoop::run() (RunLoopGLib.cpp:108) ==2181500== by 0x3781F5: WebDriver::WebDriverService::run(int, char**) (WebDriverService.cpp:186) ==2181500== by 0x375AED: main (WebDriverMain.cpp:39) ==2181500== Address 0x7aa7619 is 73 bytes inside a block of size 128 free'd ==2181500== at 0x484988F: free (vg_replace_malloc.c:985) ==2181500== by 0x45E459: bmalloc::DebugHeap::free(void*) (DebugHeap.cpp:139) ==2181500== by 0x45E739: pas_debug_heap_free (DebugHeap.cpp:238) ==2181500== by 0x4BAB8C: pas_try_deallocate_slow_no_cache (pas_deallocate.c:135) ==2181500== by 0x3E0BEC: pas_try_deallocate (pas_deallocate.h:209) ==2181500== by 0x3E0C75: pas_deallocate (pas_deallocate.h:221) ==2181500== by 0x3E579F: bmalloc_deallocate_inline (bmalloc_heap_inlines.h:592) ==2181500== by 0x3E5EFC: free (bmalloc.h:187) ==2181500== by 0x3E5EFC: WTF::fastFree(void*) (FastMalloc.cpp:617) ==2181500== by 0x3BD846: WTF::RefCounted<WTF::SocketConnection>::operator delete(void*) (RefCounted.h:199) ==2181500== by 0x3BC218: WTF::RefCounted<WTF::SocketConnection>::deref() const (RefCounted.h:204) ==2181500== by 0x3BA9D1: WTF::DefaultRefDerefTraits<WTF::SocketConnection>::derefIfNotNull(WTF::SocketConnection*) (Ref.h:62) ==2181500== by 0x3B89BC: WTF::Ref<WTF::SocketConnection, WTF::RawPtrTraits<WTF::SocketConnection>, WTF::DefaultRefDerefTraits<WTF::SocketConnection> >::~Ref() (Ref.h:82) ==2181500== Block was alloc'd at ==2181500== at 0x4846828: malloc (vg_replace_malloc.c:442) ==2181500== by 0x45E370: bmalloc::DebugHeap::malloc(unsigned long, bmalloc::FailureAction) (DebugHeap.cpp:118) ==2181500== by 0x45E5C6: pas_debug_heap_malloc (DebugHeap.cpp:223) ==2181500== by 0x462DF3: pas_debug_heap_allocate (pas_debug_heap.h:106) ==2181500== by 0x46374E: pas_try_allocate_intrinsic_impl_casual_case (pas_try_allocate_intrinsic.h:105) ==2181500== by 0x463F44: bmalloc_allocate_impl_casual_case (bmalloc_heap_inlines.h:69) ==2181500== by 0x464441: bmalloc_allocate_casual (bmalloc_heap.c:64) ==2181500== by 0x3E4A21: bmalloc_allocate_inline (bmalloc_heap_inlines.h:120) ==2181500== by 0x3E5995: malloc (bmalloc.h:79) ==2181500== by 0x3E5995: WTF::fastMalloc(unsigned long) (FastMalloc.cpp:559) ==2181500== by 0x3B892B: WTF::RefCounted<WTF::SocketConnection>::operator new(unsigned long) (RefCounted.h:199) ==2181500== by 0x3B51A7: WTF::SocketConnection::create(WTF::GRefPtr<_GSocketConnection>&&, WTF::HashMap<WTF::CString, std::pair<WTF::CString, void (*)(WTF::SocketConnection&, _GVariant*, void*)>, WTF::DefaultHash<WTF::CString>, WTF::HashTraits<WTF::CString>, WTF::HashTraits<std::pair<WTF::CString, void (*)(WTF::SocketConnection&, _GVariant*, void*)> >, WTF::HashTableTraits, (WTF::ShouldValidateKey)0> const&, void*) (SocketConnection.h:41) ==2181500== by 0x3B6A91: WebDriver::SessionHost::connectToBrowser(std::unique_ptr<WebDriver::ConnectToBrowserAsyncData, std::default_delete<WebDriver::ConnectToBrowserAsyncData> >&&)::{lambda()#1}::operator()()::{lambda(_GObject*, _GAsyncResult*, void*)#1}::operator()(_GObject*, _GAsyncResult*, void*) const (SessionHostGlib.cpp:217) ==2181500== ``` Tentative patch incoming.
Attachments
Michael Catanzaro
Comment 1 Wednesday, November 13, 2024 7:43:52 PM UTC
(In reply to Lauro Moura from comment #0) > Tentative patch incoming. Thanks :)
Lauro Moura
Comment 2 Thursday, November 14, 2024 2:00:54 PM UTC
EWS
Comment 3 Thursday, November 14, 2024 5:28:32 PM UTC
Committed 286596@main (5d047d3c9925): <https://commits.webkit.org/286596@main> Reviewed commits have been landed. Closing PR #36635 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.