Update from 1.0.0-stable

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.

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;

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

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);