After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 673893 - gvfsd-gphoto2 crashed with SIGSEGV in __pthread_mutex_destroy()
gvfsd-gphoto2 crashed with SIGSEGV in __pthread_mutex_destroy()
Product: gvfs
Classification: Core
Component: gphoto backend
Other Linux
: Normal major
: ---
Assigned To: Martin Pitt
Depends on:
Reported: 2012-04-11 09:19 UTC by Martin Pitt
Modified: 2012-04-11 11:12 UTC
See Also:
GNOME target: ---
GNOME version: ---

gphoto2: Initialize mutex early enough (1.43 KB, patch)
2012-04-11 10:16 UTC, Martin Pitt
committed Details | Review

Description Martin Pitt 2012-04-11 09:19:24 UTC

When plugging in a camera, and gphoto fails to initialize it, gvfsd-gphoto2 crashes with:

  • #0 __pthread_mutex_destroy
    at pthread_mutex_destroy.c line 28
  • #1 g_cond_broadcast
    at /build/buildd/glib2.0-2.32.0/./glib/gthread-posix.c line 782
  • #2 release_device
    at gvfsbackendgphoto2.c line 621
  • #3 do_mount
    at gvfsbackendgphoto2.c line 1668
  • #4 g_vfs_job_run
    at gvfsjob.c line 198
  • #5 g_thread_pool_new
    at /build/buildd/glib2.0-2.32.0/./glib/gthreadpool.c line 496
  • #6 start_thread
    at pthread_create.c line 308
  • #7 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 112
  • #8 ??
  • #2 release_device
    at gvfsbackendgphoto2.c line 621

This cannot be true, before line 621 release_device() derefs gphoto2_backend dozens of times. So that 0x0 is wrong.

I think that gphoto2_backend->lock is also not NULL, as g_mutex_clear() derefs it: g_mutex_impl_free (mutex->p); it seems it's gphoto2_backend->lock->p which is NULL then, i. e. the mutex was not initialized properly.

The only access to the lock aside from the _lock()/_unlock() calls is in do_mount() line 1715, i. e. it gets initialized *after* release_device() is called (which tries to free it again).

So to fix this, we either need to move the g_mutex_init() further up the code, before the first release_device() call, or release_device() must check if the lock is initialized before trying to clear it. The former seems safer to me (as there is no official API to check whether a mutex is initialized).
Comment 1 Martin Pitt 2012-04-11 09:20:36 UTC
The "trace" expander swallowed parts of the analysis, so the description looks a bit odd now.
Comment 2 Martin Pitt 2012-04-11 10:10:40 UTC
Heh, when I try to run gvfsd-gphoto2 without arguments, I get a very similar crash:

$ LD_LIBRARY_PATH=common/.libs daemon/.libs/gvfsd-gphoto2 
initing 0x14db0e0
try_mount 0x14db0e0
finalizing 0x14db0e0
Segmentation fault

  • #0 __pthread_mutex_destroy
    at pthread_mutex_destroy.c line 28
  • #1 g_mutex_impl_free
    at /build/buildd/glib2.0-2.32.0/./glib/gthread-posix.c line 112
  • #2 g_mutex_clear
    at /build/buildd/glib2.0-2.32.0/./glib/gthread-posix.c line 187
  • #3 release_device
    at gvfsbackendgphoto2.c line 621
  • #4 g_vfs_backend_gphoto2_finalize
    at gvfsbackendgphoto2.c line 635
  • #5 g_object_unref
    at /build/buildd/glib2.0-2.32.0/./gobject/gobject.c line 3018
  • #6 g_vfs_job_mount_finalize
    at gvfsjobmount.c line 52
  • #7 g_object_unref
    at /build/buildd/glib2.0-2.32.0/./gobject/gobject.c line 3018
  • #8 daemon_main
    at daemon-main.c line 293
  • #9 main
    at daemon-main-generic.c line 39

I. e. again g_vfs_backend_gphoto2_finalize() calls release_device(), but do_mount() never ran.
Comment 3 Martin Pitt 2012-04-11 10:16:50 UTC
Created attachment 211818 [details] [review]
gphoto2: Initialize mutex early enough

This fixes the crashes that I can reproduce, and should also fix the reported crash. OK to push?
Comment 4 Tomas Bzatek 2012-04-11 11:06:53 UTC
Review of attachment 211818 [details] [review]:

You're right, nice catch. Please commit to master (still haven't branched for 3.5).
Comment 5 Martin Pitt 2012-04-11 11:12:16 UTC
Comment on attachment 211818 [details] [review]
gphoto2: Initialize mutex early enough

Pushed, thanks!