Switch to a safer FREE_AND_NULL implementation

This one only evaluates the input once, so it cannot mess up even if
there are side effects.

10 files changed, 34 insertions, 14 deletions

diff --git a/src/common/address.h b/src/common/address.h
index 84d136d671..6f59e1c962 100644
--- a/src/common/address.h
+++ b/src/common/address.h
@@ -208,7 +208,7 @@ MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family,
 tor_addr_t *addr));
 void interface_address6_list_free_(smartlist_t * addrs);// XXXX
 #define interface_address6_list_free(addrs) \
-  FREE_AND_NULL(interface_address6_list, (addrs))
+  FREE_AND_NULL_UNMATCHED(smartlist_t, interface_address6_list_free_, (addrs))
 MOCK_DECL(smartlist_t *,get_interface_address6_list,(int severity,
                                                      sa_family_t family,
                                                      int include_internal));
diff --git a/src/common/aes.h b/src/common/aes.h
index 4874f728d4..c2720d29b8 100644
--- a/src/common/aes.h
+++ b/src/common/aes.h
@@ -18,7 +18,8 @@ typedef struct aes_cnt_cipher aes_cnt_cipher_t;
 aes_cnt_cipher_t* aes_new_cipher(const uint8_t *key, const uint8_t *iv,
                                  int key_bits);
 void aes_cipher_free_(aes_cnt_cipher_t *cipher);
-#define aes_cipher_free(cipher) FREE_AND_NULL(aes_cipher, (cipher))
+#define aes_cipher_free(cipher) \
+  FREE_AND_NULL_UNMATCHED(aes_cnt_cipher_t, aes_cipher_free_, (cipher))
 void aes_crypt_inplace(aes_cnt_cipher_t *cipher, char *data, size_t len);
 
 int evaluate_evp_for_aes(int force_value);
diff --git a/src/common/compat_libevent.h b/src/common/compat_libevent.h
index 65807315c9..2c86194d04 100644
--- a/src/common/compat_libevent.h
+++ b/src/common/compat_libevent.h
@@ -20,7 +20,8 @@ void suppress_libevent_log_msg(const char *msg);
   (sock),(tcp),(cb),(data));
 
 void tor_event_free_(struct event *ev);
-#define tor_event_free(ev) FREE_AND_NULL(tor_event, (ev))
+#define tor_event_free(ev) \
+  FREE_AND_NULL_UNMATCHED(struct event, tor_event_free_, (ev))
 
 typedef struct periodic_timer_t periodic_timer_t;
 
diff --git a/src/common/compress.h b/src/common/compress.h
index eee4a315db..7420f169fd 100644
--- a/src/common/compress.h
+++ b/src/common/compress.h
@@ -81,7 +81,8 @@ tor_compress_output_t tor_compress_process(tor_compress_state_t *state,
                                            const char **in, size_t *in_len,
                                            int finish);
 void tor_compress_free_(tor_compress_state_t *state);
-#define tor_compress_free(st) FREE_AND_NULL(tor_compress, (st))
+#define tor_compress_free(st) \
+  FREE_AND_NULL_UNMATCHED(tor_compress_state_t, tor_compress_free_, (st))
 
 size_t tor_compress_state_size(const tor_compress_state_t *state);
 
diff --git a/src/common/compress_lzma.h b/src/common/compress_lzma.h
index 1e92fd660c..986bfe9a67 100644
--- a/src/common/compress_lzma.h
+++ b/src/common/compress_lzma.h
@@ -32,7 +32,9 @@ tor_lzma_compress_process(tor_lzma_compress_state_t *state,
                           int finish);
 
 void tor_lzma_compress_free_(tor_lzma_compress_state_t *state);
-#define tor_lzma_compress_free(st) FREE_AND_NULL(tor_lzma_compress, (st))
+#define tor_lzma_compress_free(st)                      \
+  FREE_AND_NULL_UNMATCHED(tor_lzma_compress_state_t,   \
+                           tor_lzma_compress_free_, (st))
 
 size_t tor_lzma_compress_state_size(const tor_lzma_compress_state_t *state);
 
