Deprecated {OPENSSL,CRYPTO}_debug_mem_{push,pop}
authorRich Salz <rsalz@akamai.com>
Wed, 10 Jul 2019 20:22:12 +0000 (16:22 -0400)
committerRichard Levitte <levitte@openssl.org>
Wed, 17 Jul 2019 12:48:06 +0000 (14:48 +0200)
They were only used for recursive ASN1 parsing.
Even if the internal memory-debugging facility remains,
this simplification seems worthwhile.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9342)

CHANGES
crypto/asn1/tasn_new.c
crypto/mem_dbg.c
doc/man3/OPENSSL_malloc.pod
include/internal/cryptlib.h
include/openssl/crypto.h
util/indent.pro
util/libcrypto.num
util/private.num

diff --git a/CHANGES b/CHANGES
index f6062af..6b9e7c4 100644 (file)
--- 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]
index 0612a04..f9b924c 100644 (file)
@@ -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;
 }
 
index 8fcdbec..3b1e37f 100644 (file)
@@ -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;
index f1de27a..38edf49 100644 (file)
@@ -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<OPENSSL_xxx> API. These are
@@ -155,15 +156,6 @@ the B<CRYPTO_MEM_CHECK_ON>.
 To disable tracking call CRYPTO_mem_ctrl() with a B<mode> argument of
 the B<CRYPTO_MEM_CHECK_OFF>.
 
-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<b> or FILE B<fp>. 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<This applies specially if OpenSSL was built with the
-configuration option> C<crypto-mdebug> I<enabled.  In case, swapping out
-only, say, the malloc() implementation is outright dangerous.>
+at once, especially if OpenSSL was built with the
+configuration option> C<crypto-mdebug>.
+
+=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
 
index 1aa1dc6..d54ca24 100644 (file)
@@ -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);
 
index 7953119..875ee55 100644 (file)
@@ -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);
 
 /*-
index 3d3f747..de260ae 100644 (file)
@@ -36,7 +36,6 @@
 -T ACCESS_DESCRIPTION
 -T ADDED_OBJ
 -T AES_KEY
--T APP_INFO
 -T ARGS
 -T ASIdOrRange
 -T ASIdOrRanges
 -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_
index d036249..648aed9 100644 (file)
@@ -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:
index fb5d0b2..f63319d 100644 (file)
@@ -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