Merge pull request #604 from casperisfine/ssl-write-barrier

Implement Write Barrier for all OpenSSL types

23 files changed, 51 insertions, 38 deletions

diff --git a/ext/openssl/ossl_bn.c b/ext/openssl/ossl_bn.c
index bf2bac36..ce0d3ec7 100644
--- a/ext/openssl/ossl_bn.c
+++ b/ext/openssl/ossl_bn.c
@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_bn_type = {
     {
 	0, ossl_bn_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c
index cb8fbc3c..110610e1 100644
--- a/ext/openssl/ossl_cipher.c
+++ b/ext/openssl/ossl_cipher.c
@@ -42,7 +42,7 @@ static const rb_data_type_t ossl_cipher_type = {
     {
 	0, ossl_cipher_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_config.c b/ext/openssl/ossl_config.c
index 0bac0274..0e598b4d 100644
--- a/ext/openssl/ossl_config.c
+++ b/ext/openssl/ossl_config.c
@@ -22,7 +22,7 @@ static const rb_data_type_t ossl_config_type = {
     {
         0, nconf_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 CONF *
diff --git a/ext/openssl/ossl_digest.c b/ext/openssl/ossl_digest.c
index fc326ec1..16aeeb81 100644
--- a/ext/openssl/ossl_digest.c
+++ b/ext/openssl/ossl_digest.c
@@ -35,7 +35,7 @@ static const rb_data_type_t ossl_digest_type = {
     {
 	0, ossl_digest_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_engine.c b/ext/openssl/ossl_engine.c
index 1abde7f7..9e86321d 100644
--- a/ext/openssl/ossl_engine.c
+++ b/ext/openssl/ossl_engine.c
@@ -78,7 +78,7 @@ static const rb_data_type_t ossl_engine_type = {
     {
 	0, ossl_engine_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_hmac.c b/ext/openssl/ossl_hmac.c
index 1a5f471a..c485ba7e 100644
--- a/ext/openssl/ossl_hmac.c
+++ b/ext/openssl/ossl_hmac.c
@@ -42,7 +42,7 @@ static const rb_data_type_t ossl_hmac_type = {
     {
 	0, ossl_hmac_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_ns_spki.c b/ext/openssl/ossl_ns_spki.c
index 9b114736..9bed1f33 100644
--- a/ext/openssl/ossl_ns_spki.c
+++ b/ext/openssl/ossl_ns_spki.c
@@ -50,7 +50,7 @@ static const rb_data_type_t ossl_netscape_spki_type = {
     {
 	0, ossl_netscape_spki_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c
index 9c8d768d..82f80d4a 100644
--- a/ext/openssl/ossl_ocsp.c
+++ b/ext/openssl/ossl_ocsp.c
@@ -86,7 +86,7 @@ static const rb_data_type_t ossl_ocsp_request_type = {
     {
 	0, ossl_ocsp_request_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -100,7 +100,7 @@ static const rb_data_type_t ossl_ocsp_response_type = {
     {
 	0, ossl_ocsp_response_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -114,7 +114,7 @@ static const rb_data_type_t ossl_ocsp_basicresp_type = {
     {
 	0, ossl_ocsp_basicresp_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -128,7 +128,7 @@ static const rb_data_type_t ossl_ocsp_singleresp_type = {
     {
 	0, ossl_ocsp_singleresp_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -142,7 +142,7 @@ static const rb_data_type_t ossl_ocsp_certid_type = {
     {
 	0, ossl_ocsp_certid_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_pkcs12.c b/ext/openssl/ossl_pkcs12.c
index fb947df1..164b2da4 100644
--- a/ext/openssl/ossl_pkcs12.c
+++ b/ext/openssl/ossl_pkcs12.c
@@ -44,7 +44,7 @@ static const rb_data_type_t ossl_pkcs12_type = {
     {
 	0, ossl_pkcs12_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c
index dbe53476..78dcbd66 100644
--- a/ext/openssl/ossl_pkcs7.c
+++ b/ext/openssl/ossl_pkcs7.c
@@ -65,7 +65,7 @@ const rb_data_type_t ossl_pkcs7_type = {
     {
 	0, ossl_pkcs7_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -79,7 +79,7 @@ static const rb_data_type_t ossl_pkcs7_signer_info_type = {
     {
 	0, ossl_pkcs7_signer_info_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -93,7 +93,7 @@ static const rb_data_type_t ossl_pkcs7_recip_info_type = {
     {
 	0, ossl_pkcs7_recip_info_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
index a8e97d0b..a5baa218 100644
--- a/ext/openssl/ossl_pkey.c
+++ b/ext/openssl/ossl_pkey.c
@@ -35,7 +35,7 @@ const rb_data_type_t ossl_evp_pkey_type = {
     {
 	0, ossl_evp_pkey_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c
index 92842f95..329c12d9 100644
--- a/ext/openssl/ossl_pkey_ec.c
+++ b/ext/openssl/ossl_pkey_ec.c
@@ -530,7 +530,7 @@ static const rb_data_type_t ossl_ec_group_type = {
     {
 	0, ossl_ec_group_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
@@ -1115,7 +1115,7 @@ static const rb_data_type_t ossl_ec_point_type = {
     {
 	0, ossl_ec_point_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index b5bd508c..ce6a5d4e 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -77,7 +77,7 @@ static const rb_data_type_t ossl_sslctx_type = {
     {
         ossl_sslctx_mark, ossl_sslctx_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
@@ -1553,6 +1553,10 @@ ossl_ssl_mark(void *ptr)
 {
     SSL *ssl = ptr;
     rb_gc_mark((VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_ptr_idx));
+
+    // Note: this reference is stored as @verify_callback so we don't need to mark it.
+    // However we do need to ensure GC compaction won't move it, hence why
+    // we call rb_gc_mark here.
     rb_gc_mark((VALUE)SSL_get_ex_data(ssl, ossl_ssl_ex_vcb_idx));
 }
 
@@ -1567,7 +1571,7 @@ const rb_data_type_t ossl_ssl_type = {
     {
         ossl_ssl_mark, ossl_ssl_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
@@ -1646,6 +1650,8 @@ ossl_ssl_initialize(int argc, VALUE *argv, VALUE self)
     SSL_set_ex_data(ssl, ossl_ssl_ex_ptr_idx, (void *)self);
     SSL_set_info_callback(ssl, ssl_info_cb);
     verify_cb = rb_attr_get(v_ctx, id_i_verify_callback);
+    // We don't need to trigger a write barrier because it's already
+    // an instance variable of this object.
     SSL_set_ex_data(ssl, ossl_ssl_ex_vcb_idx, (void *)verify_cb);
 
     rb_call_super(0, NULL);
diff --git a/ext/openssl/ossl_ssl_session.c b/ext/openssl/ossl_ssl_session.c
index 139a474b..c5df902c 100644
--- a/ext/openssl/ossl_ssl_session.c
+++ b/ext/openssl/ossl_ssl_session.c
@@ -19,7 +19,7 @@ const rb_data_type_t ossl_ssl_session_type = {
     {
 	0, ossl_ssl_session_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE ossl_ssl_session_alloc(VALUE klass)
diff --git a/ext/openssl/ossl_ts.c b/ext/openssl/ossl_ts.c
index b33ff10c..f698bdc7 100644
--- a/ext/openssl/ossl_ts.c
+++ b/ext/openssl/ossl_ts.c
@@ -83,7 +83,7 @@ static const rb_data_type_t ossl_ts_req_type = {
     {
         0, ossl_ts_req_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -97,7 +97,7 @@ static  const rb_data_type_t ossl_ts_resp_type = {
     {
         0, ossl_ts_resp_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static void
@@ -111,7 +111,7 @@ static const rb_data_type_t ossl_ts_token_info_type = {
     {
         0, ossl_ts_token_info_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_x509attr.c b/ext/openssl/ossl_x509attr.c
index 60846cfe..d1d8bb5e 100644
--- a/ext/openssl/ossl_x509attr.c
+++ b/ext/openssl/ossl_x509attr.c
@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_x509attr_type = {
     {
 	0, ossl_x509attr_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c
index 94435416..aa6b9bb7 100644
--- a/ext/openssl/ossl_x509cert.c
+++ b/ext/openssl/ossl_x509cert.c
@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_x509_type = {
     {
 	0, ossl_x509_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c
index 6c1d9153..80e29f9d 100644
--- a/ext/openssl/ossl_x509crl.c
+++ b/ext/openssl/ossl_x509crl.c
@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_x509crl_type = {
     {
 	0, ossl_x509crl_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_x509ext.c b/ext/openssl/ossl_x509ext.c
index e54102c7..ac46fcf1 100644
--- a/ext/openssl/ossl_x509ext.c
+++ b/ext/openssl/ossl_x509ext.c
@@ -55,7 +55,7 @@ static const rb_data_type_t ossl_x509ext_type = {
     {
 	0, ossl_x509ext_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
@@ -108,7 +108,7 @@ static const rb_data_type_t ossl_x509extfactory_type = {
     {
 	0, ossl_x509extfactory_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
diff --git a/ext/openssl/ossl_x509name.c b/ext/openssl/ossl_x509name.c
index 13a2b2c0..9591912f 100644
--- a/ext/openssl/ossl_x509name.c
+++ b/ext/openssl/ossl_x509name.c
@@ -46,7 +46,7 @@ static const rb_data_type_t ossl_x509name_type = {
     {
 	0, ossl_x509name_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c
index 77a7d3f2..f0581851 100644
--- a/ext/openssl/ossl_x509req.c
+++ b/ext/openssl/ossl_x509req.c
@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_x509req_type = {
     {
 	0, ossl_x509req_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_x509revoked.c b/ext/openssl/ossl_x509revoked.c
index 10b8aa4a..108447c8 100644
--- a/ext/openssl/ossl_x509revoked.c
+++ b/ext/openssl/ossl_x509revoked.c
@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_x509rev_type = {
     {
 	0, ossl_x509rev_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
diff --git a/ext/openssl/ossl_x509store.c b/ext/openssl/ossl_x509store.c
index 7c546187..f27381ca 100644
--- a/ext/openssl/ossl_x509store.c
+++ b/ext/openssl/ossl_x509store.c
@@ -116,6 +116,9 @@ static void
 ossl_x509store_mark(void *ptr)
 {
     X509_STORE *store = ptr;
+    // Note: this reference is stored as @verify_callback so we don't need to mark it.
+    // However we do need to ensure GC compaction won't move it, hence why
+    // we call rb_gc_mark here.
     rb_gc_mark((VALUE)X509_STORE_get_ex_data(store, store_ex_verify_cb_idx));
 }
 
@@ -130,7 +133,7 @@ static const rb_data_type_t ossl_x509store_type = {
     {
         ossl_x509store_mark, ossl_x509store_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 /*
@@ -187,8 +190,9 @@ ossl_x509store_set_vfy_cb(VALUE self, VALUE cb)
     X509_STORE *store;
 
     GetX509Store(self, store);
-    X509_STORE_set_ex_data(store, store_ex_verify_cb_idx, (void *)cb);
     rb_iv_set(self, "@verify_callback", cb);
+    // We don't need to trigger a write barrier because `rb_iv_set` did it.
+    X509_STORE_set_ex_data(store, store_ex_verify_cb_idx, (void *)cb);
 
     return cb;
 }
@@ -507,6 +511,9 @@ static void
 ossl_x509stctx_mark(void *ptr)
 {
     X509_STORE_CTX *ctx = ptr;
+    // Note: this reference is stored as @verify_callback so we don't need to mark it.
+    // However we do need to ensure GC compaction won't move it, hence why
+    // we call rb_gc_mark here.
     rb_gc_mark((VALUE)X509_STORE_CTX_get_ex_data(ctx, stctx_ex_verify_cb_idx));
 }
 
@@ -526,7 +533,7 @@ static const rb_data_type_t ossl_x509stctx_type = {
     {
         ossl_x509stctx_mark, ossl_x509stctx_free,
     },
-    0, 0, RUBY_TYPED_FREE_IMMEDIATELY,
+    0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
 };
 
 static VALUE
@@ -614,8 +621,8 @@ ossl_x509stctx_verify(VALUE self)
     X509_STORE_CTX *ctx;
 
     GetX509StCtx(self, ctx);
-    X509_STORE_CTX_set_ex_data(ctx, stctx_ex_verify_cb_idx,
-			       (void *)rb_iv_get(self, "@verify_callback"));
+    VALUE cb = rb_iv_get(self, "@verify_callback");
+    X509_STORE_CTX_set_ex_data(ctx, stctx_ex_verify_cb_idx, (void *)cb);
 
     switch (X509_verify_cert(ctx)) {
       case 1: