Avoid a race condition that could lead to a segmentation fault.
authorTobias Brunner <tobias@strongswan.org>
Thu, 25 Feb 2010 07:56:05 +0000 (08:56 +0100)
committerTobias Brunner <tobias@strongswan.org>
Thu, 25 Feb 2010 08:26:16 +0000 (09:26 +0100)
Let's assume the callback function of a callback job returns
JOB_REQUEUE_FAIR in one call and JOB_REQUEUE_NONE in the next. Before
this fix, the thread executing the callback job would requeue the job
before unregistering itself. If there was a context switch right after
the job got requeued, and if the thread that requeued the job never got
resumed until a second thread executed the job and, due to the return
value of JOB_REQUEUE_NONE, destroyed it, then when the first thread
eventually got resumed and tried to lock the mutex to unregister itself
the pointer wouldn't be valid anymore, thus resulting in a segmentation fault.

src/charon/processing/jobs/callback_job.c

index 7e35dcd..45e4911 100644 (file)
@@ -182,7 +182,7 @@ static void cancel(private_callback_job_t *this)
  */
 static void execute(private_callback_job_t *this)
 {
-       bool cleanup = FALSE;
+       bool cleanup = FALSE, requeue = FALSE;
 
        thread_cleanup_push((thread_cleanup_t)destroy, this);
 
@@ -206,8 +206,7 @@ static void execute(private_callback_job_t *this)
                                continue;
                        case JOB_REQUEUE_FAIR:
                        {
-                               charon->processor->queue_job(charon->processor,
-                                                                                        &this->public.job_interface);
+                               requeue = TRUE;
                                break;
                        }
                        case JOB_REQUEUE_NONE:
@@ -225,6 +224,11 @@ static void execute(private_callback_job_t *this)
        /* manually create a cancellation point to avoid that a cancelled thread
         * goes back into the thread pool */
        thread_cancellation_point();
+       if (requeue)
+       {
+               charon->processor->queue_job(charon->processor,
+                                                                        &this->public.job_interface);
+       }
        thread_cleanup_pop(cleanup);
 }