lib: Consistenly return sane results for all *_init()

After looking at gnutls_init(), I went and audited all other
*_init(gnutls_*_t) functions, to see if Bug #1414 applies in more
situations.  We had an inconsistent mix: some functions that went out
of their way to leave the parameter uninitialized on failure (such as
gnutls_x509_crt_init()); many that always left the parameter
initialized on failure (such as gnutls_x509_ext_ct_scts_init()), often
by relying on the gnutls_free() macro that assigns the pointer to NULL
after using the gnutls_free_function() callback pointer (such as
gnutls_pkcs11_obj_init()); but a few others that left stale pointers
on certain failures (such as gnutls_priority_init2()) or even which
used the wrong deallocation function (such as
gnutls_pkcs11_privkey_init()).

As with gnutls_init(), portable programs should either pre-initialize
memory to zero before calling _init() if they plan to unconditionally
call _deinit() (safe for all but gnutls_pkcs11_privkey_init()), or
they should avoid calling _deinit() if _init() failed.  But since we
can't force all existing clients to change, it is safest if we
unconditionally and consistently initialize the client's memory before
ALL failure paths.

Rather than try to adjust documentation of each *_init() function
(including those not needing a change), I instead generalized
documentation into the manual.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/doc/cha-gtls-app.texi b/doc/cha-gtls-app.texi
index e0e971aea0bfa22e1356d29d9475311c8e103287..3547544e75176ed129228ed0e081cc57f5327299 100644 (file)
--- a/doc/cha-gtls-app.texi
+++ b/doc/cha-gtls-app.texi
@@ -100,6 +100,22 @@ that future extensions of the API can be extended to provide
 additional information via positive returned values (see for example
 @funcref{gnutls_certificate_set_x509_key_file}).
 
+In @acronym{GnuTLS}, many objects are represented as opaque types that
+are initialized by passing an address to storage of that type to a
+pointer parameter of a function name @code{gnutls_@var{obj}_init}, and
+which have a counterpart function @code{gnutls_@var{obj}_deinit}.  It
+is safe, but not mandatory, to pre-initialize the opaque storage to
+contain all zeroes (such as by using @code{calloc()} or
+@code{memset()}).  If the initializer succeeds, the storage must be
+passed to the counterpart deinitializer when the object is no longer
+in use to avoid memory leaks.  As of version 3.8.0, if the initializer
+function fails, it is safe, but not mandatory, to call the counterpart
+deinitializer, regardless of whether the storage was pre-initialized.
+However, this was not guaranteed in earlier versions; for maximum
+portability to older library versions, callers should either
+pre-initialize the storage to zero before initialization or refrain
+from calling the deinitializer if the initializer fails.
+
 For certain operations such as TLS handshake and TLS packet receive
 there is the notion of fatal and non-fatal error codes.
 Fatal errors terminate the TLS session immediately and further sends

