From: Vsevolod Stakhov Date: Tue, 19 Jun 2018 10:46:35 +0000 (+0100) Subject: [CritFix] Fix maps race conditions on reload X-Git-Tag: 1.7.7~85 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1bd9b92735543e0ad651a60f2b05c567e8139933;p=rspamd.git [CritFix] Fix maps race conditions on reload --- diff --git a/src/controller.c b/src/controller.c index 4c08b5e13..27e1bd6e7 100644 --- a/src/controller.c +++ b/src/controller.c @@ -3802,10 +3802,12 @@ start_controller_worker (struct rspamd_worker *worker) rspamd_worker_init_monitored (worker, ctx->ev_base, ctx->resolver); } - rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, TRUE); + rspamd_map_watch (worker->srv->cfg, ctx->ev_base, + ctx->resolver, worker, TRUE); } else { - rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, FALSE); + rspamd_map_watch (worker->srv->cfg, ctx->ev_base, + ctx->resolver, worker, FALSE); } rspamd_lua_run_postloads (ctx->cfg->lua_state, ctx->cfg, ctx->ev_base, worker); diff --git a/src/fuzzy_storage.c b/src/fuzzy_storage.c index 234822018..5499fa732 100644 --- a/src/fuzzy_storage.c +++ b/src/fuzzy_storage.c @@ -2903,7 +2903,7 @@ start_fuzzy (struct rspamd_worker *worker) ctx->resolver = dns_resolver_init (worker->srv->logger, ctx->ev_base, worker->srv->cfg); - rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, 0); + rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, worker, 0); /* Get peer pipe */ memset (&srv_cmd, 0, sizeof (srv_cmd)); diff --git a/src/libserver/worker_util.c b/src/libserver/worker_util.c index e706e95f4..774805544 100644 --- a/src/libserver/worker_util.c +++ b/src/libserver/worker_util.c @@ -386,9 +386,6 @@ rspamd_worker_stop_accept (struct rspamd_worker *worker) g_hash_table_unref (worker->signal_events); #endif - - /* Cleanup maps */ - rspamd_map_remove_all (worker->srv->cfg); } static rspamd_fstring_t * diff --git a/src/libutil/map.c b/src/libutil/map.c index 12e4f0a4e..203af03c7 100644 --- a/src/libutil/map.c +++ b/src/libutil/map.c @@ -1058,7 +1058,7 @@ rspamd_map_schedule_periodic (struct rspamd_map *map, gdouble timeout; struct map_periodic_cbdata *cbd; - if (map->scheduled_check) { + if (map->scheduled_check || (map->wrk && map->wrk->wanna_die)) { /* Do not schedule check if some check is already scheduled */ return; } @@ -1583,45 +1583,49 @@ rspamd_map_periodic_callback (gint fd, short what, void *ud) return; } - bk = g_ptr_array_index (cbd->map->backends, cbd->cur_backend); - g_assert (bk != NULL); - - if (cbd->need_modify) { - /* Load data from the next backend */ - switch (bk->protocol) { - case MAP_PROTO_HTTP: - case MAP_PROTO_HTTPS: - rspamd_map_http_read_callback (fd, what, cbd); - break; - case MAP_PROTO_FILE: - rspamd_map_file_read_callback (fd, what, cbd); - break; - case MAP_PROTO_STATIC: - rspamd_map_static_read_callback (fd, what, cbd); - break; - } - } - else { - /* Check the next backend */ - switch (bk->protocol) { - case MAP_PROTO_HTTP: - case MAP_PROTO_HTTPS: - rspamd_map_http_check_callback (fd, what, cbd); - break; - case MAP_PROTO_FILE: - rspamd_map_file_check_callback (fd, what, cbd); - break; - case MAP_PROTO_STATIC: - rspamd_map_static_check_callback (fd, what, cbd); - break; + if (!(cbd->map->wrk && cbd->map->wrk->wanna_die)) { + bk = g_ptr_array_index (cbd->map->backends, cbd->cur_backend); + g_assert (bk != NULL); + + if (cbd->need_modify) { + /* Load data from the next backend */ + switch (bk->protocol) { + case MAP_PROTO_HTTP: + case MAP_PROTO_HTTPS: + rspamd_map_http_read_callback (fd, what, cbd); + break; + case MAP_PROTO_FILE: + rspamd_map_file_read_callback (fd, what, cbd); + break; + case MAP_PROTO_STATIC: + rspamd_map_static_read_callback (fd, what, cbd); + break; + } + } else { + /* Check the next backend */ + switch (bk->protocol) { + case MAP_PROTO_HTTP: + case MAP_PROTO_HTTPS: + rspamd_map_http_check_callback (fd, what, cbd); + break; + case MAP_PROTO_FILE: + rspamd_map_file_check_callback (fd, what, cbd); + break; + case MAP_PROTO_STATIC: + rspamd_map_static_check_callback (fd, what, cbd); + break; + } } } } /* Start watching event for all maps */ void -rspamd_map_watch (struct rspamd_config *cfg, struct event_base *ev_base, - struct rspamd_dns_resolver *resolver, gboolean active_http) +rspamd_map_watch (struct rspamd_config *cfg, + struct event_base *ev_base, + struct rspamd_dns_resolver *resolver, + struct rspamd_worker *worker, + gboolean active_http) { GList *cur = cfg->maps; struct rspamd_map *map; @@ -1631,6 +1635,7 @@ rspamd_map_watch (struct rspamd_config *cfg, struct event_base *ev_base, map = cur->data; map->ev_base = ev_base; map->r = resolver; + map->wrk = worker; if (active_http) { map->active_http = active_http; @@ -1905,9 +1910,9 @@ rspamd_map_parse_backend (struct rspamd_config *cfg, const gchar *map_line) if (access (bk->uri, R_OK) == -1) { if (errno != ENOENT) { msg_err_config ("cannot open file '%s': %s", bk->uri, strerror (errno)); - return NULL; - + goto err; } + msg_info_config ( "map '%s' is not found, but it can be loaded automatically later", bk->uri); @@ -1928,7 +1933,7 @@ rspamd_map_parse_backend (struct rspamd_config *cfg, const gchar *map_line) else { if (!(up.field_set & 1 << UF_HOST)) { msg_err_config ("cannot parse HTTP url: %s: no host", bk->uri); - return NULL; + goto err; } tok.begin = bk->uri + up.field_data[UF_HOST].off; @@ -1956,7 +1961,8 @@ rspamd_map_parse_backend (struct rspamd_config *cfg, const gchar *map_line) } bk->data.hd = hdata; - }else if (bk->protocol == MAP_PROTO_STATIC) { + } + else if (bk->protocol == MAP_PROTO_STATIC) { sdata = g_malloc0 (sizeof (*sdata)); bk->data.sd = sdata; } diff --git a/src/libutil/map.h b/src/libutil/map.h index e12df4369..9c7485c58 100644 --- a/src/libutil/map.h +++ b/src/libutil/map.h @@ -78,8 +78,11 @@ struct rspamd_map* rspamd_map_add_from_ucl (struct rspamd_config *cfg, /** * Start watching of maps by adding events to libevent event loop */ -void rspamd_map_watch (struct rspamd_config *cfg, struct event_base *ev_base, - struct rspamd_dns_resolver *resolver, gboolean active_http); +void rspamd_map_watch (struct rspamd_config *cfg, + struct event_base *ev_base, + struct rspamd_dns_resolver *resolver, + struct rspamd_worker *worker, + gboolean active_http); /** * Remove all maps watched (remove events) diff --git a/src/libutil/map_private.h b/src/libutil/map_private.h index b8543356b..1b40e8e8a 100644 --- a/src/libutil/map_private.h +++ b/src/libutil/map_private.h @@ -120,6 +120,7 @@ struct rspamd_map { map_dtor_t dtor; void **user_data; struct event_base *ev_base; + struct rspamd_worker *wrk; gchar *description; gchar *name; guint32 id; diff --git a/src/plugins/surbl.c b/src/plugins/surbl.c index 238f14fb2..64b1b14f6 100644 --- a/src/plugins/surbl.c +++ b/src/plugins/surbl.c @@ -230,14 +230,18 @@ read_exceptions_list (gchar * chunk, if (data->cur_data == NULL) { t = data->prev_data; - for (i = 0; i < MAX_LEVELS; i++) { - if (t[i] != NULL) { - g_hash_table_destroy (t[i]); + if (t) { + for (i = 0; i < MAX_LEVELS; i++) { + if (t[i] != NULL) { + g_hash_table_destroy (t[i]); + } + t[i] = NULL; } - t[i] = NULL; + + g_free (t); } - data->cur_data = data->prev_data; + data->cur_data = g_malloc0 (MAX_LEVELS * sizeof (GHashTable *)); } return rspamd_parse_kv_list ( @@ -283,6 +287,8 @@ dtor_exceptions_list (struct map_cb_data *data) } t[i] = NULL; } + + g_free (t); } } @@ -397,15 +403,12 @@ surbl_module_init (struct rspamd_config *cfg, struct module_ctx **ctx) surbl_module_ctx->use_redirector = 0; surbl_module_ctx->suffixes = NULL; - surbl_module_ctx->surbl_pool = rspamd_mempool_new (rspamd_mempool_suggest_size (), NULL); + surbl_module_ctx->surbl_pool = rspamd_mempool_new (rspamd_mempool_suggest_size (), + NULL); surbl_module_ctx->redirectors = NULL; surbl_module_ctx->whitelist = NULL; - rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool, - (rspamd_mempool_destruct_t) rspamd_map_helper_destroy_hash, - surbl_module_ctx->whitelist); - surbl_module_ctx->exceptions = rspamd_mempool_alloc0 ( - surbl_module_ctx->surbl_pool, MAX_LEVELS * sizeof (GHashTable *)); + surbl_module_ctx->exceptions = NULL; surbl_module_ctx->redirector_cbid = -1; @@ -1081,16 +1084,7 @@ surbl_module_reconfig (struct rspamd_config *cfg) surbl_module_ctx->redirectors = NULL; surbl_module_ctx->whitelist = NULL; /* Zero exceptions hashes */ - surbl_module_ctx->exceptions = rspamd_mempool_alloc0 ( - surbl_module_ctx->surbl_pool, - MAX_LEVELS * sizeof (GHashTable *)); - /* Register destructors */ - rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool, - (rspamd_mempool_destruct_t) rspamd_map_helper_destroy_hash, - surbl_module_ctx->whitelist); - rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool, - (rspamd_mempool_destruct_t) g_hash_table_destroy, - surbl_module_ctx->redirector_tlds); + surbl_module_ctx->exceptions = NULL; rspamd_mempool_add_destructor (surbl_module_ctx->surbl_pool, (rspamd_mempool_destruct_t) g_list_free, @@ -1178,7 +1172,7 @@ format_surbl_request (rspamd_mempool_t * pool, /* Not a numeric url */ result = rspamd_mempool_alloc (pool, len); /* Now we should try to check for exceptions */ - if (!forced) { + if (!forced && surbl_module_ctx->exceptions) { for (i = MAX_LEVELS - 1; i >= 0; i--) { t = surbl_module_ctx->exceptions[i]; if (t != NULL && dots_num >= i + 1) { diff --git a/src/rspamd.c b/src/rspamd.c index b5454f38d..cba581419 100644 --- a/src/rspamd.c +++ b/src/rspamd.c @@ -970,7 +970,6 @@ rspamd_hup_handler (gint signo, short what, gpointer arg) RVERSION " is restarting"); g_hash_table_foreach (rspamd_main->workers, kill_old_workers, NULL); - rspamd_map_remove_all (rspamd_main->cfg); rspamd_log_close_priv (rspamd_main->logger, rspamd_main->workers_uid, rspamd_main->workers_gid); diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c index d94ab6455..055421a41 100644 --- a/src/rspamd_proxy.c +++ b/src/rspamd_proxy.c @@ -2165,7 +2165,7 @@ start_rspamd_proxy (struct rspamd_worker *worker) { ctx->ev_base, worker->srv->cfg); double_to_tv (ctx->timeout, &ctx->io_tv); - rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, 0); + rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, worker, 0); rspamd_upstreams_library_config (worker->srv->cfg, ctx->cfg->ups_ctx, ctx->ev_base, ctx->resolver->r); diff --git a/src/worker.c b/src/worker.c index 6b02b5753..f26a86ff7 100644 --- a/src/worker.c +++ b/src/worker.c @@ -676,7 +676,7 @@ start_worker (struct rspamd_worker *worker) ctx->resolver = dns_resolver_init (worker->srv->logger, ctx->ev_base, worker->srv->cfg); - rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, 0); + rspamd_map_watch (worker->srv->cfg, ctx->ev_base, ctx->resolver, worker, 0); rspamd_upstreams_library_config (worker->srv->cfg, ctx->cfg->ups_ctx, ctx->ev_base, ctx->resolver->r);