From 3af2635f117f8da563d180bc1c58702aecb16e0c Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Thu, 6 Jul 2017 09:21:36 +0900 Subject: bio: prevent possible GC issue in ossl_obj2bio() Prevent the new object created by StringValue() from being GCed. Luckily, as none of the callers of ossl_obj2bio() reads from the returned BIO after possible triggering GC, this has not been a real problem. As a bonus, ossl_protect_obj2bio() function which is no longer used anywhere is removed. --- ext/openssl/ossl_bio.c | 13 +++---------- ext/openssl/ossl_bio.h | 3 +-- ext/openssl/ossl_config.c | 2 +- ext/openssl/ossl_pkcs12.c | 2 +- ext/openssl/ossl_pkcs7.c | 14 +++++++------- ext/openssl/ossl_pkey.c | 2 +- ext/openssl/ossl_pkey_dh.c | 2 +- ext/openssl/ossl_pkey_dsa.c | 2 +- ext/openssl/ossl_pkey_ec.c | 6 +++--- ext/openssl/ossl_pkey_rsa.c | 2 +- ext/openssl/ossl_ssl_session.c | 2 +- ext/openssl/ossl_x509cert.c | 2 +- ext/openssl/ossl_x509crl.c | 2 +- ext/openssl/ossl_x509req.c | 2 +- 14 files changed, 24 insertions(+), 32 deletions(-) (limited to 'ext/openssl') diff --git a/ext/openssl/ossl_bio.c b/ext/openssl/ossl_bio.c index 1609b097..ae46ac43 100644 --- a/ext/openssl/ossl_bio.c +++ b/ext/openssl/ossl_bio.c @@ -10,8 +10,9 @@ #include "ossl.h" BIO * -ossl_obj2bio(VALUE obj) +ossl_obj2bio(volatile VALUE *pobj) { + VALUE obj = *pobj; BIO *bio; if (RB_TYPE_P(obj, T_FILE)) { @@ -40,18 +41,10 @@ ossl_obj2bio(VALUE obj) bio = BIO_new_mem_buf(RSTRING_PTR(obj), RSTRING_LENINT(obj)); if (!bio) ossl_raise(eOSSLError, NULL); } - + *pobj = obj; return bio; } -BIO * -ossl_protect_obj2bio(VALUE obj, int *status) -{ - BIO *ret = NULL; - ret = (BIO*)rb_protect((VALUE (*)(VALUE))ossl_obj2bio, obj, status); - return ret; -} - VALUE ossl_membio2str0(BIO *bio) { diff --git a/ext/openssl/ossl_bio.h b/ext/openssl/ossl_bio.h index 1705d0ac..2c3d952b 100644 --- a/ext/openssl/ossl_bio.h +++ b/ext/openssl/ossl_bio.h @@ -10,8 +10,7 @@ #if !defined(_OSSL_BIO_H_) #define _OSSL_BIO_H_ -BIO *ossl_obj2bio(VALUE); -BIO *ossl_protect_obj2bio(VALUE,int*); +BIO *ossl_obj2bio(volatile VALUE *); VALUE ossl_membio2str0(BIO*); VALUE ossl_membio2str(BIO*); VALUE ossl_protect_membio2str(BIO*,int*); diff --git a/ext/openssl/ossl_config.c b/ext/openssl/ossl_config.c index ebf6ae2a..28392e20 100644 --- a/ext/openssl/ossl_config.c +++ b/ext/openssl/ossl_config.c @@ -41,7 +41,7 @@ DupConfigPtr(VALUE obj) OSSL_Check_Kind(obj, cConfig); str = rb_funcall(obj, rb_intern("to_s"), 0); - bio = ossl_obj2bio(str); + bio = ossl_obj2bio(&str); conf = NCONF_new(NULL); if(!conf){ BIO_free(bio); diff --git a/ext/openssl/ossl_pkcs12.c b/ext/openssl/ossl_pkcs12.c index 0b9c7816..8502a6de 100644 --- a/ext/openssl/ossl_pkcs12.c +++ b/ext/openssl/ossl_pkcs12.c @@ -178,7 +178,7 @@ ossl_pkcs12_initialize(int argc, VALUE *argv, VALUE self) if(rb_scan_args(argc, argv, "02", &arg, &pass) == 0) return self; passphrase = NIL_P(pass) ? NULL : StringValueCStr(pass); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); d2i_PKCS12_bio(in, &pkcs); DATA_PTR(self) = pkcs; BIO_free(in); diff --git a/ext/openssl/ossl_pkcs7.c b/ext/openssl/ossl_pkcs7.c index 4040355f..40cc5f23 100644 --- a/ext/openssl/ossl_pkcs7.c +++ b/ext/openssl/ossl_pkcs7.c @@ -209,7 +209,7 @@ ossl_pkcs7_s_read_smime(VALUE klass, VALUE arg) VALUE ret, data; ret = NewPKCS7(cPKCS7); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); out = NULL; pkcs7 = SMIME_read_PKCS7(in, &out); BIO_free(in); @@ -241,7 +241,7 @@ ossl_pkcs7_s_write_smime(int argc, VALUE *argv, VALUE klass) SafeGetPKCS7(pkcs7, p7); if(!NIL_P(data) && PKCS7_is_detached(p7)) flg |= PKCS7_DETACHED; - in = NIL_P(data) ? NULL : ossl_obj2bio(data); + in = NIL_P(data) ? NULL : ossl_obj2bio(&data); if(!(out = BIO_new(BIO_s_mem()))){ BIO_free(in); ossl_raise(ePKCS7Error, NULL); @@ -278,7 +278,7 @@ ossl_pkcs7_s_sign(int argc, VALUE *argv, VALUE klass) pkey = GetPrivPKeyPtr(key); /* NO NEED TO DUP */ flg = NIL_P(flags) ? 0 : NUM2INT(flags); ret = NewPKCS7(cPKCS7); - in = ossl_obj2bio(data); + in = ossl_obj2bio(&data); if(NIL_P(certs)) x509s = NULL; else{ x509s = ossl_protect_x509_ary2sk(certs, &status); @@ -334,7 +334,7 @@ ossl_pkcs7_s_encrypt(int argc, VALUE *argv, VALUE klass) else ciph = GetCipherPtr(cipher); /* NO NEED TO DUP */ flg = NIL_P(flags) ? 0 : NUM2INT(flags); ret = NewPKCS7(cPKCS7); - in = ossl_obj2bio(data); + in = ossl_obj2bio(&data); x509s = ossl_protect_x509_ary2sk(certs, &status); if(status){ BIO_free(in); @@ -385,7 +385,7 @@ ossl_pkcs7_initialize(int argc, VALUE *argv, VALUE self) if(rb_scan_args(argc, argv, "01", &arg) == 0) return self; arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); p7 = PEM_read_bio_PKCS7(in, &pkcs, NULL, NULL); if (!p7) { OSSL_BIO_reset(in); @@ -777,7 +777,7 @@ ossl_pkcs7_verify(int argc, VALUE *argv, VALUE self) x509st = GetX509StorePtr(store); flg = NIL_P(flags) ? 0 : NUM2INT(flags); if(NIL_P(indata)) indata = ossl_pkcs7_get_data(self); - in = NIL_P(indata) ? NULL : ossl_obj2bio(indata); + in = NIL_P(indata) ? NULL : ossl_obj2bio(&indata); if(NIL_P(certs)) x509s = NULL; else{ x509s = ossl_protect_x509_ary2sk(certs, &status); @@ -844,7 +844,7 @@ ossl_pkcs7_add_data(VALUE self, VALUE data) if(!PKCS7_content_new(pkcs7, NID_pkcs7_data)) ossl_raise(ePKCS7Error, NULL); } - in = ossl_obj2bio(data); + in = ossl_obj2bio(&data); if(!(out = PKCS7_dataInit(pkcs7, NULL))) goto err; for(;;){ if((len = BIO_read(in, buf, sizeof(buf))) <= 0) diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 6ab1b618..314d1d94 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -144,7 +144,7 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self) rb_scan_args(argc, argv, "11", &data, &pass); pass = ossl_pem_passwd_value(pass); - bio = ossl_obj2bio(data); + bio = ossl_obj2bio(&data); if (!(pkey = d2i_PrivateKey_bio(bio, NULL))) { OSSL_BIO_reset(bio); if (!(pkey = PEM_read_bio_PrivateKey(bio, NULL, ossl_pem_passwd_cb, (void *)pass))) { diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c index dd85b7b9..92832710 100644 --- a/ext/openssl/ossl_pkey_dh.c +++ b/ext/openssl/ossl_pkey_dh.c @@ -222,7 +222,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self) } else { arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); dh = PEM_read_bio_DHparams(in, NULL, NULL, NULL); if (!dh){ OSSL_BIO_reset(in); diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c index 85085419..cd4313b1 100644 --- a/ext/openssl/ossl_pkey_dsa.c +++ b/ext/openssl/ossl_pkey_dsa.c @@ -229,7 +229,7 @@ ossl_dsa_initialize(int argc, VALUE *argv, VALUE self) else { pass = ossl_pem_passwd_value(pass); arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); dsa = PEM_read_bio_DSAPrivateKey(in, NULL, ossl_pem_passwd_cb, (void *)pass); if (!dsa) { OSSL_BIO_reset(in); diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index 10800d23..5262d3b2 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -217,7 +217,7 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self) BIO *in; pass = ossl_pem_passwd_value(pass); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); ec = PEM_read_bio_ECPrivateKey(in, NULL, ossl_pem_passwd_cb, (void *)pass); if (!ec) { @@ -775,7 +775,7 @@ static VALUE ossl_ec_group_initialize(int argc, VALUE *argv, VALUE self) if ((group = EC_GROUP_dup(arg1_group)) == NULL) ossl_raise(eEC_GROUP, "EC_GROUP_dup"); } else { - BIO *in = ossl_obj2bio(arg1); + BIO *in = ossl_obj2bio(&arg1); group = PEM_read_bio_ECPKParameters(in, NULL, NULL, NULL); if (!group) { @@ -1381,7 +1381,7 @@ static VALUE ossl_ec_point_initialize(int argc, VALUE *argv, VALUE self) point = EC_POINT_bn2point(group, bn, NULL, ossl_bn_ctx); } else { - BIO *in = ossl_obj2bio(arg1); + BIO *in = ossl_obj2bio(&arg1); /* BUG: finish me */ diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c index 5aa09d0d..1fcdd52c 100644 --- a/ext/openssl/ossl_pkey_rsa.c +++ b/ext/openssl/ossl_pkey_rsa.c @@ -236,7 +236,7 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self) else { pass = ossl_pem_passwd_value(pass); arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); rsa = PEM_read_bio_RSAPrivateKey(in, NULL, ossl_pem_passwd_cb, (void *)pass); if (!rsa) { OSSL_BIO_reset(in); diff --git a/ext/openssl/ossl_ssl_session.c b/ext/openssl/ossl_ssl_session.c index 1b602a6c..661f53ee 100644 --- a/ext/openssl/ossl_ssl_session.c +++ b/ext/openssl/ossl_ssl_session.c @@ -49,7 +49,7 @@ static VALUE ossl_ssl_session_initialize(VALUE self, VALUE arg1) if ((ctx = SSL_get1_session(ssl)) == NULL) ossl_raise(eSSLSession, "no session available"); } else { - BIO *in = ossl_obj2bio(arg1); + BIO *in = ossl_obj2bio(&arg1); ctx = PEM_read_bio_SSL_SESSION(in, NULL, NULL, NULL); diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c index cecc3ca0..87086a7c 100644 --- a/ext/openssl/ossl_x509cert.c +++ b/ext/openssl/ossl_x509cert.c @@ -161,7 +161,7 @@ ossl_x509_initialize(int argc, VALUE *argv, VALUE self) return self; } arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); x509 = PEM_read_bio_X509(in, &x, NULL, NULL); DATA_PTR(self) = x; if (!x509) { diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c index f9819f58..035025ab 100644 --- a/ext/openssl/ossl_x509crl.c +++ b/ext/openssl/ossl_x509crl.c @@ -115,7 +115,7 @@ ossl_x509crl_initialize(int argc, VALUE *argv, VALUE self) return self; } arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); crl = PEM_read_bio_X509_CRL(in, &x, NULL, NULL); DATA_PTR(self) = x; if (!crl) { diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c index 220d2f40..15bc7052 100644 --- a/ext/openssl/ossl_x509req.c +++ b/ext/openssl/ossl_x509req.c @@ -123,7 +123,7 @@ ossl_x509req_initialize(int argc, VALUE *argv, VALUE self) return self; } arg = ossl_to_der_if_possible(arg); - in = ossl_obj2bio(arg); + in = ossl_obj2bio(&arg); req = PEM_read_bio_X509_REQ(in, &x, NULL, NULL); DATA_PTR(self) = x; if (!req) { -- cgit v1.2.3