Implement Write Barrier for all OpenSSL types

byroot · byroot · commit 2c7c6de69e3e · 2023-06-01T11:19:03.000+02:00
The vast majority have no reference so it's just a matter of setting the flags.

For the couple exception, they have very little references so it's
easy.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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:

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ static const rb_data_type_t ossl_bn_type = {`
`41`	`41`	`{`
`42`	`42`	`0, ossl_bn_free,`
`43`	`43`	`},`
`44`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`44`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`45`	`45`	`};`
`46`	`46`
`47`	`47`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ static const rb_data_type_t ossl_cipher_type = {`
`42`	`42`	`{`
`43`	`43`	`0, ossl_cipher_free,`
`44`	`44`	`},`
`45`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`45`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`46`	`46`	`};`
`47`	`47`
`48`	`48`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ static const rb_data_type_t ossl_config_type = {`
`22`	`22`	`{`
`23`	`23`	`0, nconf_free,`
`24`	`24`	`},`
`25`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`25`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`26`	`26`	`};`
`27`	`27`
`28`	`28`	`CONF *`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ static const rb_data_type_t ossl_digest_type = {`
`35`	`35`	`{`
`36`	`36`	`0, ossl_digest_free,`
`37`	`37`	`},`
`38`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`38`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`39`	`39`	`};`
`40`	`40`
`41`	`41`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ static const rb_data_type_t ossl_engine_type = {`
`78`	`78`	`{`
`79`	`79`	`0, ossl_engine_free,`
`80`	`80`	`},`
`81`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`81`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`82`	`82`	`};`
`83`	`83`
`84`	`84`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ static const rb_data_type_t ossl_netscape_spki_type = {`
`50`	`50`	`{`
`51`	`51`	`0, ossl_netscape_spki_free,`
`52`	`52`	`},`
`53`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`53`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`54`	`54`	`};`
`55`	`55`
`56`	`56`	`static VALUE`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ static const rb_data_type_t ossl_pkcs12_type = {`
`44`	`44`	`{`
`45`	`45`	`0, ossl_pkcs12_free,`
`46`	`46`	`},`
`47`		`- 0, 0, RUBY_TYPED_FREE_IMMEDIATELY,`
	`47`	`+ 0, 0, RUBY_TYPED_FREE_IMMEDIATELY \| RUBY_TYPED_WB_PROTECTED,`
`48`	`48`	`};`
`49`	`49`
`50`	`50`	`static VALUE`