From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 25 Feb 2010 07:56:05 +0000 (+0100)
Subject: Avoid a race condition that could lead to a segmentation fault.
X-Git-Tag: 4.4.0~297
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=608af0a44564606dc3d10c59e110e9b3a7e08e97;p=thirdparty%2Fstrongswan.git

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 7e35dcdcbd..45e49112e1 100644
--- 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);
 }