Unify <TYPE>_up_ref methods signature and behaviour.

Add a status return value instead of void.
Add some sanity checks on reference counter value.
Update the docs.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>

15 files changed, 71 insertions, 30 deletions

diff --git a/CHANGES b/CHANGES
index b096ec6f0f..10ec93fc17 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,7 +2,15 @@
  OpenSSL CHANGES
  _______________
 
- Changes between 1.0.2g and 1.1.0  [xx XXX xxxx]
+ Changes between 1.0.2h and 1.1.0  [xx XXX 2016]
+
+  *) Unify TYPE_up_ref(obj) methods signature.
+     SSL_CTX_up_ref(), SSL_up_ref(), X509_up_ref(), EVP_PKEY_up_ref(),
+     X509_CRL_up_ref(), X509_OBJECT_up_ref_count() methods are now returning an
+     int (instead of void) like all others TYPE_up_ref() methods.
+     So now these methods also check the return value of CRYPTO_atomic_add(),
+     and the validity of object reference counter.
+     [fdasilvayy@gmail.com]
 
   *) With Windows Visual Studio builds, the .pdb files are installed
      alongside the installed libraries and executables.  For a static
diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index a8fa301b31..94b311fa90 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -196,10 +196,16 @@ EVP_PKEY *EVP_PKEY_new(void)
     return ret;
 }
 
