Avoid a race condition that could lead to a segmentation fault.

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.

diff --git a/src/charon/processing/jobs/callback_job.c b/src/charon/processing/jobs/callback_job.c
index 7e35dcdcbd48ec3142a58775ec434a56ca81a228..45e49112e108e5c49d16454d754aaa0c5a279bb1 100644 (file)
--- a/src/charon/processing/jobs/callback_job.c
+++ b/src/charon/processing/jobs/callback_job.c
@@ -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);
 }