Add ASYNC_block_pause and ASYNC_unblock_pause

There are potential deadlock situations that can occur if code executing
within the context of a job aquires a lock, and then pauses the job. This
adds an ability to temporarily block pauses from occuring whilst performing
work and holding a lock.

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/crypto/async/async.c b/crypto/async/async.c
index d08ac132b7f7ff73624f40c93f282e0afa5779cc..9b9963fb5ed4625315eb7bc989385a6d7adcdd85 100644 (file)
--- a/crypto/async/async.c
+++ b/crypto/async/async.c
@@ -80,6 +80,7 @@ static async_ctx *async_ctx_new(void)
 
     async_fibre_init_dispatcher(&nctx->dispatcher);
     nctx->currjob = NULL;
+    nctx->blocked = 0;
     if(!async_set_ctx(nctx))
         goto err;
 
@@ -286,7 +287,9 @@ int ASYNC_pause_job(void)
 {
     ASYNC_JOB *job;
 
-    if(!async_get_ctx() || !async_get_ctx()->currjob) {
+    if (async_get_ctx() == NULL
+            || async_get_ctx()->currjob == NULL
+            || async_get_ctx()->blocked) {
         /*
          * Could be we've deliberately not been started within a job so this is
          * counted as success.
@@ -297,8 +300,8 @@ int ASYNC_pause_job(void)
     job = async_get_ctx()->currjob;
     job->status = ASYNC_JOB_PAUSING;
 
-    if(!async_fibre_swapcontext(&job->fibrectx,
-                               &async_get_ctx()->dispatcher, 1)) {
+    if (!async_fibre_swapcontext(&job->fibrectx,
+                                 &async_get_ctx()->dispatcher, 1)) {
         ASYNCerr(ASYNC_F_ASYNC_PAUSE_JOB, ASYNC_R_FAILED_TO_SWAP_CONTEXT);
         return 0;
     }
@@ -405,3 +408,28 @@ void ASYNC_clear_wake(ASYNC_JOB *job)
     async_read1(job->wait_fd, &dummy);
     job->wake_set = 0;
 }
+
+void ASYNC_block_pause(void)
+{
+    if (async_get_ctx() == NULL
+            || async_get_ctx()->currjob == NULL) {
+        /*
+         * We're not in a job anyway so ignore this
+         */
+        return;
+    }
+    async_get_ctx()->blocked++;
+}
+
+void ASYNC_unblock_pause(void)
+{
+    if (async_get_ctx() == NULL
+            || async_get_ctx()->currjob == NULL) {
+        /*
+         * We're not in a job anyway so ignore this
+         */
+        return;
+    }
+    if(async_get_ctx()->blocked > 0)
+        async_get_ctx()->blocked--;
+}

diff --git a/crypto/async/async_locl.h b/crypto/async/async_locl.h
index 90c0db56a367f0bda8fa3d52ddf10acf839558b4..ba329788b3a4811437c52c88da86b7b6b2e1f88f 100644 (file)
--- a/crypto/async/async_locl.h
+++ b/crypto/async/async_locl.h
@@ -63,6 +63,7 @@ typedef struct async_ctx_st async_ctx;
 struct async_ctx_st {
     async_fibre dispatcher;
     ASYNC_JOB *currjob;
+    unsigned int blocked;
 };
 
 struct async_job_st {

diff --git a/doc/crypto/ASYNC_start_job.pod b/doc/crypto/ASYNC_start_job.pod
index 4abe01726380e160fab4e85fb6be8cbc9032d6ec..86134b598f9507d57dda3dc7c83836f9bbde4360 100644 (file)
--- a/doc/crypto/ASYNC_start_job.pod
+++ b/doc/crypto/ASYNC_start_job.pod
@@ -4,7 +4,8 @@
 
 ASYNC_init_pool, ASYNC_free_pool, ASYNC_start_job, ASYNC_pause_job,
 ASYNC_in_job, ASYNC_get_wait_fd, ASYNC_get_current_job, ASYNC_wake,
-ASYNC_clear_wake - asynchronous job management functions
+ASYNC_clear_wake, ASYNC_block_pause, ASYNC_unblock_pause - asynchronous job
+management functions
 
 =head1 SYNOPSIS
 
@@ -21,6 +22,8 @@ ASYNC_clear_wake - asynchronous job management functions
  ASYNC_JOB *ASYNC_get_current_job(void);
  void ASYNC_wake(ASYNC_JOB *job);
  void ASYNC_clear_wake(ASYNC_JOB *job);
+ void ASYNC_block_pause(void);
+ void ASYNC_unblock_pause(void);
 
 =head1 DESCRIPTION
 
@@ -121,6 +124,20 @@ engine can signal to the user code that the job should be resumed using
 ASYNC_wake(). Once resumed the engine would clear the wake signal by calling
 ASYNC_clear_wake().
 
+The ASYNC_block_pause() function will prevent the currently active job from
+pausing. The block will remain in place until a subsequent call to
+ASYNC_unblock_pause(). These functions can be nested, e.g. if you call
+ASYNC_block_pause() twice then you must call ASYNC_unblock_pause() twice in
+order to reenable pausing. If these functions are called while there is no
+currently active job then they have no effect. This functionality can be useful
+to avoid deadlock scenarios. For example during the execution of an ASYNC_JOB an
+application aquires a lock. It then calls some cryptographic function which
+invokes ASYNC_pause_job(). This returns control back to the code that created
+the ASYNC_JOB. If that code then attempts to aquire the same lock before
+resuming the original job then a deadlock can occur. By calling
+ASYNC_block_pause() immediately after aquiring the lock and
+ASYNC_unblock_pause() immediately before releasing it then this situation cannot
+occur.
 
 =head1 RETURN VALUES
 

diff --git a/include/openssl/async.h b/include/openssl/async.h
index 9592cd6757a2dd80606a79091ce63f61cef7d22d..6e7cf72b0b9deb5ff6628a733eea7a00570686c8 100644 (file)
--- a/include/openssl/async.h
+++ b/include/openssl/async.h
@@ -78,6 +78,8 @@ int ASYNC_get_wait_fd(ASYNC_JOB *job);
 ASYNC_JOB *ASYNC_get_current_job(void);
 void ASYNC_wake(ASYNC_JOB *job);
 void ASYNC_clear_wake(ASYNC_JOB *job);
+void ASYNC_block_pause(void);
+void ASYNC_unblock_pause(void);
 
 /* BEGIN ERROR CODES */
 /*

diff --git a/test/asynctest.c b/test/asynctest.c
index 9ce23fbec2d26bb9c038092b87d045357638a397..d89e8ad78937771c532813dfbacc87de0bb47352 100644 (file)
--- a/test/asynctest.c
+++ b/test/asynctest.c
@@ -114,6 +114,16 @@ static int wake(void *args)
     return 1;
 }
 
+static int blockpause(void *args)
+{
+    ASYNC_block_pause();
+    ASYNC_pause_job();
+    ASYNC_unblock_pause();
+    ASYNC_pause_job();
+
+    return 1;
+}
+
 static int test_ASYNC_init_pool()
 {
     ASYNC_JOB *job1 = NULL, *job2 = NULL, *job3 = NULL;
@@ -210,8 +220,6 @@ static int test_ASYNC_get_wait_fd()
     ASYNC_JOB *job = NULL;
     int funcret, fd;
 
-    currjob = NULL;
-
     if (       !ASYNC_init_pool(1, 0)
             || ASYNC_start_job(&job, &funcret, wake, NULL, 0)
                 != ASYNC_PAUSE
@@ -235,6 +243,27 @@ static int test_ASYNC_get_wait_fd()
     ASYNC_free_pool();
     return 1;
 }
+
+static int test_ASYNC_block_pause()
+{
+    ASYNC_JOB *job = NULL;
+    int funcret;
+
+    if (       !ASYNC_init_pool(1, 0)
+            || ASYNC_start_job(&job, &funcret, blockpause, NULL, 0)
+                != ASYNC_PAUSE
+            || ASYNC_start_job(&job, &funcret, blockpause, NULL, 0)
+                != ASYNC_FINISH
+            || funcret != 1) {
+        fprintf(stderr, "test_ASYNC_block_pause() failed\n");
+        ASYNC_free_pool();
+        return 0;
+    }
+
+    ASYNC_free_pool();
+    return 1;
+}
+
 #endif
 
 int main(int argc, char **argv)
@@ -250,7 +279,8 @@ int main(int argc, char **argv)
     if (       !test_ASYNC_init_pool()
             || !test_ASYNC_start_job()
             || !test_ASYNC_get_current_job()
-            || !test_ASYNC_get_wait_fd()) {
+            || !test_ASYNC_get_wait_fd()
+            || !test_ASYNC_block_pause()) {
         return 1;
     }
 #endif

diff --git a/util/libeay.num b/util/libeay.num
index 06816fc1f344dae39e7799884cc55f68114506e7..3dc21373c1755858dda953587e2643be3d05e80f 100755 (executable)
--- a/util/libeay.num
+++ b/util/libeay.num
@@ -4659,3 +4659,5 @@ ASYNC_clear_wake                        5018      EXIST::FUNCTION:
 ASYNC_get_current_job                   5019   EXIST::FUNCTION:
 ASYNC_get_wait_fd                       5020   EXIST::FUNCTION:
 ERR_load_ASYNC_strings                  5021   EXIST::FUNCTION:
+ASYNC_unblock_pause                     5022   EXIST::FUNCTION:
+ASYNC_block_pause                       5023   EXIST::FUNCTION: