Update from 1.0.0-stable

diff --git a/CHANGES b/CHANGES
index 65ff95a788d0b1e71be4fe798ac38aa82b0c14dc..d74262e1bba975aed2bc14130852d21919995608 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,16 @@
 
  Changes between 0.9.8k and 1.0  [xx XXX xxxx]
 
+  *) In BIO_pop() and BIO_push() use the ctrl argument (which was NULL) to
+     indicate the initial BIO being pushed or popped. This makes it possible
+     to determine whether the BIO is the one explicitly called or as a result
+     of the ctrl being passed down the chain. Fix BIO_pop() and SSL BIOs so
+     it handles reference counts correctly and doesn't zero out the I/O bio
+     when it is not being explicitly popped. WARNING: applications which
+     included workarounds for the old buggy behaviour will need to be modified
+     or they could free up already freed BIOs.
+     [Steve Henson]
+
   *) Rename uni2asc and asc2uni functions to OPENSSL_uni2asc and
      OPENSSL_asc2uni the original names were too generic and cause name
      clashes on Netware.

diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
index 3f52ae953c2946b88e6d04c360aa52a4c2b631f7..77f4de9c32413e8b28ad808ffab67873ca2a07f9 100644 (file)
--- a/crypto/bio/bio_lib.c
+++ b/crypto/bio/bio_lib.c
@@ -429,7 +429,7 @@ BIO *BIO_push(BIO *b, BIO *bio)
        if (bio != NULL)
                bio->prev_bio=lb;
        /* called to do internal processing */
-       BIO_ctrl(b,BIO_CTRL_PUSH,0,NULL);
+       BIO_ctrl(b,BIO_CTRL_PUSH,0,lb);
        return(b);
        }
 
@@ -441,7 +441,7 @@ BIO *BIO_pop(BIO *b)
        if (b == NULL) return(NULL);
        ret=b->next_bio;
 
-       BIO_ctrl(b,BIO_CTRL_POP,0,NULL);
+       BIO_ctrl(b,BIO_CTRL_POP,0,b);
 
        if (b->prev_bio != NULL)
                b->prev_bio->next_bio=b->next_bio;

diff --git a/doc/crypto/BIO_f_ssl.pod b/doc/crypto/BIO_f_ssl.pod
index f0b731731f505a5940d534f813bc542964e71d3b..bc5861ab34b31f2edce94ae4516234a81ccc8683 100644 (file)
--- a/doc/crypto/BIO_f_ssl.pod
+++ b/doc/crypto/BIO_f_ssl.pod
@@ -308,6 +308,15 @@ a client and also echoes the request to standard output.
 
  BIO_free_all(sbio);
 
+=head1 BUGS
+
+In OpenSSL versions before 1.0.0 the BIO_pop() call was handled incorrectly,
+the I/O BIO reference count was incorrectly incremented (instead of
+decremented) and dissociated with the SSL BIO even if the SSL BIO was not
+explicitly being popped (e.g. a pop higher up the chain). Applications which
+included workarounds for this bug (e.g. freeing BIOs more than once) should
+be modified to handle this fix or they may free up an already freed BIO.
+
 =head1 SEE ALSO
 
 TBA

diff --git a/ssl/bio_ssl.c b/ssl/bio_ssl.c
index da6dfd2262649de84bf469e9e40908022f68d95c..af319af302a1bc4a56f649e0522d14a6f43f6fdb 100644 (file)
--- a/ssl/bio_ssl.c
+++ b/ssl/bio_ssl.c
@@ -398,17 +398,19 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr)
                        }
                break;
        case BIO_CTRL_POP:
-               /* ugly bit of a hack */
-               if (ssl->rbio != ssl->wbio) /* we are in trouble :-( */
+               /* Only detach if we are the BIO explicitly being popped */
+               if (b == ptr)
                        {
-                       BIO_free_all(ssl->wbio);
-                       }
-               if (b->next_bio != NULL)
-                       {
-                       CRYPTO_add(&b->next_bio->references,1,CRYPTO_LOCK_BIO);
+                       /* Shouldn't happen in practice because the
+                        * rbio and wbio are the same when pushed.
+                        */
+                       if (ssl->rbio != ssl->wbio)
+                               BIO_free_all(ssl->wbio);
+                       if (b->next_bio != NULL)
+                               CRYPTO_add(&b->next_bio->references,-1,CRYPTO_LOCK_BIO);
+                       ssl->wbio=NULL;
+                       ssl->rbio=NULL;
                        }
-               ssl->wbio=NULL;
-               ssl->rbio=NULL;
                break;
        case BIO_C_DO_STATE_MACHINE:
                BIO_clear_retry_flags(b);