summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2022-10-14 13:40:50 -0500
committerEric Blake <eblake@redhat.com>2022-11-02 12:27:43 -0500
commit21d5c40cb74ca6aeceabc163cc4cf6e8da66e95e (patch)
tree73bdee86239965d148cc57ee0b3fee96faf7482a
parente454d10566fc4895531c7d41279a1678e9ad83db (diff)
downloadgnutls-21d5c40cb74ca6aeceabc163cc4cf6e8da66e95e.tar.gz
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>
-rw-r--r--doc/cha-gtls-app.texi16
-rw-r--r--lib/pkcs11_privkey.c5
-rw-r--r--lib/priority.c1
-rw-r--r--lib/privkey.c1
-rw-r--r--lib/pubkey.c1
-rw-r--r--lib/x509/crl.c1
-rw-r--r--lib/x509/crq.c3
-rw-r--r--lib/x509/ocsp.c2
-rw-r--r--lib/x509/privkey.c1
-rw-r--r--lib/x509/spki.c1
-rw-r--r--lib/x509/verify-high.c1
-rw-r--r--lib/x509/x509.c1
12 files changed, 31 insertions, 3 deletions
diff --git a/doc/cha-gtls-app.texi b/doc/cha-gtls-app.texi
index e0e971aea0..3547544e75 100644
--- 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 673794ec81..513d68ee8f 100644
--- 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 cd4b11ede0..8ea86d719c 100644
--- 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 2ec87dd4c7..1303311e57 100644
--- 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 be1b045fa7..fd72b49674 100644
--- 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 56103e105b..d4fc7d93ac 100644
--- 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 26030220ab..162f16a638 100644
--- 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 81f3d7eb86..d233c63a3f 100644
--- 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 792a4134d7..674dc71dce 100644
--- 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 c87ff1b3b2..454f8cf3c5 100644
--- 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 0c46881398..5d29929e7d 100644
--- 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 50dcc8e650..0c03ec635b 100644
--- 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 =