diff --git a/src/common/compress_zlib.h b/src/common/compress_zlib.h
index 3377e0e8de..701e2f9e89 100644
--- a/src/common/compress_zlib.h
+++ b/src/common/compress_zlib.h
@@ -32,7 +32,9 @@ tor_zlib_compress_process(tor_zlib_compress_state_t *state,
                           int finish);
 
 void tor_zlib_compress_free_(tor_zlib_compress_state_t *state);
-#define tor_zlib_compress_free(st) FREE_AND_NULL(tor_zlib_compress, (st))
+#define tor_zlib_compress_free(st)                      \
+  FREE_AND_NULL_UNMATCHED(tor_zlib_compress_state_t,   \
+                           tor_zlib_compress_free_, (st))
 
 size_t tor_zlib_compress_state_size(const tor_zlib_compress_state_t *state);
 
diff --git a/src/common/compress_zstd.h b/src/common/compress_zstd.h
index 9f67a9c588..3e18febb12 100644
--- a/src/common/compress_zstd.h
+++ b/src/common/compress_zstd.h
@@ -32,7 +32,9 @@ tor_zstd_compress_process(tor_zstd_compress_state_t *state,
                           int finish);
 
 void tor_zstd_compress_free_(tor_zstd_compress_state_t *state);
-#define tor_zstd_compress_free(st) FREE_AND_NULL(tor_zstd_compress, (st))
+#define tor_zstd_compress_free(st)                      \
+  FREE_AND_NULL_UNMATCHED(tor_zstd_compress_state_t,   \
+                           tor_zstd_compress_free_, (st))
 
 size_t tor_zstd_compress_state_size(const tor_zstd_compress_state_t *state);
 
diff --git a/src/common/log.c b/src/common/log.c
index 83098b1011..d305c9ae5e 100644
--- a/src/common/log.c
+++ b/src/common/log.c
@@ -65,7 +65,7 @@ typedef struct logfile_t {
 
 static void log_free_(logfile_t *victim);
 #define log_free(lg)    \
-  FREE_AND_NULL(log, (lg))
+  FREE_AND_NULL_UNMATCHED(logfile_t, log_free_, (lg))
 
 /** Helper: map a log severity to descriptive string. */
 static inline const char *
diff --git a/src/common/timers.h b/src/common/timers.h
index 6c9594d31b..86e8920169 100644
--- a/src/common/timers.h
+++ b/src/common/timers.h
@@ -18,7 +18,7 @@ void timer_get_cb(const tor_timer_t *t,
 void timer_schedule(tor_timer_t *t, const struct timeval *delay);
 void timer_disable(tor_timer_t *t);
 void timer_free_(tor_timer_t *t);
-#define timer_free(t) FREE_AND_NULL(timer, (t))
+#define timer_free(t) FREE_AND_NULL_UNMATCHED(tor_timer_t, timer_free_, (t))
 
 void timers_initialize(void);
 void timers_shutdown(void);
diff --git a/src/common/util.h b/src/common/util.h
index c5bd3f0bda..9ed11260dc 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -109,13 +109,24 @@ extern int dmalloc_free(const char *file, const int line, void *pnt,
 
 void tor_log_mallinfo(int severity);
 
+/* Helper macro: free a variable of type 'typename' using freefn, and
+ * set the variable to NULL.
+ *
+ * We use this for legacy cases when freefn and typename don't line up
+ * perfectly.
+ */
+#define FREE_AND_NULL_UNMATCHED(typename, freefn, var)                  \
+  do {                                                                  \
+    /* only evaluate (var) once. */                                     \
+    typename **tmp__free__ptr ## freefn = &(var);                       \
+    freefn(*tmp__free__ptr ## freefn);                                  \
+    (*tmp__free__ptr ## freefn) = NULL;                                 \
+  } while (0)
+
 /* Helper macro: free a variable of type 'type' using type_free_, and
  * set the variable to NULL. */
-#define FREE_AND_NULL(type, var)                                \
-  do {                                                          \
-      type ## _free_(var);                                      \
-      (var) = NULL;                                             \
-  } while (0)
+#define FREE_AND_NULL(type, var)                                        \
+  FREE_AND_NULL_UNMATCHED(type ## _t, type ## _free_, (var))
 
 /** Macro: yield a pointer to the field at position <b>off</b> within the
  * structure <b>st</b>.  Example: