luakit / luakit

Fast, small, webkit based browser framework extensible by Lua.
https://luakit.github.io/
GNU General Public License v3.0
2.11k stars 251 forks source link

(Maybe) crash with recent libraries on certain web pages. #1017

Open taobert opened 2 years ago

taobert commented 2 years ago

Presis: On current Debian Sid, with current develop (luakit 2.3.1-29-g6df6f982), i'm seeing a segmentation fault if i go to Arch's download page.

I'd appreciate it if this could be independently confirmed (@msdemlei?), so that i can rule out it just being me having a broken install.

Current Behavior: The gdb backtrace is:

(gdb) bt
#0  0x0000555555592fdf in webview_get_endpoint (w=0x0) at widgets/webview.c:1336
#1  0x00005555555799df in ipc_channel_send (L=0x7fff9d3d1380) at clib/ipc.c:46
#2  0x00007ffff3356a26 in  () at /usr/lib/x86_64-linux-gnu/libluajit-5.1.so.2
#3  0x00007ffff33b11ee in lua_pcall () at /usr/lib/x86_64-linux-gnu/libluajit-5.1.so.2
#4  0x0000555555572ada in luaH_dofunction (L=0x7fff9d3d1380, nargs=2, nret=-1) at ./common/lualib.h:110
#5  0x00005555555737f3 in luaH_object_emit_signal
    (L=0x7fff9d3d1380, oud=-2, name=0x7fff85651ef8 "get-plugin-configuration", nargs=1, nret=0)
    at common/luaobject.c:337
#6  0x0000555555579bc5 in ipc_channel_recv (L=0x7fff9d3d1380, arg=0x5555559fe270 "\004\030", arglen=87)
    at clib/ipc.c:79
#7  0x000055555556c088 in ipc_recv_lua_ipc (UNUSED_ipc=0x555555abf120, msg=0x5555559fe270, length=87) at ipc.c:64
#8  0x000055555556f57d in ipc_dispatch (ipc=0x555555abf120, header=..., payload=0x5555559fe270) at common/ipc.c:65
#9  0x000055555556fb21 in ipc_recv_and_dispatch_or_enqueue (ipc=0x555555abf120) at common/ipc.c:185
#10 0x000055555556fb00 in ipc_recv_and_dispatch_or_enqueue (ipc=0x555555abf120) at common/ipc.c:180
#11 0x000055555556fb92 in ipc_recv (UNUSED_channel=0x7fff8c004d10, UNUSED_cond=G_IO_IN, ipc=0x555555abf120)
    at common/ipc.c:200
#12 0x00007ffff1a0560f in g_main_context_dispatch () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#13 0x00007ffff1a059c8 in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#14 0x00007ffff1a05c7f in g_main_loop_run () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x00007ffff3cd1265 in gtk_main () at /usr/lib/x86_64-linux-gnu/libgtk-3.so.0
#16 0x000055555556ef91 in main (argc=2, argv=0x7fffffffdca8) at luakit.c:254

Note that w passed into webview_get_endpoint is NULL. webview_get_endpoint immediately dereferences this, causing the crash.

Discussion: The NULL w is returned from webview_get_by_id called with a plausible looking page_id, but that's as far as i've got. webview_get_by_id is quite brief:

widget_t*
webview_get_by_id(guint64 view_id)
{
    for (unsigned i = 0; i < globalconf.webviews->len; i++) {
        widget_t *w = g_ptr_array_index(globalconf.webviews, i);
        if (webkit_web_view_get_page_id(WEBKIT_WEB_VIEW(w->widget)) == view_id)
            return w;
    }
    return NULL;
}

so assuming that the page_id is correct, perhaps globalconf.webviews is getting b0rked somehow, or perhaps the behaviour of g_ptr_array_index, webkit_web_view_get_page_id or WEBKIT_WEB_VIEW have changed.

It was not happening on a separate install of sid until i ran apt upgrade, now it does, so my supposition is that this is a change in a library rather than luakit. This is further supported by luakit 2.2 showing the same behaviour.

Workaround: For now, the patch:

diff --git a/widgets/webview.c b/widgets/webview.c
index 247c0ee8..14ad91f0 100644
--- a/widgets/webview.c
+++ b/widgets/webview.c
@@ -1333,6 +1333,10 @@ webview_connect_to_endpoint(widget_t *w, ipc_endpoint_t *ipc)
 ipc_endpoint_t *
 webview_get_endpoint(widget_t *w)
 {
+    if(NULL == w || NULL == w->info) {
+        error("webview_get_endpoint called with bad widget\n");
+        return 0;
+    }
     g_assert(w->info->tok == L_TK_WEBVIEW);
     webview_data_t *d = w->data;
     return d->ipc;

stops the crash, but can also stop the page loading, so is not really a viable fix, but may be a workable short term stop-gap.

msdemlei commented 2 years ago

On Sat, Oct 01, 2022 at 08:16:14PM -0700, taobert wrote:

I'd appreciate it if this could be independently confirmed @.***?), so that i can rule out it just being me having a broken install.

I've just tried the git HEAD on bullseye, and all is fine. It's a lot more convenient to test on sid when I'm home, so bear with me on that.

so assuming that the page_id is correct, perhaps globalconf.webviews is getting b0rked somehow,

Yes, that would be my guess, too. Since things appear to be fine on bullseye, I would guess that either changes in webkitgtk (shudder) or in glib (shudder, too, as that may be a subtle -- as in: concurreny -- problem then) are to be blamed. Ouch.

  • if(NULL == w || NULL == w->info) {
  • error("webview_get_endpoint called with bad widget\n");
  • return 0;
  • }

Uhhh... I'll need to be really desperate to even consider this :-)

msdemlei commented 2 years ago

On Sat, Oct 01, 2022 at 08:16:14PM -0700, taobert wrote:

I'd appreciate it if this could be independently confirmed @.***?),

Running in a sid podman, I interestingly get a "web process crashed" right on startup (reproducibly); this goes along with a note about 'BadShmSeg (invalid shared segment parameter)' on the console, so I'm going to believe this is related to the somewhat crazy setup through the podman. After a "Reload page", things are just fine, so... I'll not delve into this unless it actually hurts a use case anyone really cares about.

That aside, archlinux.org renders without a problem. I hence suspect there is some other ingredient to your segfault. For the record, my test is on i386, but these days that tends to uncover rather than hide bugs.

taobert commented 2 years ago

Thankyou. I'll try to figure out what i've got wrong then.