Bug 280150 – [GTK] Crash on inputting booking field on IRCTC website in ScriptMessageClientGtk::didPostMessage

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Akarshan Biswas]

Created attachment 472629 [details]
The text file containing back trace from the core dump after webkitgtk crashed

Experienced a weird crash while inputting fields during booking in irctc.co.in website.

Attaching back trace with this.
Unfortunately, reproducing this is difficult since it needs to be a valid login for filing up the reservation form for booking trains on that website and that too does not crash when I tried testing it later.
Maybe, the bt attached might shred some light.

Filing this issue nevertheless for improvement.

Also, Michael says: "the ScriptMessageClientGtk does not own its WebKitUserContentManager and just assumes it will never outlive the WebKitUserContentManager. Presumably that assumption is wrong."

[Michael Catanzaro]

WebKitNavigationClient has the same problem. Today I accidentally broke the process launcher when making local changes to WebKit. This resulted in a similar use after free:

(gdb) bt
#0  _g_log_abort (breakpoint=1) at ../../../../Projects/glib/glib/gmessages.c:429
#1  0x00007ff3750db686 in g_logv (log_domain=0x7ff37571ad66 "epiphany", log_level=G_LOG_LEVEL_WARNING, format=0x7ff37571af43 "Web process crashed", args=0x7ffe52366ea8) at ../../../../Projects/glib/glib/gmessages.c:1273
#2  0x00007ff3750db77d in g_log (log_domain=0x7ff37571ad66 "epiphany", log_level=G_LOG_LEVEL_WARNING, format=0x7ff37571af43 "Web process crashed") at ../../../../Projects/glib/glib/gmessages.c:1315
#3  0x00007ff37569f8c6 in process_terminated_cb (web_view=0x5084490, reason=WEBKIT_WEB_PROCESS_CRASHED, user_data=0x0) at ../../../../Projects/epiphany/embed/ephy-web-view.c:788
#4  0x00007ff375541c26 in g_cclosure_marshal_VOID__ENUM (closure=0x50a96f0, return_value=0x0, n_param_values=2, param_values=0x7ffe523672d0, invocation_hint=0x7ffe52367170, marshal_data=0x0) at ../../../../Projects/glib/gobject/gmarshal.c:972
#5  0x00007ff37553d182 in g_closure_invoke (closure=0x50a96f0, return_value=0x0, n_param_values=2, param_values=0x7ffe523672d0, invocation_hint=0x7ffe52367170) at ../../../../Projects/glib/gobject/gclosure.c:833
#6  0x00007ff37556025c in signal_emit_unlocked_R (node=0x7ffe52367420, detail=0, instance=0x5084490, emission_return=0x0, instance_and_params=0x7ffe523672d0) at ../../../../Projects/glib/gobject/gsignal.c:3887
#7  0x00007ff37555f14d in signal_emit_valist_unlocked (instance=0x5084490, signal_id=293, detail=0, var_args=0x7ffe52367628) at ../../../../Projects/glib/gobject/gsignal.c:3519
#8  0x00007ff37555de53 in g_signal_emit_valist (instance=0x5084490, signal_id=293, detail=0, var_args=0x7ffe52367628) at ../../../../Projects/glib/gobject/gsignal.c:3262
#9  0x00007ff37555f6de in g_signal_emit (instance=0x5084490, signal_id=293, detail=0) at ../../../../Projects/glib/gobject/gsignal.c:3582
#10 0x00007ff36f44dc55 in webkitWebViewWebProcessTerminated (webView=0x1, reason=(WEBKIT_WEB_PROCESS_EXCEEDED_MEMORY_LIMIT | WEBKIT_WEB_PROCESS_TERMINATED_BY_API | unknown: 0x4)) at /home/mcatanzaro/Projects/WebKit/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:5085
#11 0x00007ff36f41e868 in NavigationClient::processDidTerminate (this=<optimized out>, reason=WebKit::ProcessTerminationReason::ExceededCPULimit) at /home/mcatanzaro/Projects/WebKit/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp:117
#12 0x00007ff36f36ff0e in WebKit::WebPageProxy::dispatchProcessDidTerminate (this=0x7ff359000c40, reason=WebKit::ProcessTerminationReason::Crash) at /home/mcatanzaro/Projects/WebKit/Source/WebKit/UIProcess/WebPageProxy.cpp:10408
#13 0x00007ff36f3c53dc in WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch (this=this@entry=0x7ff3590000c0, reason=reason@entry=WebKit::ProcessTerminationReason::Crash) at /home/mcatanzaro/Projects/WebKit/Source/WebKit/UIProcess/WebProcessProxy.cpp:1293
#14 0x00007ff36f3c65f5 in WebKit::WebProcessProxy::didFinishLaunching (this=0x7ff3590000c0, launcher=<optimized out>, connectionIdentifier=...) at /home/mcatanzaro/Projects/WebKit/Source/WebKit/UIProcess/WebProcessProxy.cpp:1384
#15 0x00007ff36f4a8be5 in WebKit::ProcessLauncher::launchProcess()::$_0::operator()(GIOCondition) (this=0x7ff359025da8, condition=<optimized out>) at /home/mcatanzaro/Projects/WebKit/Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:258
#16 WTF::Detail::CallableWrapper<WebKit::ProcessLauncher::launchProcess()::$_0, int, GIOCondition>::call (this=0x7ff359025da0, in=<optimized out>) at WTF/Headers/wtf/Function.h:53
#17 0x00007ff375299caf in socket_source_dispatch (source=0x48d8800, callback=0x7ff36e6add50 <WTF::GSocketMonitor::socketSourceCallback(_GSocket*, GIOCondition, WTF::GSocketMonitor*)>, user_data=0x7ff359014550) at ../../../../Projects/glib/gio/gsocket.c:4266
#18 0x00007ff3750cdf45 in g_main_dispatch (context=0x4820530) at ../../../../Projects/glib/glib/gmain.c:3357
#19 0x00007ff3750cf2cc in g_main_context_dispatch_unlocked (context=0x4820530) at ../../../../Projects/glib/glib/gmain.c:4208
#20 0x00007ff3750cf48c in g_main_context_iterate_unlocked (context=0x4820530, block=1, dispatch=1, self=0x4825570) at ../../../../Projects/glib/glib/gmain.c:4273
#21 0x00007ff3750cf5ba in g_main_context_iteration (context=0x4820530, may_block=1) at ../../../../Projects/glib/glib/gmain.c:4338
#22 0x00007ff3752f2a44 in g_application_run (application=0x4865590, argc=1, argv=0x7ffe52367cf8) at ../../../../Projects/glib/gio/gapplication.c:2715
#23 0x0000000000402e07 in main (argc=1, argv=0x7ffe52367cf8) at ../../../../Projects/epiphany/src/ephy-main.c:445

