[Fix] Switch to refcounts to avoid more races

Issue: #550
Reported by: @moisseev

diff --git a/src/libutil/map.c b/src/libutil/map.c
index d396e642a99260bd1c36d3a1cc9a124c80767123..3d965c5a5f907242660979f22fd83266742ec002 100644 (file)
--- a/src/libutil/map.c
+++ b/src/libutil/map.c
@@ -68,10 +68,12 @@ write_http_request (struct http_callback_data *cbd)
 
                rspamd_http_connection_write_message (cbd->conn, msg, cbd->data->host,
                                NULL, cbd, cbd->fd, &cbd->tv, cbd->ev_base);
+               REF_RETAIN (cbd);
        }
        else {
                msg_err_pool ("cannot connect to %s: %s", cbd->data->host,
                                strerror (errno));
+               REF_RELEASE (cbd);
        }
 }
 
@@ -240,7 +242,7 @@ http_map_error (struct rspamd_http_connection *conn,
 
        msg_err_pool ("connection with http server terminated incorrectly: %s",
                        err->message);
-       free_http_cbdata (cbd);
+       REF_RELEASE (cbd);
 }
 
 static int
@@ -290,15 +292,13 @@ http_map_finish (struct rspamd_http_connection *conn,
                                if (cbd->out_fd == -1) {
                                        msg_err_pool ("cannot open pubkey file %s for writing: %s",
                                                        fpath, strerror (errno));
-                                       free_http_cbdata (cbd);
-
-                                       return 0;
+                                       goto end;
                                }
 
                                rspamd_http_connection_reset (cbd->conn);
                                write_http_request (cbd);
 
-                               return 0;
+                               goto end;
                        }
                        else {
                                /* Unsinged version - just open file */
@@ -307,9 +307,7 @@ http_map_finish (struct rspamd_http_connection *conn,
                                if (in == NULL) {
                                        msg_err_pool ("cannot read tempfile %s: %s", cbd->tmpfile,
                                                        strerror (errno));
-                                       free_http_cbdata (cbd);
-
-                                       return 0;
+                                       goto end;
                                }
                        }
                }
