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 737842 - SIGSEGV when GVfsChannel is blocked
SIGSEGV when GVfsChannel is blocked
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks: 710986
 
 
Reported: 2014-10-03 12:33 UTC by Ondrej Holy
Modified: 2014-10-09 09:20 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
gvfschannel: do not crash when channel is blocked (2.39 KB, patch)
2014-10-08 14:26 UTC, Ondrej Holy 		committed Details | Review
gvfschannel: do not call close job if channel is blocked (861 bytes, patch)
2014-10-08 14:26 UTC, Ondrej Holy 		committed Details | Review
gvfschannel: don't abort when finalizing (927 bytes, patch)
2014-10-08 14:27 UTC, Ondrej Holy 		committed Details | Review

Description Ondrej Holy 2014-10-03 12:33:38 UTC 
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7de12a5 in g_vfs_job_emit_finished (job=0x0) at gvfsjob.c:322
322	  g_assert (!job->finished);

g_vfs_channel_send_reply is executed if channel got new request and backend->priv->block_requests = TRUE. Consequently send_reply_cb is executed and crashed if channel->priv->current_job = NULL.

It is pretty similar to the bug 675181 and attachement 240642.

+ Trace 234170

  • #0 g_vfs_job_emit_finished
    at gvfsjob.c line 322
  • #1 send_reply_cb
    at gvfschannel.c line 628
  • #2 g_task_return_now
    at gtask.c line 1076
  • #3 g_task_return
    at gtask.c line 1129
  • #4 async_ready_write_callback_wrapper
    at goutputstream.c line 741
  • #5 g_task_return_now
    at gtask.c line 1076
  • #6 complete_in_idle_cb
    at gtask.c line 1085
  • #7 g_main_dispatch
    at gmain.c line 3064
  • #8 g_main_context_dispatch
    at gmain.c line 3663
  • #9 g_main_context_iterate
    at gmain.c line 3734
  • #10 g_main_loop_run
    at gmain.c line 3928
  • #11 daemon_main
    at daemon-main.c line 405
  • #12 main
    at daemon-main-generic.c line 42 






Comment 1


Ondrej Holy





          2014-10-06 15:02:37 UTC
        



See downstream bug for additional debug info:
https://bugzilla.redhat.com/show_bug.cgi?id=1149760






Comment 2


Ondrej Holy





          2014-10-08 14:26:16 UTC
        



Created attachment 288044 [details] [review]
gvfschannel: do not crash when channel is blocked

Backend could crash, when channel is blocked and got new request unless current job is set. Therefor send_reply_cb is called without current job and cause following error:
    
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7de12a5 in g_vfs_job_emit_finished (job=0x0) at gvfsjob.c:322
322      g_assert (!job->finished);
    
To fix the problem be sure error job is set.






Comment 3


Ondrej Holy





          2014-10-08 14:26:54 UTC
        



Created attachment 288045 [details] [review]
gvfschannel: do not call close job if channel is blocked     

This is necessary to avoid SIGPIPE on send_replay.






Comment 4


Ondrej Holy





          2014-10-08 14:27:35 UTC
        



Created attachment 288046 [details] [review]
gvfschannel: don't abort when finalizing

Remove g_assert (channel->priv->backend_handle == NULL), because it could happen when backend is force unmounted.






Comment 5


Ondrej Holy





          2014-10-09 07:30:28 UTC
        



Ross notices that SIGPIPE is ignored in deamon_init:

#ifdef SIGPIPE
  /* Ignore SIGPIPE to avoid killing daemons on cancelled transfer *
   * See https://bugzilla.gnome.org/show_bug.cgi?id=649041         *
   */
  signal (SIGPIPE, SIG_IGN);
#endif

So we have to figure out why it isn't ignored properly:

Program received signal SIGPIPE, Broken pipe.




+
Trace
    234198






Thread 140737144416000
          (LWP 22436)


    

  • 
#0
write

    
                from
                /lib64/libpthread.so.0

    

    • 

  • 
#1
g_unix_output_stream_write

    
                at gunixoutputstream.c
                  line
                  367

    

    • 

  • 
#2
g_pollable_output_stream_default_write_nonblocking

    
                at gpollableoutputstream.c
                  line
                  156

    

    • 

  • 
#3
write_async_pollable

    
                at goutputstream.c
                  line
                  1580

    

    • 

  • 
#4
g_output_stream_real_write_async

    
                at goutputstream.c
                  line
                  1623

    

    • 

  • 
