Bug 20779 – segmentation fault in swfdec mozilla plugin

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Riccardo Magliocchetti]

Since swfdec-mozilla 0.8.0 we have a segmentation fault with webkit based browsers because we dereference a null pointer. Please note that swfdec is linux only for now.

I'm seeing this with midori 0.19 / 0.21-git and webkit 1.0.1 / r36309 so it's not a regression.

You can reproduce this by installing swfdec and swfdec-mozilla packages (see http://swfdec.freedesktop.org for instructions) and pointing your browser to any site that have a flash object.

This is the swfdec bug containing the patch that introduced the segmentation fault:
https://bugs.freedesktop.org/show_bug.cgi?id=16717

The bug tracking this issue in swfdec is here:
https://bugs.freedesktop.org/show_bug.cgi?id=17521

This is the stack trace:

[Switching to Thread 0xb57cd720 (LWP 7846)]
0xb4bdf742 in plugin_set_window (instance=0xb4c06820, window=0xb4c06828)
    at plugin.c:398
398         plugin_x11_setup_windowed (instance->pdata, (Window)
window->window, 
(gdb) bt 1 full
#0  0xb4bdf742 in plugin_set_window (instance=0xb4c06820, window=0xb4c06828)
    at plugin.c:398
No locals.
(More stack frames follow...)
(gdb) print window
$1 = (NPWindow *) 0xb4c06828
(gdb) print instance->pdata
$2 = (void *) 0x9d86000
(gdb) print window->ws_info
$3 = (void *) 0x0
(gdb) print window->ws_info->visual
Attempt to dereference a generic pointer.

[Benjamin Otte]

Yeah, it seems Webkit doesn't provide a ws_info field as it should according to http://developer.mozilla.org/en/NPWindow
The code to use it was added because using the wrong Visual might cause an X error, see https://bugs.freedesktop.org/show_bug.cgi?id=16717 and https://bugzilla.mozilla.org/show_bug.cgi?id=445250

[Jeff Cook]

I may be experiencing this bug as well -- not entirely sure if it's the same or different, but it looks the same to me. This occurs whenever I leave a page with a Flash element and swfdec, using epiphany-webkit 2.23.91 and webkit r36705. Again, the first time a Flash element is loaded, thing is fine, but when one tries to leave this page, thing become sad and crashes. :(

I've attached the full backtrace I encountered. Hope it helps.

Arch Linux w/ kernel 2.6.26 (x86_64)
Webkit r36705
Gnome 2.22.3 / epiphany-webkit 2.23.91
swfdec 0.8.0

Thanks in advance.

[Jeff Cook]

Created attachment 23602 [details]
A backtrace from a segfault caused by swfdec 0.8.0 and webkit.

A backtrace from a segfault caused by swfdec 0.8.0 and webkit.

[marcoil]

According to http://developer.mozilla.org/en/Gecko_Plugin_API_Reference/Drawing_and_Event_Handling and http://developer.mozilla.org/en/NPP_SetWindow , passing a null window indicates that the window is being destroyed. I seem to have misread that as window *and* ws_info, sorry for that.

It should be easy to fix, all's needed is to change the order of deleting ws_info and calling the plugin in PluginView::stop in both WebCore/plugins/gtk/PluginViewGtk.cpp:276 and WebCore/plugins/qt/PluginViewQt.cpp:234.

I can't patch myself right away, any takers?

[Jeff Cook]

I've included a patch that seems to resolve the issue. Thanks for your help and attention. : )

[Jeff Cook]

Created attachment 23935 [details]
Stop segfault on leaving page with plugin load

[Riccardo Magliocchetti]

I was doing the same thing(In reply to comment #5)
> I've included a patch that seems to resolve the issue. Thanks for your help and
> attention. : )

i was doing the exact same thing :) it still crash when you close the page with the flash file (webkit r37056 and midori 0.2.1). Should file a new bug?

[Jeff Cook]

Created attachment 23936 [details]
Stop segfault on leaving page with plugin load - Qt

[Jeff Cook]

I don't get that crash with epiphany-webkit. Perhaps you're experiencing a Midori bug?

[Marco Barisione]

I was doing the same, and this is why I assigned the bug to me :)

Can you please provide a ChangeLog entry with the patch? (maybe a patch for both gtk and qt is enough)
Did you test the qt patch?
When the patch is ready for review you should set the review field to "?", so a reviewer can review it.

Even fixing this bug I can still see problems (using GtkLauncher) with both swfdec and the adobe flash plugin, if you want you can open a bug report and try to fix that too.

[Riccardo Magliocchetti]

(In reply to comment #9)
> I don't get that crash with epiphany-webkit. Perhaps you're experiencing a
> Midori bug?

Nope, it happens with GtkLauncher too, filed bug 21240.

[Jeff Cook]

Thank you for the help Marco. I've uploaded a new patch with ChangeLog and marked it ready for review.

Thus far, I've been unable to get QtLauncher to function with swfdec, so I haven't been able to test the patch. I hope to figure this out shortly, and will mark that as needing review once I get that funny business sorted.

[Jeff Cook]

Created attachment 24017 [details]
Stop segfault on leaving page with plugin load -Gtk w/ changelog

[Marco Barisione]

Comment on attachment 24017 [details]
Stop segfault on leaving page with plugin load -Gtk w/ changelog

Patches that need a review should be marked as "?", not "+".

It's nice to have a link to the bug report in the changelog entry but I don't know if it's a rule or what.

The changelog entry should also say how the bug was fixed, not only describe the bug.

[Jeff Cook]

(In reply to comment #14)
> (From update of attachment 24017 [details] [edit])
> Patches that need a review should be marked as "?", not "+".
> 
> It's nice to have a link to the bug report in the changelog entry but I don't
> know if it's a rule or what.
> 
> The changelog entry should also say how the bug was fixed, not only describe
> the bug.
> 

Thanks Marco. I misclicked on the +, meant ?. Submitting updated patch.

[Jeff Cook]

Created attachment 24080 [details]
Patch for GTK with better changelog.

[Alp Toker]

(In reply to comment #16)
> Created an attachment (id=24080) [edit]
> Patch for GTK with better changelog.
> 

Hi,

in r37535 a fix for a plugin crasher was landed. It adds a quirk. I wonder if your patch achieves the same thing without the need for a quirk. If so, we can back out r37535 and land your patch instead.

Can you check this out? Thanks

[Riccardo Magliocchetti]

(In reply to comment #17)
> Hi,
> 
> in r37535 a fix for a plugin crasher was landed. It adds a quirk. I wonder if
> your patch achieves the same thing without the need for a quirk. If so, we can
> back out r37535 and land your patch instead.
> 
> Can you check this out? Thanks

Hi Alp,

Webkit r37567 + this patch + swfdec -> ok
Webkit r37567 + this patch + flash 10 rc -> ok
Webkit r37567 + this patch + reverted r37535 with a patch generated by trac  + flash 10 rc -> segmentation fault in adobe plugin

Please land this patch :)

[Holger Freyther]

Comment on attachment 24080 [details]
Patch for GTK with better changelog.

Looks sane. I did land this patch.

[Holger Freyther]

Comment on attachment 24080 [details]
Patch for GTK with better changelog.

Clearing review flag as this has been applied.

[Simon Hausmann]

Landed Jeff's patch for the Qt port in 38125

[Gustavo Noronha (kov)]

Closing, since it is landed; please reopen if needed.