@@ -320,9 +318,7 @@ http_map_finish (struct rspamd_http_connection *conn,
                        if (fstat (cbd->out_fd, &st) == -1) {
                                msg_err_pool ("cannot stat pubkey file %s: %s",
                                                fpath, strerror (errno));
-                               free_http_cbdata (cbd);
-
-                               return 0;
+                               goto end;
                        }
 
                        aux_data = mmap (NULL, st.st_size, PROT_READ, MAP_SHARED,
@@ -333,9 +329,7 @@ http_map_finish (struct rspamd_http_connection *conn,
                        if (aux_data == MAP_FAILED) {
                                msg_err_pool ("cannot map pubkey file %s: %s",
                                                fpath, strerror (errno));
-                               free_http_cbdata (cbd);
-
-                               return 0;
+                               goto end;
                        }
 
                        cbd->pk = rspamd_pubkey_from_base32 (aux_data, st.st_size,
@@ -345,9 +339,7 @@ http_map_finish (struct rspamd_http_connection *conn,
                        if (cbd->pk == NULL) {
                                msg_err_pool ("cannot load pubkey file %s: bad pubkey",
                                                fpath);
-                               free_http_cbdata (cbd);
-
-                               return 0;
+                               goto end;
                        }
 
                        rspamd_snprintf (fpath, sizeof (fpath), "%s.sig", cbd->tmpfile);
@@ -356,16 +348,14 @@ http_map_finish (struct rspamd_http_connection *conn,
                        if (cbd->out_fd == -1) {
                                msg_err_pool ("cannot open signature file %s for writing: %s",
                                                fpath, strerror (errno));
-                               free_http_cbdata (cbd);
-
-                               return 0;
+                               goto end;
                        }
 
                        cbd->stage = map_load_signature;
                        rspamd_http_connection_reset (cbd->conn);
                        write_http_request (cbd);
 
-                       return 0;
+                       goto end;
                }
                else if (cbd->stage == map_load_signature) {
                        /* We can now check signature */
@@ -377,15 +367,11 @@ http_map_finish (struct rspamd_http_connection *conn,
                        if (in == NULL) {
                                msg_err_pool ("cannot read tempfile %s: %s", cbd->tmpfile,
                                                strerror (errno));
-                               free_http_cbdata (cbd);
-
-                               return 0;
+                               goto end;
                        }
 
                        if (!rspamd_map_check_sig_pk (cbd->tmpfile, map, in, inlen, cbd->pk)) {
-                               free_http_cbdata (cbd);
-
-                               return 0;
+                               goto end;
                        }
                }
 
@@ -413,7 +399,8 @@ http_map_finish (struct rspamd_http_connection *conn,
                                map->uri, cbd->data->host, msg->code);
        }
 
-       free_http_cbdata (cbd);
+end:
+       REF_RELEASE (cbd);
 
        return 0;
 }
@@ -436,7 +423,7 @@ http_map_read (struct rspamd_http_connection *conn,
 
        if (write (cbd->out_fd, chunk, len) == -1) {
                msg_err_pool ("cannot write to %s: %s", cbd->tmpfile, strerror (errno));
-               free_http_cbdata (cbd);
+               REF_RELEASE (cbd);
 
                return -1;
        }
@@ -618,9 +605,10 @@ rspamd_map_dns_callback (struct rdns_reply *reply, void *arg)
                else {
                        /* We could not resolve host, so cowardly fail here */
                        msg_err_pool ("cannot resolve %s", cbd->data->host);
-                       free_http_cbdata (cbd);
                }
        }
+
+       REF_RELEASE (cbd);
 }
 
 /**
@@ -665,23 +653,30 @@ http_callback (gint fd, short what, void *ud)
        cbd->cbdata.map = cbd->map;
        cbd->stage = map_resolve_host2;
        double_to_tv (map->cfg->map_timeout, &cbd->tv);
+       REF_INIT_RETAIN (cbd, free_http_cbdata);
 
        msg_debug_pool ("reading map data from %s", data->host);
        /* Send both A and AAAA requests */
        if (map->r->r) {
-               rdns_make_request_full (map->r->r, rspamd_map_dns_callback, cbd,
+               if (rdns_make_request_full (map->r->r, rspamd_map_dns_callback, cbd,
                                map->cfg->dns_timeout, map->cfg->dns_retransmits, 1,
-                               data->host, RDNS_REQUEST_A);
-               rdns_make_request_full (map->r->r, rspamd_map_dns_callback, cbd,
+                               data->host, RDNS_REQUEST_A)) {
+                       REF_RETAIN (cbd);
+               }
+               if (rdns_make_request_full (map->r->r, rspamd_map_dns_callback, cbd,
                                map->cfg->dns_timeout, map->cfg->dns_retransmits, 1,
-                               data->host, RDNS_REQUEST_AAAA);
+                               data->host, RDNS_REQUEST_AAAA)) {
+                       REF_RETAIN (cbd);
+               }
                jitter_timeout_event (map, FALSE, FALSE, FALSE);
        }
        else {
                msg_warn_pool ("cannot load map: DNS resolver is not initialized");
-               free_http_cbdata (cbd);
                jitter_timeout_event (map, FALSE, FALSE, TRUE);
        }
+
+       /* We don't need own ref as it is now refcounted by DNS requests */
+       REF_RELEASE (cbd);
 }
 
 /* Start watching event for all maps */

diff --git a/src/libutil/map_private.h b/src/libutil/map_private.h
index 10f94c3ea702ed8c3c62dc8c0d3900198dff453a..85e468e4e2a1dc44b05591d3a9c76ff76b43e5cc 100644 (file)
--- a/src/libutil/map_private.h
+++ b/src/libutil/map_private.h
@@ -20,6 +20,7 @@
 #include "mem_pool.h"
 #include "keypair.h"
 #include "unix-std.h"
+#include "ref.h"
 
 enum fetch_proto {
        MAP_PROTO_FILE,
@@ -77,6 +78,7 @@ struct http_callback_data {
        struct http_map_data *data;
        struct map_cb_data cbdata;
        struct rspamd_cryptobox_pubkey *pk;
+       gchar *tmpfile;
 
        enum {
                map_resolve_host2 = 0, /* 2 requests sent */
@@ -86,8 +88,9 @@ struct http_callback_data {
                map_load_signature
        } stage;
        gint out_fd;
-       gchar *tmpfile;
        gint fd;
+
+       ref_entry_t ref;
 };
 
 #endif /* SRC_LIBUTIL_MAP_PRIVATE_H_ */