diff --git a/lib/pkcs11_privkey.c b/lib/pkcs11_privkey.c
index 673794ec81089a9fd82206b2c32ed32e239bc5a8..513d68ee8f5f459b0a08cc18d9e7aa782718a20e 100644 (file)
--- a/lib/pkcs11_privkey.c
+++ b/lib/pkcs11_privkey.c
@@ -78,6 +78,7 @@
 int gnutls_pkcs11_privkey_init(gnutls_pkcs11_privkey_t * key)
 {
        int ret;
+       *key = NULL;
        FAIL_IF_LIB_ERROR;
 
        *key = gnutls_calloc(1, sizeof(struct gnutls_pkcs11_privkey_st));
@@ -88,7 +89,7 @@ int gnutls_pkcs11_privkey_init(gnutls_pkcs11_privkey_t * key)
 
        (*key)->uinfo = p11_kit_uri_new();
        if ((*key)->uinfo == NULL) {
-               free(*key);
+               gnutls_free(*key);
                gnutls_assert();
                return GNUTLS_E_MEMORY_ERROR;
        }
@@ -97,7 +98,7 @@ int gnutls_pkcs11_privkey_init(gnutls_pkcs11_privkey_t * key)
        if (ret < 0) {
                gnutls_assert();
                p11_kit_uri_free((*key)->uinfo);
-               free(*key);
+               gnutls_free(*key);
                return GNUTLS_E_LOCKING_ERROR;
        }
 

diff --git a/lib/priority.c b/lib/priority.c
index cd4b11ede01df7a02618121ee1e51146b5baea8c..8ea86d719c41fe4bfd767b9348e933466a6c9aaf 100644 (file)
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -2921,6 +2921,7 @@ gnutls_priority_init2(gnutls_priority_t * priority_cache,
        const char *ep;
        int ret;
 
+       *priority_cache = NULL;
        if (flags & GNUTLS_PRIORITY_INIT_DEF_APPEND) {
                if (priorities == NULL)
                        return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);

diff --git a/lib/privkey.c b/lib/privkey.c
index 2ec87dd4c74c2fa99e5695e68ced34ffb5e1dc50..1303311e5744f4124c9564c95fc1ce3be876120c 100644 (file)
--- a/lib/privkey.c
+++ b/lib/privkey.c
@@ -420,6 +420,7 @@ _gnutls_privkey_update_spki_params(gnutls_privkey_t key,
  **/
 int gnutls_privkey_init(gnutls_privkey_t * key)
 {
+       *key = NULL;
        FAIL_IF_LIB_ERROR;
 
        *key = gnutls_calloc(1, sizeof(struct gnutls_privkey_st));

diff --git a/lib/pubkey.c b/lib/pubkey.c
index be1b045fa72107805c5efb488513721e885dbdc8..fd72b4967425d1fb03c7ad2b1860d2850e6ddaba 100644 (file)
--- a/lib/pubkey.c
+++ b/lib/pubkey.c
@@ -128,6 +128,7 @@ int gnutls_pubkey_get_key_usage(gnutls_pubkey_t key, unsigned int *usage)
  **/
 int gnutls_pubkey_init(gnutls_pubkey_t * key)
 {
+       *key = NULL;
        FAIL_IF_LIB_ERROR;
 
        *key = gnutls_calloc(1, sizeof(struct gnutls_pubkey_st));

diff --git a/lib/x509/crl.c b/lib/x509/crl.c
index 56103e105b613f9eea8607f6c76e8ab164b05d9e..d4fc7d93acfdde0a30a330dc8c8a210697e059a0 100644 (file)
--- a/lib/x509/crl.c
+++ b/lib/x509/crl.c
@@ -68,6 +68,7 @@ int result;
  **/
 int gnutls_x509_crl_init(gnutls_x509_crl_t * crl)
 {
+       *crl = NULL;
        FAIL_IF_LIB_ERROR;
 
        *crl = gnutls_calloc(1, sizeof(gnutls_x509_crl_int));

diff --git a/lib/x509/crq.c b/lib/x509/crq.c
index 26030220ab08aee5fb19b82e8e0e33fd58e6d700..162f16a638b94645932a7448bbfa3542525a5e8e 100644 (file)
--- a/lib/x509/crq.c
+++ b/lib/x509/crq.c
@@ -53,7 +53,8 @@
 int gnutls_x509_crq_init(gnutls_x509_crq_t * crq)
 {
        int result;
-       
+
+       *crq = NULL;
        FAIL_IF_LIB_ERROR;
 
        *crq = gnutls_calloc(1, sizeof(gnutls_x509_crq_int));

diff --git a/lib/x509/ocsp.c b/lib/x509/ocsp.c
index 81f3d7eb86d4ccbb0565ec70fbd7fcb554b9dc7e..d233c63a3faab967a0957153aa73ac234dbd0bef 100644 (file)
--- a/lib/x509/ocsp.c
+++ b/lib/x509/ocsp.c
@@ -70,6 +70,7 @@ int gnutls_ocsp_req_init(gnutls_ocsp_req_t * req)
            gnutls_calloc(1, sizeof(gnutls_ocsp_req_int));
        int ret;
 
+       *req = NULL;
        if (!tmp)
                return GNUTLS_E_MEMORY_ERROR;
 
@@ -119,6 +120,7 @@ int gnutls_ocsp_resp_init(gnutls_ocsp_resp_t * resp)
            gnutls_calloc(1, sizeof(gnutls_ocsp_resp_int));
        int ret;
 
+       *resp = NULL;
        if (!tmp)
                return GNUTLS_E_MEMORY_ERROR;
 

diff --git a/lib/x509/privkey.c b/lib/x509/privkey.c
index 792a4134d72f425ffa26b90ad9902201dc666244..674dc71dce99f2ba15650fdca82d6f7b2a94f23f 100644 (file)
--- a/lib/x509/privkey.c
+++ b/lib/x509/privkey.c
@@ -47,6 +47,7 @@
  **/
 int gnutls_x509_privkey_init(gnutls_x509_privkey_t * key)
 {
+       *key = NULL;
        FAIL_IF_LIB_ERROR;
 
        *key = gnutls_calloc(1, sizeof(gnutls_x509_privkey_int));

diff --git a/lib/x509/spki.c b/lib/x509/spki.c
index c87ff1b3b2f27f8d837b971843cff7490d95184c..454f8cf3c559896cb162d2c460ef325b33100163 100644 (file)
--- a/lib/x509/spki.c
+++ b/lib/x509/spki.c
@@ -45,6 +45,7 @@ gnutls_x509_spki_init(gnutls_x509_spki_t *spki)
 {
        gnutls_x509_spki_t tmp;
 
+       *spki = NULL;
        FAIL_IF_LIB_ERROR;
 
        tmp =

diff --git a/lib/x509/verify-high.c b/lib/x509/verify-high.c
index 0c46881398007d8fb40318c3a29f7dc941a8aa07..5d29929e7d5c6277aed20c7eab312f7df7d5b6d8 100644 (file)
--- a/lib/x509/verify-high.c
+++ b/lib/x509/verify-high.c
@@ -103,6 +103,7 @@ gnutls_x509_trust_list_init(gnutls_x509_trust_list_t * list,
 {
        gnutls_x509_trust_list_t tmp;
 
+       *list = NULL;
        FAIL_IF_LIB_ERROR;
 
        tmp =

diff --git a/lib/x509/x509.c b/lib/x509/x509.c
index 50dcc8e6508688a52f761b6c8c66e36920028425..0c03ec635b2b68a31d0833c257e89a5937e95460 100644 (file)
--- a/lib/x509/x509.c
+++ b/lib/x509/x509.c
@@ -201,6 +201,7 @@ int gnutls_x509_crt_init(gnutls_x509_crt_t * cert)
        gnutls_x509_crt_t tmp;
        int result;
 
+       *cert = NULL;
        FAIL_IF_LIB_ERROR;
 
        tmp =