diff options
author | Eric Blake <eblake@redhat.com> | 2022-10-14 13:40:50 -0500 |
---|---|---|
committer | Eric Blake <eblake@redhat.com> | 2022-11-02 12:27:43 -0500 |
commit | 21d5c40cb74ca6aeceabc163cc4cf6e8da66e95e (patch) | |
tree | 73bdee86239965d148cc57ee0b3fee96faf7482a | |
parent | e454d10566fc4895531c7d41279a1678e9ad83db (diff) | |
download | gnutls-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.texi | 16 | ||||
-rw-r--r-- | lib/pkcs11_privkey.c | 5 | ||||
-rw-r--r-- | lib/priority.c | 1 | ||||
-rw-r--r-- | lib/privkey.c | 1 | ||||
-rw-r--r-- | lib/pubkey.c | 1 | ||||
-rw-r--r-- | lib/x509/crl.c | 1 | ||||
-rw-r--r-- | lib/x509/crq.c | 3 | ||||
-rw-r--r-- | lib/x509/ocsp.c | 2 | ||||
-rw-r--r-- | lib/x509/privkey.c | 1 | ||||
-rw-r--r-- | lib/x509/spki.c | 1 | ||||
-rw-r--r-- | lib/x509/verify-high.c | 1 | ||||
-rw-r--r-- | lib/x509/x509.c | 1 |
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 = |