s3-aio-fork: Fix a segfault in vfs_aio_fork
authorVolker Lendecke <vl@samba.org>
Sat, 31 Mar 2012 11:37:20 +0000 (13:37 +0200)
committerVolker Lendecke <vl@samba.org>
Sat, 31 Mar 2012 13:25:54 +0000 (15:25 +0200)
aio_suspend does not signal the main process with a signal, it just waits. The
aio_fork module does not use the signal at all, it directly calls back into the
main smbd by calling smbd_aio_complete_aio_ex. This is an abstraction
violation, but the alternative would have been to use signals where they are
not needed. However, in wait_for_aio_completion this bites us: With aio_fork we
call handle_aio_completed twice on the same aio_ex struct: Once from the call
to handle_aio_completion within the aio_fork module and once from the code in
wait_for_aio_completion.

This patch fixes it in a pretty bad way by introducing flag variables and more
state. But the mid-term plan is to replace the posix aio calls from the vfs and
do pread_send/recv and pwrite_send/recv at the vfs layer, so this will
significantly change anyway.

Thanks to Kirill Malkin <kirill.malkin@starboardstorage.com> for reporting this
crash!

Autobuild-User: Volker Lendecke <vl@samba.org>
Autobuild-Date: Sat Mar 31 15:25:55 CEST 2012 on sn-devel-104

source3/modules/vfs_aio_fork.c

index 30d4b93..fa3db93 100644 (file)
@@ -101,6 +101,8 @@ struct aio_child {
        bool dont_delete;       /* Marked as in use since last cleanup */
        bool cancelled;
        bool read_cmd;
+       bool called_from_suspend;
+       bool completion_done;
 };
 
 struct aio_child_list {
@@ -432,6 +434,10 @@ static void handle_aio_completion(struct event_context *event_ctx,
                       child->retval.size);
        }
 
+       if (child->called_from_suspend) {
+               child->completion_done = true;
+               return;
+       }
        aio_ex = (struct aio_extra *)child->aiocb->aio_sigevent.sigev_value.sival_ptr;
        smbd_aio_complete_aio_ex(aio_ex);
        TALLOC_FREE(aio_ex);
@@ -850,7 +856,9 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle,
                                             handle_aio_completion,
                                             child);
 
-                       while (1) {
+                       child->called_from_suspend = true;
+
+                       while (!child->completion_done) {
                                if (tevent_loop_once(ev) == -1) {
                                        goto out;
                                }
@@ -859,12 +867,6 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle,
                                        errno = EAGAIN;
                                        goto out;
                                }
-
-                               /* We set child->aiocb to NULL in our hooked
-                                * AIO_RETURN(). */
-                               if (child->aiocb == NULL) {
-                                       break;
-                               }
                        }
                }
        }