crypto/bio: Don't call method->destroy if method->create failed Update-Note: If create callback of BIO_METHOD fails, the destroy method will no longer be called. This means: - The create method is responsible for cleaning up any state if it fails. - The destroy method no longer needs to consider partial states when create fails. This is a slight behavior change to BIO_METHOD, but restores our previous behavior and aligns with OpenSSL. This reverts a regression introduced by https://boringssl-review.googlesource.com/c/boringssl/+/89189. Signed-off-by: Xiangfei Ding <xfding@google.com> Change-Id: I0155df499ff6ed1169943d49d7300f4f6a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/104048 Presubmit-BoringSSL-Verified: boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Benjamin <davidben@google.com>

diff --git a/crypto/bio/bio.cc b/crypto/bio/bio.cc
index 28c3f4b..7db3793 100644
--- a/crypto/bio/bio.cc
+++ b/crypto/bio/bio.cc

@@ -15,7 +15,6 @@
 #include <openssl/bio.h>
 
 #include <assert.h>
-#include <errno.h>
 #include <limits.h>
 #include <string.h>
 
@@ -47,6 +46,8 @@
   }
 
   if (method->create != nullptr && !method->create(ret.get())) {
+    // Matching OpenSSL, don't call `destroy` if `create` fails.
+    ret->method = nullptr;
     return nullptr;
   }
 
@@ -55,7 +56,8 @@
 
 Bio::~Bio() {
   BIO *next = BIO_pop(this);
-  if (method->destroy != nullptr) {
+  // `method` may be null if `create` failed.
+  if (method != nullptr && method->destroy != nullptr) {
     method->destroy(this);
   }
   CRYPTO_free_ex_data(&g_ex_data_class, &ex_data);

diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc
index ab58fbb..c2506d6 100644
--- a/crypto/bio/bio_test.cc
+++ b/crypto/bio/bio_test.cc

@@ -1577,5 +1577,21 @@
   }
 }
 
+TEST(BIOTest, CreateFailure) {
+  UniquePtr<BIO_METHOD> method(BIO_meth_new(0, nullptr));
+  ASSERT_TRUE(method);
+  ASSERT_TRUE(
+      BIO_meth_set_create(method.get(), [](BIO *) -> int { return 0; }));
+  ASSERT_TRUE(BIO_meth_set_destroy(method.get(), [](BIO *) -> int {
+    ADD_FAILURE() << "destroy must not be called if create failed";
+    return 0;
+  }));
+
+  // If `create` fails, `BIO_new` should return nullptr without calling
+  // `destroy`.
+  UniquePtr<BIO> bio(BIO_new(method.get()));
+  EXPECT_FALSE(bio);
+}
+
 }  // namespace
 BSSL_NAMESPACE_END