Make SSL_set_bio's ownership easier to reason about.

SSL_set_bio has some rather complex ownership story because whether rbio/wbio
are both owning depends on whether they are equal. Moreover, whether
SSL_set_bio(ssl, rbio, wbio) frees ssl->rbio depends on whether rbio is the
existing rbio or not. The current logic doesn't even get it right; see tests.

Simplify this. First, rbio and wbio are always owning. All the weird ownership
cases which we're stuck with for compatibility will live in SSL_set_bio. It
will internally BIO_up_ref if necessary and appropriately no-op the left or
right side as needed. It will then call more well-behaved ssl_set_rbio or
ssl_set_wbio functions as necessary.

Change-Id: I6b4b34e23ed01561a8c0aead8bb905363ee413bb
Reviewed-on: https://boringssl-review.googlesource.com/8240
Reviewed-by: Adam Langley <agl@google.com>

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 7f7bf5a..0d95695 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -477,11 +477,8 @@
   ssl_free_wbio_buffer(ssl);
   assert(ssl->bbio == NULL);
 
-  int free_wbio = ssl->wbio != ssl->rbio;
   BIO_free_all(ssl->rbio);
-  if (free_wbio) {
-    BIO_free_all(ssl->wbio);
-  }
+  BIO_free_all(ssl->wbio);
 
   BUF_MEM_free(ssl->init_buf);
 
@@ -523,19 +520,18 @@
   ssl->handshake_func = ssl3_accept;
 }
 
-void SSL_set_bio(SSL *ssl, BIO *rbio, BIO *wbio) {
+static void ssl_set_rbio(SSL *ssl, BIO *rbio) {
+  BIO_free_all(ssl->rbio);
+  ssl->rbio = rbio;
+}
+
+static void ssl_set_wbio(SSL *ssl, BIO *wbio) {
   /* If the output buffering BIO is still in place, remove it. */
   if (ssl->bbio != NULL) {
     ssl->wbio = BIO_pop(ssl->wbio);
   }
 
-  if (ssl->rbio != rbio) {
-    BIO_free_all(ssl->rbio);
-  }
-  if (ssl->wbio != wbio && ssl->rbio != ssl->wbio) {
-    BIO_free_all(ssl->wbio);
-  }
-  ssl->rbio = rbio;
+  BIO_free_all(ssl->wbio);
   ssl->wbio = wbio;
 
   /* Re-attach |bbio| to the new |wbio|. */
@@ -544,6 +540,31 @@
   }
 }
 
+void SSL_set_bio(SSL *ssl, BIO *rbio, BIO *wbio) {
+  /* For historical reasons, this function has many different cases in ownership
+   * handling. */
+
+  /* If the two arguments are equal, one fewer reference is granted than
+   * taken. */
+  if (rbio != NULL && rbio == wbio) {
+    BIO_up_ref(rbio);
+  }
+
+  /* If at most one of rbio or wbio is changed, only adopt one reference. */
+  if (rbio == SSL_get_rbio(ssl)) {
+    ssl_set_wbio(ssl, wbio);
+    return;
+  }
+  if (wbio == SSL_get_wbio(ssl)) {
+    ssl_set_rbio(ssl, rbio);
+    return;
+  }
+
+  /* Otherwise, adopt both references. */
+  ssl_set_rbio(ssl, rbio);
+  ssl_set_wbio(ssl, wbio);
+}
+
 BIO *SSL_get_rbio(const SSL *ssl) { return ssl->rbio; }
 
 BIO *SSL_get_wbio(const SSL *ssl) {