Merge branch 'macro_free_v2_squashed'

Author: nmathewson

changes/bug24337

@@ -0,0 +1,8 @@
+  o Minor features (defensive programming):
+    - Most of the functions in Tor that free objects have been replaced
+      with macros that free the objects and set the corresponding pointers
+      to NULL. This change should help prevent a large class of dangling
+      pointer bugs. Closes ticket 24337.
+
+    - Where possible, the tor_free() macro now only evaluates its input once.
+      Part of ticket 24337.

configure.ac

@@ -1991,7 +1991,6 @@ if test "x$enable_gcc_warnings_advisory" != "xno"; then
      -Winvalid-source-encoding
      -Winvalid-token-paste
      -Wknr-promoted-parameter
-     -Wlanguage-extension-token
      -Wlarge-by-value-copy
      -Wliteral-conversion
      -Wliteral-range

doc/HACKING/CodingStandards.md

@@ -346,6 +346,46 @@ macro, as in:
 	if (BUG(ptr == NULL))
 		return -1;
 
+Allocator conventions
+---------------------
+
+By convention, any tor type with a name like `abc_t` should be allocated
+by a function named `abc_new()`.  This function should never return
+NULL.
+
+Also, a type named `abc_t` should be freed by a function named `abc_free_()`.
+Don't call this `abc_free_()` function directly -- instead, wrap it in a
+macro called `abc_free()`, using the `FREE_AND_NULL` macro:
+
+    void abc_free_(abc_t *obj);
+    #define abc_free(obj) FREE_AND_NULL(abc_t, abc_free_, (abc))
+
+This macro will free the underlying `abc_t` object, and will also set
+the object pointer to NULL.
+
+You should define all `abc_free_()` functions to accept NULL inputs:
+
+    void
+    abc_free_(abc_t *obj)
+    {
+      if (!obj)
+        return;
+      tor_free(obj->name);
+      thing_free(obj->thing);
+      tor_free(obj);
+    }
+
+If you need a free function that takes a `void *` argument (for example,
+to use it as a function callback), define it with a name like
+`abc_free_void()`:
+
+    static void
+    abc_free_void_(void *obj)
+    {
+      abc_free_(obj);
+    }
+
+
 Doxygen comment conventions
 ---------------------------
 

src/common/address.c

@@ -1759,14 +1759,14 @@ get_interface_address6,(int severity, sa_family_t family, tor_addr_t *addr))
       break;
   } SMARTLIST_FOREACH_END(a);
 
-  free_interface_address6_list(addrs);
+  interface_address6_list_free(addrs);
   return rv;
 }
 
 /** Free a smartlist of IP addresses returned by get_interface_address6_list.
  */
 void
-free_interface_address6_list(smartlist_t *addrs)
+interface_address6_list_free_(smartlist_t *addrs)
 {
   if (addrs != NULL) {
     SMARTLIST_FOREACH(addrs, tor_addr_t *, a, tor_free(a));
@@ -1781,7 +1781,7 @@ free_interface_address6_list(smartlist_t *addrs)
  * An empty smartlist means that there are no addresses of the selected type
  * matching these criteria.
  * Returns NULL on failure.
- * Use free_interface_address6_list to free the returned list.
+ * Use interface_address6_list_free to free the returned list.
  */
 MOCK_IMPL(smartlist_t *,
 get_interface_address6_list,(int severity,

src/common/address.h

@@ -206,7 +206,9 @@ const char * fmt_addr32(uint32_t addr);
 
 MOCK_DECL(int,get_interface_address6,(int severity, sa_family_t family,
 tor_addr_t *addr));
-void free_interface_address6_list(smartlist_t * addrs);
+void interface_address6_list_free_(smartlist_t * addrs);// XXXX
+#define interface_address6_list_free(addrs) \
+  FREE_AND_NULL(smartlist_t, interface_address6_list_free_, (addrs))
 MOCK_DECL(smartlist_t *,get_interface_address6_list,(int severity,
                                                      sa_family_t family,
                                                      int include_internal));
@@ -321,13 +323,8 @@ int addr_mask_get_bits(uint32_t mask);
 int tor_inet_ntoa(const struct in_addr *in, char *buf, size_t buf_len);
 char *tor_dup_ip(uint32_t addr) ATTR_MALLOC;
 MOCK_DECL(int,get_interface_address,(int severity, uint32_t *addr));
-/** Free a smartlist of IP addresses returned by get_interface_address_list.
- */
-static inline void
-free_interface_address_list(smartlist_t *addrs)
-{
-  free_interface_address6_list(addrs);
-}
+#define interface_address_list_free(lst)\
+  interface_address6_list_free(lst)
 /** Return a smartlist of the IPv4 addresses of all interfaces on the server.
  * Excludes loopback and multicast addresses. Only includes internal addresses
  * if include_internal is true. (Note that a relay behind NAT may use an

src/common/aes.c

@@ -110,7 +110,7 @@ aes_new_cipher(const uint8_t *key, const uint8_t *iv, int key_bits)
   return (aes_cnt_cipher_t *) cipher;
 }
 void
-aes_cipher_free(aes_cnt_cipher_t *cipher_)
+aes_cipher_free_(aes_cnt_cipher_t *cipher_)
 {
   if (!cipher_)
     return;
@@ -324,7 +324,7 @@ aes_set_key(aes_cnt_cipher_t *cipher, const uint8_t *key, int key_bits)
 /** Release storage held by <b>cipher</b>
  */
 void
-aes_cipher_free(aes_cnt_cipher_t *cipher)
+aes_cipher_free_(aes_cnt_cipher_t *cipher)
 {
   if (!cipher)
     return;

src/common/aes.h

@@ -17,7 +17,9 @@ 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);
+void aes_cipher_free_(aes_cnt_cipher_t *cipher);
+#define aes_cipher_free(cipher) \
+  FREE_AND_NULL(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);

src/common/buffers.c

@@ -409,7 +409,7 @@ buf_slack(const buf_t *buf)
 
 /** Release storage held by <b>buf</b>. */
 void
-buf_free(buf_t *buf)
+buf_free_(buf_t *buf)
 {
   if (!buf)
     return;

src/common/buffers.h

@@ -24,7 +24,8 @@ struct tor_compress_state_t;
 buf_t *buf_new(void);
 buf_t *buf_new_with_capacity(size_t size);
 size_t buf_get_default_chunk_size(const buf_t *buf);
-void buf_free(buf_t *buf);
+void buf_free_(buf_t *buf);
+#define buf_free(b) FREE_AND_NULL(buf_t, buf_free_, (b))
 void buf_clear(buf_t *buf);
 buf_t *buf_copy(const buf_t *buf);
 

src/common/compat.c

@@ -1905,9 +1905,12 @@ tor_passwd_dup(const struct passwd *pw)
   return new_pw;
 }
 
+#define tor_passwd_free(pw) \
+  FREE_AND_NULL(struct passwd, tor_passwd_free_, (pw))
+
 /** Helper: free one of our cached 'struct passwd' values. */
 static void
-tor_passwd_free(struct passwd *pw)
+tor_passwd_free_(struct passwd *pw)
 {
   if (!pw)
     return;