These probably aren't the only broken clients. We should audit all API clients and make sure they're not using raw pointers to WebKitWebViews.

[Michael Catanzaro]

I just hit this crash on an internal Red Hat website, but wasn't able to reproduce it unfortunately. It would be easier to debug if it was reproducible. Anyway, my stack trace looks identical to the one you attached. Notably:

(gdb) frame 4
#4  0x00007f18c360d0a1 in ScriptMessageClientGtk::didPostMessage (this=0x7f18aa771840, serializedScriptValue=...)
    at /buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp:411
warning: 411	/buildstream/gnome/sdk/webkitgtk-6.0.bst/Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp: No such file or directory
(gdb) print m_manager
$1 = 0x55c993bc4a10
(gdb) print ((GObject*)m_manager)->ref_count
$2 = 2863311530

2863311530 is notably 0b10101010101010101010101010101010, so clearly the WebKitUserContentManager is not valid. Should have used a smart pointer....

[Akarshan Biswas]

If my understanding is correct, it is indeed  declared as a raw pointer here: https://github.com/WebKit/WebKit/blob/8bac10aa5b34def64db6d3d3cdabd4f75482b42c/Source/WebKit/UIProcess/API/glib/WebKitUserContentManager.cpp#L437 

Maybe using a smart pointer like [GRefPtr](https://trac.webkit.org/wiki/GRefPtr) will ensure its proper memory management?


I am sure if g_signal_emit() expects a raw pointer, we can pass it as m_manager.get()


How that raw pointer will handle after being passed to the g_signal_emit(), I have no idea tbh, so I won't comment further.

[Michael Catanzaro]

Right, it either needs to be GRefPtr, or GWeakPtr if that would lead to a reference cycle (which I need to verify). But ideally we would also understand why this is happening in the first place.

(In reply to Akarshan Biswas from comment #3) 
> How that raw pointer will handle after being passed to the g_signal_emit(),
> I have no idea tbh, so I won't comment further.

Doesn't matter, since there's no need to hold ownership there.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #1)
> WebKitNavigationClient has the same problem. Today I accidentally broke the
> process launcher when making local changes to WebKit. This resulted in a
> similar use after free:

Unfortunately the backtrace I posted here does not at all correspond to what I said was happening, so I'll mark these comments as Obsolete to keep the discussion focused on ScriptMessageClientGtk.

[Michael Catanzaro]

I found a reproducer. The bug occurs when unfocusing a form element in a tab in a related web view (a tab opened by another tab, controlled by the original tab) after the opener web view was closed. But sadly only on this one particular internal Red Hat website. :(

Problem is clear enough though: register five script message handlers, then unregister four of them, then destroy the WebKitUserContentManager and also the ScriptMessageClientGtk (so using a smart pointer wouldn't help here), leaving one message handler remaining....

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #6)
> then destroy the WebKitUserContentManager and also
> the ScriptMessageClientGtk (so using a smart pointer wouldn't help here),
> leaving one message handler remaining....

Sorry, the ScriptMessageClientGtk only gets destroyed when the message handler is unregistered, so a smart pointer would definitely help. Let's do that.

[Michael Catanzaro]

Epiphany fix: https://gitlab.gnome.org/GNOME/epiphany/-/merge_requests/1635

[Michael Catanzaro]

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

[EWS]

Committed 285924@main (55790984429f): <https://commits.webkit.org/285924@main>

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