-void EVP_PKEY_up_ref(EVP_PKEY *pkey)
+int EVP_PKEY_up_ref(EVP_PKEY *pkey)
 {
     int i;
-    CRYPTO_atomic_add(&pkey->references, 1, &i, pkey->lock);
+
+    if (CRYPTO_atomic_add(&pkey->references, 1, &i, pkey->lock) <= 0)
+        return 0;
+
+    REF_PRINT_COUNT("EVP_PKEY", pkey);
+    REF_ASSERT_ISNT(i < 2);
+    return ((i > 1) ? 1 : 0);
 }
 
 /*
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index c4ca619d8c..3a8c657b85 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -417,18 +417,17 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x)
     return ret;
 }
 
-void X509_OBJECT_up_ref_count(X509_OBJECT *a)
+int X509_OBJECT_up_ref_count(X509_OBJECT *a)
 {
     switch (a->type) {
     default:
         break;
     case X509_LU_X509:
-        X509_up_ref(a->data.x509);
-        break;
+        return X509_up_ref(a->data.x509);
     case X509_LU_CRL:
-        X509_CRL_up_ref(a->data.crl);
-        break;
+        return X509_CRL_up_ref(a->data.crl);
     }
+    return 1;
 }
 
 X509 *X509_OBJECT_get0_X509(X509_OBJECT *a)
diff --git a/crypto/x509/x509_set.c b/crypto/x509/x509_set.c
index 360ead87d3..59a6b29f89 100644
--- a/crypto/x509/x509_set.c
+++ b/crypto/x509/x509_set.c
@@ -146,10 +146,16 @@ int X509_set_pubkey(X509 *x, EVP_PKEY *pkey)
     return (X509_PUBKEY_set(&(x->cert_info.key), pkey));
 }
 
-void X509_up_ref(X509 *x)
+int X509_up_ref(X509 *x)
 {
     int i;
-    CRYPTO_atomic_add(&x->references, 1, &i, x->lock);
+
+    if (CRYPTO_atomic_add(&x->references, 1, &i, x->lock) <= 0)
+        return 0;
+
+    REF_PRINT_COUNT("X509", x);
+    REF_ASSERT_ISNT(i < 2);
+    return ((i > 1) ? 1 : 0);
 }
 
 long X509_get_version(X509 *x)
diff --git a/crypto/x509/x509cset.c b/crypto/x509/x509cset.c
index ab5f192a15..d8259254f0 100644
--- a/crypto/x509/x509cset.c
+++ b/crypto/x509/x509cset.c
@@ -132,10 +132,16 @@ int X509_CRL_sort(X509_CRL *c)
     return 1;
 }
 
-void X509_CRL_up_ref(X509_CRL *crl)
+int X509_CRL_up_ref(X509_CRL *crl)
 {
     int i;
-    CRYPTO_atomic_add(&crl->references, 1, &i, crl->lock);
+
+    if (CRYPTO_atomic_add(&crl->references, 1, &i, crl->lock) <= 0)
+        return 0;
+
+    REF_PRINT_COUNT("X509_CRL", crl);
+    REF_ASSERT_ISNT(i < 2);
+    return ((i > 1) ? 1 : 0);
 }
 
 long X509_CRL_get_version(X509_CRL *crl)
diff --git a/doc/crypto/EVP_PKEY_new.pod b/doc/crypto/EVP_PKEY_new.pod
index 05ac08795e..b639c66454 100644
--- a/doc/crypto/EVP_PKEY_new.pod
+++ b/doc/crypto/EVP_PKEY_new.pod
@@ -9,7 +9,7 @@ EVP_PKEY_new, EVP_PKEY_up_ref, EVP_PKEY_free - private key allocation functions.
  #include <openssl/evp.h>
 
  EVP_PKEY *EVP_PKEY_new(void);
- void EVP_PKEY_up_ref(EVP_PKEY *key);
+ int EVP_PKEY_up_ref(EVP_PKEY *key);
  void EVP_PKEY_free(EVP_PKEY *key);
 
 
@@ -37,7 +37,7 @@ used.
 EVP_PKEY_new() returns either the newly allocated B<EVP_PKEY> structure or
 B<NULL> if an error occurred.
 
-EVP_PKEY_up_ref() and EVP_PKEY_free() do not return a value.
+EVP_PKEY_up_ref() returns 1 for success and 0 for failure.
 
 =head1 SEE ALSO
 
diff --git a/doc/crypto/X509_new.pod b/doc/crypto/X509_new.pod
index 8db6cdb245..484408c4b7 100644
--- a/doc/crypto/X509_new.pod
+++ b/doc/crypto/X509_new.pod
@@ -10,7 +10,7 @@ X509_new, X509_free, X509_up_ref - X509 certificate ASN1 allocation functions
 
  X509 *X509_new(void);
  void X509_free(X509 *a);
- void X509_up_ref(X509 *a);
+ int X509_up_ref(X509 *a);
  STACK_OF(X509) *X509_chain_up_ref(STACK_OF(X509) *x);
 
 =head1 DESCRIPTION
@@ -46,7 +46,7 @@ If the allocation fails, X509_new() returns B<NULL> and sets an error
 code that can be obtained by L<ERR_get_error(3)>.
 Otherwise it returns a pointer to the newly allocated structure.
 
-X509_free() and X509_up_ref() do not return a value.
+X509_up_ref() returns 1 for success and 0 for failure.
 
 X509_chain_up_ref() returns a copy of the stack or B<NULL> if an error
 occurred.
diff --git a/doc/ssl/SSL_CTX_new.pod b/doc/ssl/SSL_CTX_new.pod
index f2cdc717ef..8f232a0c6f 100644
--- a/doc/ssl/SSL_CTX_new.pod
+++ b/doc/ssl/SSL_CTX_new.pod
@@ -17,7 +17,7 @@ functions
  #include <openssl/ssl.h>
 
  SSL_CTX *SSL_CTX_new(const SSL_METHOD *method);
- void SSL_CTX_up_ref(SSL_CTX *ctx);
+ int SSL_CTX_up_ref(SSL_CTX *ctx);
 
  const SSL_METHOD *TLS_method(void);
  const SSL_METHOD *TLS_server_method(void);
@@ -184,6 +184,8 @@ the reason.
 
 The return value points to an allocated SSL_CTX object.
 
+SSL_CTX_up_ref() returns 1 for success and 0 for failure.
+
 =back
 
 =head1 HISTORY
diff --git a/doc/ssl/SSL_new.pod b/doc/ssl/SSL_new.pod
index f0e07951e3..cee6b24858 100644
--- a/doc/ssl/SSL_new.pod
+++ b/doc/ssl/SSL_new.pod
@@ -9,7 +9,7 @@ SSL_new, SSL_up_ref - create a new SSL structure for a connection
  #include <openssl/ssl.h>
 
  SSL *SSL_new(SSL_CTX *ctx);
- void SSL_up_ref(SSL *s);
+ int SSL_up_ref(SSL *s);
 
 =head1 DESCRIPTION
 
@@ -38,6 +38,8 @@ find out the reason.
 
 The return value points to an allocated SSL structure.
 
+SSL_up_ref() returns 1 for success and 0 for failure.
+
 =back
 
 =head1 SEE ALSO
diff --git a/doc/ssl/ssl.pod b/doc/ssl/ssl.pod
index 1ade6acda1..88198d1be7 100644
--- a/doc/ssl/ssl.pod
+++ b/doc/ssl/ssl.pod
@@ -271,7 +271,7 @@ protocol context defined in the B<SSL_CTX> structure.
 
 =item SSL_CTX *B<SSL_CTX_new>(const SSL_METHOD *meth);
 
-=item void SSL_CTX_up_ref(SSL_CTX *ctx);
+=item int SSL_CTX_up_ref(SSL_CTX *ctx);
 
 =item int B<SSL_CTX_remove_session>(SSL_CTX *ctx, SSL_SESSION *c);
 
@@ -599,7 +599,7 @@ fresh handle for each connection.
 
 =item SSL *B<SSL_new>(SSL_CTX *ctx);
 
-=item void SSL_up_ref(SSL *s);
+=item int SSL_up_ref(SSL *s);
 
 =item long B<SSL_num_renegotiations>(SSL *ssl);
 
diff --git a/include/openssl/evp.h b/include/openssl/evp.h
index 250730f4a0..bfb5dc8988 100644
--- a/include/openssl/evp.h
+++ b/include/openssl/evp.h
@@ -976,7 +976,7 @@ struct ec_key_st *EVP_PKEY_get1_EC_KEY(EVP_PKEY *pkey);
 # endif
 
 EVP_PKEY *EVP_PKEY_new(void);
-void EVP_PKEY_up_ref(EVP_PKEY *pkey);
+int EVP_PKEY_up_ref(EVP_PKEY *pkey);
 void EVP_PKEY_free(EVP_PKEY *pkey);
 
 EVP_PKEY *d2i_PublicKey(int type, EVP_PKEY **a, const unsigned char **pp,
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 0ab0df2749..cf02047d67 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1379,7 +1379,7 @@ void BIO_ssl_shutdown(BIO *ssl_bio);
 
 __owur int SSL_CTX_set_cipher_list(SSL_CTX *, const char *str);
 __owur SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth);
-void SSL_CTX_up_ref(SSL_CTX *ctx);
+int SSL_CTX_up_ref(SSL_CTX *ctx);
 void SSL_CTX_free(SSL_CTX *);
 __owur long SSL_CTX_set_timeout(SSL_CTX *ctx, long t);
 __owur long SSL_CTX_get_timeout(const SSL_CTX *ctx);
@@ -1554,7 +1554,7 @@ __owur int SSL_CTX_set_session_id_context(SSL_CTX *ctx, const unsigned char *sid
                                    unsigned int sid_ctx_len);
 
 SSL *SSL_new(SSL_CTX *ctx);
-void SSL_up_ref(SSL *s);
+int SSL_up_ref(SSL *s);
 __owur int SSL_set_session_id_context(SSL *ssl, const unsigned char *sid_ctx,
                                unsigned int sid_ctx_len);
 
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 009ee6aa4c..febe621b5b 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -674,7 +674,7 @@ int X509_set_notBefore(X509 *x, const ASN1_TIME *tm);
 ASN1_TIME *X509_get_notAfter(X509 *x);
 int X509_set_notAfter(X509 *x, const ASN1_TIME *tm);
 int X509_set_pubkey(X509 *x, EVP_PKEY *pkey);
-void X509_up_ref(X509 *x);
+int X509_up_ref(X509 *x);
 int X509_get_signature_type(const X509 *x);
 /*
  * This one is only used so that a binary form can output, as in
@@ -731,7 +731,7 @@ int X509_CRL_set_issuer_name(X509_CRL *x, X509_NAME *name);
 int X509_CRL_set_lastUpdate(X509_CRL *x, const ASN1_TIME *tm);
 int X509_CRL_set_nextUpdate(X509_CRL *x, const ASN1_TIME *tm);
 int X509_CRL_sort(X509_CRL *crl);
-void X509_CRL_up_ref(X509_CRL *crl);
+int X509_CRL_up_ref(X509_CRL *crl);
 
 long X509_CRL_get_version(X509_CRL *crl);
 ASN1_TIME *X509_CRL_get_lastUpdate(X509_CRL *crl);
diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h
index 1c86d3148c..ae93c7b33c 100644
--- a/include/openssl/x509_vfy.h
+++ b/include/openssl/x509_vfy.h
@@ -277,7 +277,7 @@ X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h,
                                              int type, X509_NAME *name);
 X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
                                         X509_OBJECT *x);
-void X509_OBJECT_up_ref_count(X509_OBJECT *a);
+int X509_OBJECT_up_ref_count(X509_OBJECT *a);
 void X509_OBJECT_free(X509_OBJECT *a);
 int X509_OBJECT_get_type(X509_OBJECT *a);
 X509 *X509_OBJECT_get0_X509(X509_OBJECT *a);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 9d189aa51c..e7eb3028b4 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -774,10 +774,16 @@ SSL *SSL_new(SSL_CTX *ctx)
     return NULL;
 }
 
-void SSL_up_ref(SSL *s)
+int SSL_up_ref(SSL *s)
 {
     int i;
-    CRYPTO_atomic_add(&s->references, 1, &i, s->lock);
+
+    if (CRYPTO_atomic_add(&s->references, 1, &i, s->lock) <= 0)
+        return 0;
+
+    REF_PRINT_COUNT("SSL", s);
+    REF_ASSERT_ISNT(i < 2);
+    return ((i > 1) ? 1 : 0);
 }
 
 int SSL_CTX_set_session_id_context(SSL_CTX *ctx, const unsigned char *sid_ctx,
@@ -2504,10 +2510,16 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *meth)
     return NULL;
 }
 
-void SSL_CTX_up_ref(SSL_CTX *ctx)
+int SSL_CTX_up_ref(SSL_CTX *ctx)
 {
     int i;
-    CRYPTO_atomic_add(&ctx->references, 1, &i, ctx->lock);
+
+    if (CRYPTO_atomic_add(&ctx->references, 1, &i, ctx->lock) <= 0)
+        return 0;
+
+    REF_PRINT_COUNT("SSL_CTX", ctx);
+    REF_ASSERT_ISNT(i < 2);
+    return ((i > 1) ? 1 : 0);
 }
 
 void SSL_CTX_free(SSL_CTX *a)