Replaced BIO_free with BIO_free_all
authorArmin Novak <armin.novak@thincast.com>
Thu, 8 Nov 2018 11:09:49 +0000 (12:09 +0100)
committerArmin Novak <armin.novak@thincast.com>
Thu, 8 Nov 2018 11:09:49 +0000 (12:09 +0100)
There is no point in using BIO_free with a custom recursion
to free up stacked BIOs if there is already BIO_free_all.
Using it consistently avoids memory leaks due to stacked BIOs
not being recursively freed.

libfreerdp/core/certificate.c
libfreerdp/core/gateway/rdg.c
libfreerdp/core/tcp.c
libfreerdp/core/transport.c
libfreerdp/crypto/crypto.c
libfreerdp/crypto/tls.c
winpr/libwinpr/sspi/Schannel/schannel_openssl.c
winpr/tools/makecert/makecert.c

index 0f3b6f0..65b5e07 100644 (file)
@@ -716,7 +716,7 @@ rdpRsaKey* key_new_from_content(const char* keycontent, const char* keyfile)
                goto out_free;
 
        rsa = PEM_read_bio_RSAPrivateKey(bio, NULL, NULL, NULL);
-       BIO_free(bio);
+       BIO_free_all(bio);
 
        if (!rsa)
        {
index ca678ce..51cb1e2 100644 (file)
@@ -1346,30 +1346,11 @@ void rdg_free(rdpRdg* rdg)
        if (!rdg)
                return;
 
-       if (rdg->tlsOut)
-       {
-               tls_free(rdg->tlsOut);
-               rdg->tlsOut = NULL;
-       }
-
-       if (rdg->tlsIn)
-       {
-               tls_free(rdg->tlsIn);
-               rdg->tlsIn = NULL;
-       }
-
-       if (rdg->http)
-       {
-               http_context_free(rdg->http);
-               rdg->http = NULL;
-       }
-
-       if (rdg->ntlm)
-       {
-               ntlm_free(rdg->ntlm);
-               rdg->ntlm = NULL;
-       }
-
+       tls_free(rdg->tlsOut);
+       tls_free(rdg->tlsIn);
+       http_context_free(rdg->http);
+       ntlm_free(rdg->ntlm);
+       BIO_free_all(rdg->frontBio);
        DeleteCriticalSection(&rdg->writeSection);
        free(rdg);
 }
index e25c543..41ef469 100644 (file)
@@ -627,16 +627,15 @@ static int transport_bio_buffered_new(BIO* bio)
        return 1;
 }
 
+/* Free the buffered BIO.
+ * Do not free other elements in the BIO stack,
+ * let BIO_free_all handle that. */
 static int transport_bio_buffered_free(BIO* bio)
 {
        WINPR_BIO_BUFFERED_SOCKET* ptr = (WINPR_BIO_BUFFERED_SOCKET*) BIO_get_data(bio);
-       BIO* next_bio = BIO_next(bio);
 
-       if (next_bio)
-       {
-               BIO_free(next_bio);
-               BIO_set_next(bio, NULL);
-       }
+       if (!ptr)
+               return 0;
 
        ringbuffer_destroy(&ptr->xmitBuffer);
        free(ptr);
index 9b186d6..a78d329 100644 (file)
@@ -1093,6 +1093,9 @@ BOOL transport_disconnect(rdpTransport* transport)
        if (!transport)
                return FALSE;
 
+       if (transport->rdg && (transport->rdg->frontBio == transport->frontBio))
+               transport->frontBio = NULL;
+
        if (transport->tls)
        {
                tls_free(transport->tls);
index 73dc91d..39875f7 100644 (file)
@@ -251,7 +251,7 @@ char* crypto_print_name(X509_NAME* name)
                BIO_read(outBIO, buffer, size);
        }
 
-       BIO_free(outBIO);
+       BIO_free_all(outBIO);
        return buffer;
 }
 
index 8e563b9..e312c42 100644 (file)
@@ -538,7 +538,7 @@ static BIO* BIO_new_rdp_tls(SSL_CTX* ctx, int client)
 
        if (!ssl)
        {
-               BIO_free(bio);
+               BIO_free_all(bio);
                return NULL;
        }
 
@@ -935,7 +935,7 @@ BOOL tls_accept(rdpTls* tls, BIO* underlying, rdpSettings* settings)
        }
 
        rsa = PEM_read_bio_RSAPrivateKey(bio, NULL, NULL, NULL);
