aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVsevolod Stakhov <vsevolod@highsecure.ru>2021-05-12 17:42:55 +0100
committerVsevolod Stakhov <vsevolod@highsecure.ru>2021-05-12 17:42:55 +0100
commit6a183f820fd4db594a7a9730aa084a8216827fd4 (patch)
tree826e62bc2978199251a0eb833d81290ced232449
parent9647a701abfadd92a2975566cd19610c5810aa64 (diff)
downloadrspamd-6a183f820fd4db594a7a9730aa084a8216827fd4.tar.gz
rspamd-6a183f820fd4db594a7a9730aa084a8216827fd4.zip
[Feature] Add race condition protection against hs_helper restarts
-rw-r--r--src/hs_helper.c37
-rw-r--r--src/libserver/re_cache.c7
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) {