search

libvips / nip2

A spreadsheet-like GUI for libvips.
https://libvips.github.io/libvips/
GNU General Public License v2.0
360 stars 13 forks source link

Crash in icontainer_real_child_remove on at least 8.5.1 and 8.6.0 #75

Closed bgilbert closed 5 years ago

bgilbert commented 5 years ago

Fedora has received several reports of segfaults in icontainer_real_child_remove on Fedora 27 (8.5.1) and Fedora 28 (8.6.0).

mhirsch commented 5 years ago

I thought I'd note here that I also see this on Fedora 29, with nip2 8.6.0. Is this fedora specific?

mhirsch commented 5 years ago

I pulled from git just now and built it from source on Fedora 29 and get the same segfault.

jcupitt commented 5 years ago

Thanks for the report Benjamin.

Matt, the bug report seems to suggest this happens simply when launching the program, is that right?

mhirsch commented 5 years ago

Yes, that's right. I tried modifying icontainer_real_child_remove to return when passed a NULL child and was able to start nip2, but found additional segfaults when opening an image.

jcupitt commented 5 years ago

My guess would be something in glib has changed, and that's broken some assumption that nip2 is making about ordering. It's odd I've not seen this in Ubuntu.

I have:

$ pkg-config glib-2.0 --modversion
2.58.1

I'll try fedora28 in docker and see if I can make it happen there.

mhirsch commented 5 years ago

I have

$ pkg-config glib-2.0 --modversion 2.58.1

On Fedora 29.

Thanks for looking into this!

jcupitt commented 5 years ago

I made a debug nip2 for fedora28:

https://github.com/jcupitt/docker-builds/tree/master/nip2-fedora28

I see:

# gdb nip2
(gdb) run
(nip2:23): GLib-GObject-CRITICAL **: 13:27:22.769: g_value_get_pointer: assertion 'G_VALUE_HOLDS_POINTER (value)' failed

Program received signal SIGSEGV, Segmentation fault.
0x000000000043a255 in icontainer_real_child_remove (parent=0xa75400, child=0x0)
    at icontainer.c:645
645     iContainerClass *icontainer_child_class = ICONTAINER_GET_CLASS( child );

So I see the problem, I have a debugger, and I have symbols! I'll see if I can find the cause.

jcupitt commented 5 years ago

I think I've fixed it -- I was being lazy in icontainer and declaring a signal as taking a pointer argument, when actually it was an object. This is harmless on Ubuntu, but it seems Fedora has configured gobject with some extra run-time type checks that flag this and NULL out pointers, causing the crash. This branch now works for me in that docker container anyway,

I'm not sure where to apply this patch. It could potentially be pushed out all the way back to 8.5. What do you think, @bgilbert?

mhirsch commented 5 years ago

That's great news! After applying this patch is it possible to open an image in nip2? I wonder if there are similar pointer/object declarations, since I encountered other crashes after modifying that function to ignore null pointers.

jcupitt commented 5 years ago

I searched for similar problems and fixed them all, but you're right, I didn't actually test opening an image (duh).

I'll have a go.

jcupitt commented 5 years ago

Yes, it loads images fine \o/

jcupitt commented 5 years ago

OK, merged to master, I'll put out 8.7.1 with this fix in a few days. We can backport easily if necessary. Thank you for opening the issue!

mhirsch commented 5 years ago

Thank you for fixing it! :1st_place_medal:

bgilbert commented 5 years ago

@jcupitt Thanks for tracking this down! I can apply 3caba997b63b9514ae89b242e144ee75d0659494 in the 8.6.0 packaging for Fedora 28 and 29.

jcupitt commented 5 years ago

@bgilbert should I make an official 8.6.1 with this fix, or is it simpler to just patch your 8.6.0?

bgilbert commented 5 years ago

@jcupitt Either way is fine with me.

bgilbert commented 5 years ago

Update submitted: https://bodhi.fedoraproject.org/updates/FEDORA-2019-a60176d4e5 https://bodhi.fedoraproject.org/updates/FEDORA-2019-ef8c84b6a6

jcupitt commented 5 years ago

nip2-8.7.1 released with this fix. Thanks again everyone.