-       BIO_free(bio);
+       BIO_free_all(bio);
 
        if (!rsa)
        {
@@ -979,7 +979,7 @@ BOOL tls_accept(rdpTls* tls, BIO* underlying, rdpSettings* settings)
        }
 
        x509 = PEM_read_bio_X509(bio, NULL, NULL, 0);
-       BIO_free(bio);
+       BIO_free_all(bio);
 
        if (!x509)
        {
@@ -1282,7 +1282,7 @@ fail:
        if (!rc)
                free(pemCert);
 
-       BIO_free(bio);
+       BIO_free_all(bio);
        return rc;
 }
 
@@ -1612,17 +1612,11 @@ void tls_free(rdpTls* tls)
                tls->ctx = NULL;
        }
 
-       if (tls->bio)
-       {
-               BIO_free(tls->bio);
-               tls->bio = NULL;
-       }
-
-       if (tls->underlying)
-       {
-               BIO_free(tls->underlying);
-               tls->underlying = NULL;
-       }
+       /* tls->underlying is a stacked BIO under tls->bio.
+        * BIO_free_all will free recursivly. */
+       BIO_free_all(tls->bio);
+       tls->bio = NULL;
+       tls->underlying = NULL;
 
        if (tls->PublicKey)
        {
index e208b32..f54a222 100644 (file)
@@ -125,7 +125,6 @@ int schannel_openssl_client_init(SCHANNEL_OPENSSL* context)
        {
                WLog_ERR(TAG, "BIO_new failed");
                goto out_bio_read_failed;
-               return -1;
        }
 
        status = BIO_set_write_buf_size(context->bioRead, SCHANNEL_CB_MAX_TOKEN);
@@ -183,10 +182,10 @@ out_write_alloc:
 out_read_alloc:
 out_bio_pair:
 out_set_write_buf_write:
-       BIO_free(context->bioWrite);
+       BIO_free_all(context->bioWrite);
 out_bio_write_failed:
 out_set_write_buf_read:
-       BIO_free(context->bioRead);
+       BIO_free_all(context->bioRead);
 out_bio_read_failed:
        SSL_free(context->ssl);
 out_ssl_new_failed:
@@ -324,10 +323,10 @@ out_write_buffer:
 out_read_buffer:
 out_bio_pair:
 out_set_write_buf_write:
-       BIO_free(context->bioWrite);
+       BIO_free_all(context->bioWrite);
 out_bio_write:
 out_set_write_buf_read:
-       BIO_free(context->bioRead);
+       BIO_free_all(context->bioRead);
 out_bio_read:
 out_use_certificate:
        SSL_free(context->ssl);
index 564ba0c..d356209 100644 (file)
@@ -767,7 +767,7 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa
 
                        free(x509_str);
                        x509_str = NULL;
-                       BIO_free(bio);
+                       BIO_free_all(bio);
                        bio = NULL;
 
                        if (context->pemFormat)
@@ -831,9 +831,7 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa
 
        ret = 1;
 out_fail:
-
-       if (bio)
-               BIO_free(bio);
+       BIO_free_all(bio);
 
        if (fp)
                fclose(fp);
@@ -954,9 +952,7 @@ out_fail:
        if (fp)
                fclose(fp);
 
-       if (bio)
-               BIO_free(bio);
-
+       BIO_free_all(bio);
        free(x509_str);
        free(filename);
        free(fullpath);
@@ -1176,7 +1172,7 @@ int makecert_context_process(MAKECERT_CONTEXT* context, int argc, char** argv)
 
                if (status < 0)
                {
-                       BIO_free(bio);
+                       BIO_free_all(bio);
                        return -1;
                }
 
@@ -1185,7 +1181,7 @@ int makecert_context_process(MAKECERT_CONTEXT* context, int argc, char** argv)
 
                if (!(x509_str = (BYTE*) malloc(length + 1)))
                {
-                       BIO_free(bio);
+                       BIO_free_all(bio);
                        return -1;
                }
 
@@ -1193,7 +1189,7 @@ int makecert_context_process(MAKECERT_CONTEXT* context, int argc, char** argv)
 
                if (status < 0)
                {
-                       BIO_free(bio);
+                       BIO_free_all(bio);
                        free(x509_str);
                        return -1;
                }
@@ -1230,7 +1226,7 @@ int makecert_context_process(MAKECERT_CONTEXT* context, int argc, char** argv)
                x509_str[length] = '\0';
                printf("%s", x509_str);
                free(x509_str);
-               BIO_free(bio);
+               BIO_free_all(bio);
        }
 
        /**