#5
g_output_stream_write_async

    
                at goutputstream.c
                  line
                  838

    

    • 

  • 
#6
g_vfs_channel_send_reply

    
                at gvfschannel.c
                  line
                  660

    

    • 

  • 
#7
g_vfs_read_channel_send_closed

    
                at gvfsreadchannel.c
                  line
                  284

    

    • 

  • 
#8
send_reply

    
                at gvfsjobcloseread.c
                  line
                  100

    

    • 

  • 
#9
g_cclosure_marshal_VOID__VOIDv

    
                at gmarshal.c
                  line
                  115

    

    • 

  • 
#10
g_type_class_meta_marshalv

    
                at gclosure.c
                  line
                  988

    

    • 

  • 
#11
_g_closure_invoke_va

    
                at gclosure.c
                  line
                  831

    

    • 

  • 
#12
g_signal_emit_valist

    
                at gsignal.c
                  line
                  3218

    

    • 

  • 
#13
g_signal_emit

    
                at gsignal.c
                  line
                  3365

    

    • 

  • 
#14
g_vfs_job_send_reply

    
                at gvfsjob.c
                  line
                  236

    

    • 

  • 
#15
g_vfs_job_succeeded

    
                at gvfsjob.c
                  line
                  302

    

    • 

  • 
#16
inject_error

    
                at gvfsbackendlocaltest.c
                  line
                  106

    

    • 

  • 
#17
do_close_read

    
                at gvfsbackendlocaltest.c
                  line
                  1001

    

    • 

  • 
#18
run

    
                at gvfsjobcloseread.c
                  line
                  116

    

    • 

  • 
#19
g_vfs_job_run

    
                at gvfsjob.c
                  line
                  197

    

    • 

  • 
#20
job_handler_callback

    
                at gvfsdaemon.c
                  line
                  216

    

    • 

  • 
#21
g_thread_pool_thread_proxy

    
                at gthreadpool.c
                  line
                  307

    

    • 

  • 
#22
g_thread_proxy

    
                at gthread.c
                  line
                  764

    

    • 

  • 
#23
start_thread

    
                from
                /lib64/libpthread.so.0

    

    • 

  • 
#24
clone

    
                from
                /lib64/libc.so.6

    

    • 













Comment 6


Ondrej Holy





          2014-10-09 08:23:35 UTC
        



The SIGPIPE is caused due to gdb rewrites signal handlers, however the patch is still useful. We don't want new jobs to be run if channel is blocked when force unmounting.






Comment 7


Alexander Larsson





          2014-10-09 08:30:38 UTC
        



Review of attachment 288044 [details] [review]:

::: daemon/gvfschannel.c
@@ +329,3 @@
+	  job = NULL;
+	  error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_CLOSED,
+	                               _("Channel blocked"));

This seems to leak req->data.






Comment 8


Alexander Larsson





          2014-10-09 08:33:26 UTC
        



Review of attachment 288045 [details] [review]:

As we ignore sigpipe outside gdb i don't think this is a good idea.
It complicates the code for no real reason imho.






Comment 9


Alexander Larsson





          2014-10-09 08:35:19 UTC
        



Review of attachment 288046 [details] [review]:

looks ok






Comment 10


Alexander Larsson





          2014-10-09 08:36:27 UTC
        



Review of attachment 288045 [details] [review]:

Actually, you're right, we should probably not start a new close job in the backend while unmounting...






Comment 11


Alexander Larsson





          2014-10-09 08:36:53 UTC
        



Review of attachment 288045 [details] [review]:

Change the commit message though.






Comment 12


Ondrej Holy





          2014-10-09 09:18:53 UTC
        



Comment on attachment 288046 [details] [review]
gvfschannel: don't abort when finalizing

commit 535a192f3fbba97eb2f2f1fdc2c8909a26a89fdb






Comment 13


Ondrej Holy





          2014-10-09 09:19:47 UTC
        



Comment on attachment 288045 [details] [review]
gvfschannel: do not call close job if channel is blocked     

commit ace3084150b2d4ac8c6719061365c2bc9baf5edb

with changed commit message






Comment 14


Ondrej Holy





          2014-10-09 09:20:15 UTC
        



Comment on attachment 288044 [details] [review]
gvfschannel: do not crash when channel is blocked

commit 3abf2f64900c8dfb033af748c0aadc0cf61523c5

with additional g_free






Comment 15


Ondrej Holy





          2014-10-09 09:20:45 UTC
        



Thanks for the reviews!