diff options
author | Vsevolod Stakhov <vsevolod@highsecure.ru> | 2021-05-12 17:42:55 +0100 |
---|---|---|
committer | Vsevolod Stakhov <vsevolod@highsecure.ru> | 2021-05-12 17:42:55 +0100 |
commit | 6a183f820fd4db594a7a9730aa084a8216827fd4 (patch) | |
tree | 826e62bc2978199251a0eb833d81290ced232449 | |
parent | 9647a701abfadd92a2975566cd19610c5810aa64 (diff) | |
download | rspamd-6a183f820fd4db594a7a9730aa084a8216827fd4.tar.gz rspamd-6a183f820fd4db594a7a9730aa084a8216827fd4.zip |
[Feature] Add race condition protection against hs_helper restarts
-rw-r--r-- | src/hs_helper.c | 37 | ||||
-rw-r--r-- | src/libserver/re_cache.c | 7 |
2 files changed, 37 insertions, 7 deletions
diff --git a/src/hs_helper.c b/src/hs_helper.c index f4f00cf05..cba2ca230 100644 --- a/src/hs_helper.c +++ b/src/hs_helper.c @@ -123,6 +123,7 @@ rspamd_hs_helper_cleanup_dir (struct hs_helper_ctx *ctx, gboolean forced) gint rc; gchar *pattern; gboolean ret = TRUE; + pid_t our_pid = getpid (); if (stat (ctx->hs_dir, &st) == -1) { msg_err ("cannot stat path %s, %s", @@ -160,10 +161,38 @@ rspamd_hs_helper_cleanup_dir (struct hs_helper_ctx *ctx, gboolean forced) rspamd_snprintf (pattern, len, "%s%c%s", ctx->hs_dir, G_DIR_SEPARATOR, "*.hs.new"); if ((rc = glob (pattern, 0, NULL, &globbuf)) == 0) { for (i = 0; i < globbuf.gl_pathc; i++) { - if (unlink (globbuf.gl_pathv[i]) == -1) { - msg_err ("cannot unlink %s: %s", globbuf.gl_pathv[i], - strerror (errno)); - ret = FALSE; + /* Check if we have a pid in the filename */ + const gchar *end_num = globbuf.gl_pathv[i] + + strlen (globbuf.gl_pathv[i]) - (sizeof (".hs.new") - 1); + const gchar *p = end_num - 1; + pid_t foreign_pid = -1; + + while (p > globbuf.gl_pathv[i]) { + if (g_ascii_isdigit (*p)) { + p --; + } + else { + p ++; + break; + } + } + + gulong ul; + if (p < end_num && rspamd_strtoul (p, end_num - p, &ul)) { + foreign_pid = ul; + } + + /* + * Remove only files that was left by us or some non-existing process + * There could be another race condition but it would just leave + * extra files which is relatively innocent? + */ + if (foreign_pid == -1 || foreign_pid == our_pid || kill (foreign_pid, 0) == -1) { + if (unlink(globbuf.gl_pathv[i]) == -1) { + msg_err ("cannot unlink %s: %s", globbuf.gl_pathv[i], + strerror(errno)); + ret = FALSE; + } } } } diff --git a/src/libserver/re_cache.c b/src/libserver/re_cache.c index b3326bcff..1d2f6522f 100644 --- a/src/libserver/re_cache.c +++ b/src/libserver/re_cache.c @@ -1894,6 +1894,7 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents ) struct iovec iov[7]; struct rspamd_re_cache *cache; GError *err; + pid_t our_pid = getpid (); cache = cbdata->cache; @@ -1946,8 +1947,8 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents ) return; } - rspamd_snprintf (path, sizeof (path), "%s%c%s.hs.new", cbdata->cache_dir, - G_DIR_SEPARATOR, re_class->hash); + rspamd_snprintf (path, sizeof (path), "%s%c%s.%P.hs.new", cbdata->cache_dir, + G_DIR_SEPARATOR, re_class->hash, our_pid); fd = open (path, O_CREAT|O_TRUNC|O_EXCL|O_WRONLY, 00600); if (fd == -1) { @@ -2185,7 +2186,7 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents ) g_free (hs_flags); /* Now rename temporary file to the new .hs file */ - rspamd_snprintf (npath, sizeof (path), "%s%c%s.hs", cbdata->cache_dir, + rspamd_snprintf (npath, sizeof (npath), "%s%c%s.hs", cbdata->cache_dir, G_DIR_SEPARATOR, re_class->hash); if (rename (path, npath) == -1) { |