From e7aa7c11c71e84b2d6c06b1c81d130e8c1fba296 Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Wed, 10 Jul 2019 16:22:12 -0400 Subject: [PATCH] Deprecated {OPENSSL,CRYPTO}_debug_mem_{push,pop} They were only used for recursive ASN1 parsing. Even if the internal memory-debugging facility remains, this simplification seems worthwhile. Reviewed-by: Shane Lontis Reviewed-by: Matthias St. Pierre Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/9342) --- CHANGES | 5 +- crypto/asn1/tasn_new.c | 26 ------ crypto/mem_dbg.c | 169 ++---------------------------------- doc/man3/OPENSSL_malloc.pod | 40 ++++----- include/internal/cryptlib.h | 3 - include/openssl/crypto.h | 16 ++-- util/indent.pro | 2 - util/libcrypto.num | 4 +- util/private.num | 4 +- 9 files changed, 43 insertions(+), 226 deletions(-) diff --git a/CHANGES b/CHANGES index f6062af766..6b9e7c41fa 100644 --- a/CHANGES +++ b/CHANGES @@ -9,6 +9,10 @@ Changes between 1.1.1 and 3.0.0 [xx XXX xxxx] + *) {CRYPTO,OPENSSL}_mem_debug_{push,pop} are now no-ops and have been + deprecated. + [Rich Salz] + *) A new type, EVP_KEYEXCH, has been introduced to represent key exchange algorithms. An implementation of a key exchange algorithm can be obtained by using the function EVP_KEYEXCH_fetch(). An EVP_KEYEXCH algorithm can be @@ -22,7 +26,6 @@ *) Removed the function names from error messages and deprecated the xxx_F_xxx define's. - [Rich Salz] *) Removed NextStep support and the macro OPENSSL_UNISTD [Rich Salz] diff --git a/crypto/asn1/tasn_new.c b/crypto/asn1/tasn_new.c index 0612a04a1a..f9b924c190 100644 --- a/crypto/asn1/tasn_new.c +++ b/crypto/asn1/tasn_new.c @@ -52,10 +52,6 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) else asn1_cb = 0; -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_push(it->sname ? it->sname : "asn1_item_embed_new"); -#endif - switch (it->itype) { case ASN1_ITYPE_EXTERN: @@ -85,9 +81,6 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) if (!i) goto auxerr; if (i == 2) { -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_pop(); -#endif return 1; } } @@ -110,9 +103,6 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) if (!i) goto auxerr; if (i == 2) { -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_pop(); -#endif return 1; } } @@ -141,27 +131,18 @@ int asn1_item_embed_new(ASN1_VALUE **pval, const ASN1_ITEM *it, int embed) goto auxerr2; break; } -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_pop(); -#endif return 1; memerr2: asn1_item_embed_free(pval, it, embed); memerr: ASN1err(ASN1_F_ASN1_ITEM_EMBED_NEW, ERR_R_MALLOC_FAILURE); -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_pop(); -#endif return 0; auxerr2: asn1_item_embed_free(pval, it, embed); auxerr: ASN1err(ASN1_F_ASN1_ITEM_EMBED_NEW, ASN1_R_AUX_ERROR); -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_pop(); -#endif return 0; } @@ -219,10 +200,6 @@ static int asn1_template_new(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt) *pval = NULL; return 1; } -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_push(tt->field_name - ? tt->field_name : "asn1_template_new"); -#endif /* If SET OF or SEQUENCE OF, its a STACK */ if (tt->flags & ASN1_TFLG_SK_MASK) { STACK_OF(ASN1_VALUE) *skval; @@ -239,9 +216,6 @@ static int asn1_template_new(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt) /* Otherwise pass it back to the item routine */ ret = asn1_item_embed_new(pval, it, embed); done: -#ifndef OPENSSL_NO_CRYPTO_MDEBUG - OPENSSL_mem_debug_pop(); -#endif return ret; } diff --git a/crypto/mem_dbg.c b/crypto/mem_dbg.c index 8fcdbeca9e..3b1e37f301 100644 --- a/crypto/mem_dbg.c +++ b/crypto/mem_dbg.c @@ -39,26 +39,9 @@ static int mh_mode = CRYPTO_MEM_CHECK_OFF; #ifndef OPENSSL_NO_CRYPTO_MDEBUG static unsigned long order = 0; /* number of memory requests */ -/*- - * For application-defined information (static C-string `info') - * to be displayed in memory leak list. - * Each thread has its own stack. For applications, there is - * OPENSSL_mem_debug_push("...") to push an entry, - * OPENSSL_mem_debug_pop() to pop an entry, - */ -struct app_mem_info_st { - CRYPTO_THREAD_ID threadid; - const char *file; - int line; - const char *info; - struct app_mem_info_st *next; /* tail of thread's stack */ - int references; -}; - static CRYPTO_ONCE memdbg_init = CRYPTO_ONCE_STATIC_INIT; CRYPTO_RWLOCK *memdbg_lock; static CRYPTO_RWLOCK *long_memdbg_lock; -static CRYPTO_THREAD_LOCAL appinfokey; /* memory-block description */ struct mem_st { @@ -69,7 +52,6 @@ struct mem_st { CRYPTO_THREAD_ID threadid; unsigned long order; time_t time; - APP_INFO *app_info; #ifndef OPENSSL_NO_CRYPTO_MDEBUG_BACKTRACE void *array[30]; size_t array_siz; @@ -95,8 +77,7 @@ DEFINE_RUN_ONCE_STATIC(do_memdbg_init) { memdbg_lock = CRYPTO_THREAD_lock_new(); long_memdbg_lock = CRYPTO_THREAD_lock_new(); - if (memdbg_lock == NULL || long_memdbg_lock == NULL - || !CRYPTO_THREAD_init_local(&appinfokey, NULL)) { + if (memdbg_lock == NULL || long_memdbg_lock == NULL) { CRYPTO_THREAD_lock_free(memdbg_lock); memdbg_lock = NULL; CRYPTO_THREAD_lock_free(long_memdbg_lock); @@ -106,15 +87,6 @@ DEFINE_RUN_ONCE_STATIC(do_memdbg_init) return 1; } -static void app_info_free(APP_INFO *inf) -{ - if (inf == NULL) - return; - if (--(inf->references) <= 0) { - app_info_free(inf->next); - OPENSSL_free(inf); - } -} #endif int CRYPTO_mem_ctrl(int mode) @@ -237,77 +209,14 @@ static unsigned long mem_hash(const MEM *a) return ret; } -/* returns 1 if there was an info to pop, 0 if the stack was empty. */ -static int pop_info(void) -{ - APP_INFO *current = NULL; - - if (!RUN_ONCE(&memdbg_init, do_memdbg_init)) - return 0; - - current = (APP_INFO *)CRYPTO_THREAD_get_local(&appinfokey); - if (current != NULL) { - APP_INFO *next = current->next; - - if (next != NULL) { - next->references++; - CRYPTO_THREAD_set_local(&appinfokey, next); - } else { - CRYPTO_THREAD_set_local(&appinfokey, NULL); - } - if (--(current->references) <= 0) { - current->next = NULL; - if (next != NULL) - next->references--; - OPENSSL_free(current); - } - return 1; - } - return 0; -} - int CRYPTO_mem_debug_push(const char *info, const char *file, int line) { - APP_INFO *ami, *amim; - int ret = 0; - - if (mem_check_on()) { - CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE); - - if (!RUN_ONCE(&memdbg_init, do_memdbg_init) - || (ami = OPENSSL_malloc(sizeof(*ami))) == NULL) - goto err; - - ami->threadid = CRYPTO_THREAD_get_current_id(); - ami->file = file; - ami->line = line; - ami->info = info; - ami->references = 1; - ami->next = NULL; - - amim = (APP_INFO *)CRYPTO_THREAD_get_local(&appinfokey); - CRYPTO_THREAD_set_local(&appinfokey, ami); - - if (amim != NULL) - ami->next = amim; - ret = 1; - err: - CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); - } - - return ret; + return 0; } int CRYPTO_mem_debug_pop(void) { - int ret = 0; - - if (mem_check_on()) { - CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE); - ret = pop_info(); - CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); - } - return ret; + return 0; } static unsigned long break_order_num = 0; @@ -316,7 +225,6 @@ void CRYPTO_mem_debug_malloc(void *addr, size_t num, int before_p, const char *file, int line) { MEM *m, *mm; - APP_INFO *amim; switch (before_p & 127) { case 0: @@ -359,18 +267,8 @@ void CRYPTO_mem_debug_malloc(void *addr, size_t num, int before_p, # endif m->time = time(NULL); - amim = (APP_INFO *)CRYPTO_THREAD_get_local(&appinfokey); - m->app_info = amim; - if (amim != NULL) - amim->references++; - - if ((mm = lh_MEM_insert(mh, m)) != NULL) { - /* Not good, but don't sweat it */ - if (mm->app_info != NULL) { - mm->app_info->references--; - } + if ((mm = lh_MEM_insert(mh, m)) != NULL) OPENSSL_free(mm); - } err: CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); } @@ -391,14 +289,9 @@ void CRYPTO_mem_debug_free(void *addr, int before_p, if (mem_check_on() && (mh != NULL)) { CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE); - m.addr = addr; mp = lh_MEM_delete(mh, &m); - if (mp != NULL) { - app_info_free(mp->app_info); - OPENSSL_free(mp); - } - + OPENSSL_free(mp); CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); } break; @@ -456,11 +349,9 @@ static void print_leak(const MEM *m, MEM_LEAK *l) { char buf[1024]; char *bufp = buf, *hex; - size_t len = sizeof(buf), ami_cnt; - APP_INFO *amip; + size_t len = sizeof(buf); int n; struct tm *lcl = NULL; - CRYPTO_THREAD_ID ti; lcl = localtime(&m->time); n = BIO_snprintf(bufp, len, "[%02d:%02d:%02d] ", @@ -490,56 +381,9 @@ static void print_leak(const MEM *m, MEM_LEAK *l) len -= n; l->print_cb(buf, (size_t)(bufp - buf), l->print_cb_arg); - l->chunks++; l->bytes += m->num; - amip = m->app_info; - ami_cnt = 0; - - if (amip) { - ti = amip->threadid; - - do { - int buf_len; - int info_len; - - ami_cnt++; - if (ami_cnt >= sizeof(buf) - 1) - break; - memset(buf, '>', ami_cnt); - buf[ami_cnt] = '\0'; - hex = OPENSSL_buf2hexstr((const unsigned char *)&amip->threadid, - sizeof(amip->threadid)); - n = BIO_snprintf(buf + ami_cnt, sizeof(buf) - ami_cnt, - "thread=%s, file=%s, line=%d, info=\"", - hex, amip->file, amip->line); - OPENSSL_free(hex); - if (n <= 0) - break; - buf_len = ami_cnt + n; - info_len = strlen(amip->info); - if (128 - buf_len - 3 < info_len) { - memcpy(buf + buf_len, amip->info, 128 - buf_len - 3); - buf_len = 128 - 3; - } else { - n = BIO_snprintf(buf + buf_len, sizeof(buf) - buf_len, "%s", - amip->info); - if (n < 0) - break; - buf_len += n; - } - n = BIO_snprintf(buf + buf_len, sizeof(buf) - buf_len, "\"\n"); - if (n <= 0) - break; - - l->print_cb(buf, buf_len + n, l->print_cb_arg); - - amip = amip->next; - } - while (amip && CRYPTO_THREAD_compare_id(amip->threadid, ti)); - } - #ifndef OPENSSL_NO_CRYPTO_MDEBUG_BACKTRACE { size_t i; @@ -607,7 +451,6 @@ int CRYPTO_mem_leaks_cb(int (*cb) (const char *str, size_t len, void *u), CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_OFF); /* Clean up locks etc */ - CRYPTO_THREAD_cleanup_local(&appinfokey); CRYPTO_THREAD_lock_free(memdbg_lock); CRYPTO_THREAD_lock_free(long_memdbg_lock); memdbg_lock = NULL; diff --git a/doc/man3/OPENSSL_malloc.pod b/doc/man3/OPENSSL_malloc.pod index f1de27a19c..38edf49d4d 100644 --- a/doc/man3/OPENSSL_malloc.pod +++ b/doc/man3/OPENSSL_malloc.pod @@ -72,17 +72,18 @@ OPENSSL_MALLOC_FD int CRYPTO_mem_ctrl(int mode); - int OPENSSL_mem_debug_push(const char *info) - int OPENSSL_mem_debug_pop(void); - - int CRYPTO_mem_debug_push(const char *info, const char *file, int line); - int CRYPTO_mem_debug_pop(void); - int CRYPTO_mem_leaks(BIO *b); int CRYPTO_mem_leaks_fp(FILE *fp); int CRYPTO_mem_leaks_cb(int (*cb)(const char *str, size_t len, void *u), void *u); +Deprecated: + + int OPENSSL_mem_debug_push(const char *info) + int OPENSSL_mem_debug_pop(void); + int CRYPTO_mem_debug_push(const char *info, const char *file, int line); + int CRYPTO_mem_debug_pop(void); + =head1 DESCRIPTION OpenSSL memory allocation is handled by the B API. These are @@ -155,15 +156,6 @@ the B. To disable tracking call CRYPTO_mem_ctrl() with a B argument of the B. -While checking memory, it can be useful to store additional context -about what is being done. -For example, identifying the field names when parsing a complicated -data structure. -OPENSSL_mem_debug_push() (which calls CRYPTO_mem_debug_push()) -attachs an identifying string to the allocation stack. -This must be a global or other static string; it is not copied. -OPENSSL_mem_debug_pop() removes identifying state from the stack. - At the end of the program, calling CRYPTO_mem_leaks() or CRYPTO_mem_leaks_fp() will report all "leaked" memory, writing it to the specified BIO B or FILE B. These functions return 1 if @@ -207,6 +199,9 @@ to use this (will not work on all platforms): export OPENSSL_MALLOC_FD ...app invocation... 3>/tmp/log$$ +OPENSSL_mem_debug_push(), OPENSSL_mem_debug_pop(), +CRYPTO_mem_debug_push(), and CRYPTO_mem_debug_pop() +have been deprecated and replaced with functions that only return zero. =head1 RETURN VALUES @@ -232,16 +227,19 @@ always because allocations have already happened). CRYPTO_mem_ctrl() returns -1 if an error occurred, otherwise the previous value of the mode. -OPENSSL_mem_debug_push() and OPENSSL_mem_debug_pop() -return 1 on success or 0 on failure. - =head1 NOTES While it's permitted to swap out only a few and not all the functions with CRYPTO_set_mem_functions(), it's recommended to swap them all out -at once. I C I +at once, especially if OpenSSL was built with the +configuration option> C. + +=head1 HISTORY + +OPENSSL_mem_debug_push(), OPENSSL_mem_debug_pop(), +CRYPTO_mem_debug_push(), and CRYPTO_mem_debug_pop() +were deprecated in OpenSSL 3.0. + =head1 COPYRIGHT diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h index 1aa1dc60d7..d54ca24ee1 100644 --- a/include/internal/cryptlib.h +++ b/include/internal/cryptlib.h @@ -54,11 +54,8 @@ __owur static ossl_inline int ossl_assert_int(int expr, const char *exprstr, void *align_ptr typedef struct ex_callback_st EX_CALLBACK; - DEFINE_STACK_OF(EX_CALLBACK) -typedef struct app_mem_info_st APP_INFO; - typedef struct mem_st MEM; DEFINE_LHASH_OF(MEM); diff --git a/include/openssl/crypto.h b/include/openssl/crypto.h index 79531196ed..875ee556c1 100644 --- a/include/openssl/crypto.h +++ b/include/openssl/crypto.h @@ -310,12 +310,16 @@ size_t CRYPTO_secure_used(void); void OPENSSL_cleanse(void *ptr, size_t len); # ifndef OPENSSL_NO_CRYPTO_MDEBUG -# define OPENSSL_mem_debug_push(info) \ - CRYPTO_mem_debug_push(info, OPENSSL_FILE, OPENSSL_LINE) -# define OPENSSL_mem_debug_pop() \ - CRYPTO_mem_debug_pop() -int CRYPTO_mem_debug_push(const char *info, const char *file, int line); -int CRYPTO_mem_debug_pop(void); +# if !OPENSSL_API_3 +# define OPENSSL_mem_debug_push(info) \ + CRYPTO_mem_debug_push(info, OPENSSL_FILE, OPENSSL_LINE) +# define OPENSSL_mem_debug_pop() \ + CRYPTO_mem_debug_pop() +# endif +DEPRECATEDIN_3(int CRYPTO_mem_debug_push(const char *info, + const char *file, int line)) +DEPRECATEDIN_3(int CRYPTO_mem_debug_pop(void)) + void CRYPTO_get_alloc_counts(int *mcount, int *rcount, int *fcount); /*- diff --git a/util/indent.pro b/util/indent.pro index 3d3f747bf8..de260ae6b7 100644 --- a/util/indent.pro +++ b/util/indent.pro @@ -36,7 +36,6 @@ -T ACCESS_DESCRIPTION -T ADDED_OBJ -T AES_KEY --T APP_INFO -T ARGS -T ASIdOrRange -T ASIdOrRanges @@ -586,7 +585,6 @@ -T STACK_OF_nid_triple_ -T STACK_OF_void_ -T LHASH_OF_ADDED_OBJ_ --T LHASH_OF_APP_INFO_ -T LHASH_OF_CONF_VALUE_ -T LHASH_OF_ENGINE_PILE_ -T LHASH_OF_ERR_STATE_ diff --git a/util/libcrypto.num b/util/libcrypto.num index d0362490bc..648aed9d85 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -1157,7 +1157,7 @@ EVP_PKEY_CTX_set0_keygen_info 1183 3_0_0 EXIST::FUNCTION: ENGINE_unregister_digests 1184 3_0_0 EXIST::FUNCTION:ENGINE IPAddressOrRange_new 1185 3_0_0 EXIST::FUNCTION:RFC3779 EVP_aes_256_ofb 1186 3_0_0 EXIST::FUNCTION: -CRYPTO_mem_debug_push 1187 3_0_0 EXIST::FUNCTION:CRYPTO_MDEBUG +CRYPTO_mem_debug_push 1187 3_0_0 EXIST::FUNCTION:CRYPTO_MDEBUG,DEPRECATEDIN_3 X509_PKEY_new 1188 3_0_0 EXIST::FUNCTION: X509_get_key_usage 1189 3_0_0 EXIST::FUNCTION: X509_ATTRIBUTE_create_by_txt 1190 3_0_0 EXIST::FUNCTION: @@ -1848,7 +1848,7 @@ IDEA_cbc_encrypt 1890 3_0_0 EXIST::FUNCTION:IDEA BN_CTX_secure_new 1891 3_0_0 EXIST::FUNCTION: OCSP_ONEREQ_add_ext 1892 3_0_0 EXIST::FUNCTION:OCSP CMS_uncompress 1893 3_0_0 EXIST::FUNCTION:CMS -CRYPTO_mem_debug_pop 1895 3_0_0 EXIST::FUNCTION:CRYPTO_MDEBUG +CRYPTO_mem_debug_pop 1895 3_0_0 EXIST::FUNCTION:CRYPTO_MDEBUG,DEPRECATEDIN_3 EVP_aes_192_cfb128 1896 3_0_0 EXIST::FUNCTION: OCSP_REQ_CTX_nbio 1897 3_0_0 EXIST::FUNCTION:OCSP EVP_CIPHER_CTX_copy 1898 3_0_0 EXIST::FUNCTION: diff --git a/util/private.num b/util/private.num index fb5d0b2644..f63319dd96 100644 --- a/util/private.num +++ b/util/private.num @@ -311,8 +311,8 @@ OPENSSL_clear_realloc define OPENSSL_free define OPENSSL_malloc define OPENSSL_malloc_init define -OPENSSL_mem_debug_pop define -OPENSSL_mem_debug_push define +OPENSSL_mem_debug_pop define deprecated 3.0.0 +OPENSSL_mem_debug_push define deprecated 3.0.0 OPENSSL_memdup define OPENSSL_no_config define deprecated 1.1.0 OPENSSL_realloc define -- 2.34.1