[Fix] Fix race condition with shared memory by refcounts

diff --git a/src/libutil/http.c b/src/libutil/http.c
index cedf394ae6ef4c00a12f8c5d7563593626d84508..ad47b1b20f65c9e7019c3692aa387849a65c01bc 100644 (file)
--- a/src/libutil/http.c
+++ b/src/libutil/http.c
@@ -1217,7 +1217,12 @@ rspamd_http_connection_copy_msg (struct rspamd_http_connection *conn)
                        }
 
                        /* We don't own segment, so do not try to touch it */
-                       storage->shared.shm_name = NULL;
+
+                       if (msg->body_buf.c.shared.name) {
+                               storage->shared.name = msg->body_buf.c.shared.name;
+                               REF_RETAIN (storage->shared.name);
+                       }
+
                        new_msg->body_buf.str = mmap (NULL, st.st_size,
                                        PROT_READ, MAP_SHARED,
                                        storage->shared.shm_fd, 0);
@@ -1481,10 +1486,11 @@ rspamd_http_detach_shared (struct rspamd_http_message *msg)
        rspamd_http_message_set_body_from_fstring_steal (msg, cpy_str);
 }
 
-void
-rspamd_http_connection_write_message (struct rspamd_http_connection *conn,
+static void
+rspamd_http_connection_write_message_common (struct rspamd_http_connection *conn,
                struct rspamd_http_message *msg, const gchar *host, const gchar *mime_type,
-               gpointer ud, gint fd, struct timeval *timeout, struct event_base *base)
+               gpointer ud, gint fd, struct timeval *timeout, struct event_base *base,
+               gboolean allow_shared)
 {
        struct rspamd_http_connection_private *priv = conn->priv;
        struct rspamd_http_header *hdr, *htmp;
@@ -1534,9 +1540,22 @@ rspamd_http_connection_write_message (struct rspamd_http_connection *conn,
        if (encrypted && (msg->flags &
                        (RSPAMD_HTTP_FLAG_SHMEM_IMMUTABLE|RSPAMD_HTTP_FLAG_SHMEM))) {
                /* We cannot use immutable body to encrypt message in place */
+               allow_shared = FALSE;
                rspamd_http_detach_shared (msg);
        }
 
+       if (allow_shared) {
+               if (!(msg->flags & RSPAMD_HTTP_FLAG_SHMEM) ||
+                               msg->body_buf.c.shared.name == NULL) {
+                       allow_shared = FALSE;
+               }
+               else {
+                       /* Insert new header */
+                       rspamd_http_message_add_header (msg, "Shm",
+                                       msg->body_buf.c.shared.name->shm_name);
+               }
+       }
+
        if (encrypted) {
                mode = rspamd_keypair_alg (priv->local_key);
 
@@ -1605,7 +1624,7 @@ rspamd_http_connection_write_message (struct rspamd_http_connection *conn,
        }
        else {
                if (msg->method < HTTP_SYMBOLS) {
-                       if (msg->body_buf.len == 0) {
+                       if (msg->body_buf.len == 0 || allow_shared) {
                                pbody = NULL;
                                bodylen = 0;
                                priv->outlen = 2;
@@ -1619,6 +1638,7 @@ rspamd_http_connection_write_message (struct rspamd_http_connection *conn,
                        }
                }
                else if (msg->body_buf.len > 0) {
+                       allow_shared = FALSE;
                        pbody = (gchar *)msg->body_buf.begin;
                        bodylen = msg->body_buf.len;
                        priv->outlen = 2;
@@ -1886,6 +1906,24 @@ rspamd_http_connection_write_message (struct rspamd_http_connection *conn,
        event_add (&priv->ev, priv->ptv);
 }
 
+void
+rspamd_http_connection_write_message (struct rspamd_http_connection *conn,
+               struct rspamd_http_message *msg, const gchar *host, const gchar *mime_type,
+               gpointer ud, gint fd, struct timeval *timeout, struct event_base *base)
+{
+       rspamd_http_connection_write_message_common (conn, msg, host, mime_type,
+                       ud, fd, timeout, base, FALSE);
+}
+
+void
+rspamd_http_connection_write_message_shared (struct rspamd_http_connection *conn,
+               struct rspamd_http_message *msg, const gchar *host, const gchar *mime_type,
+               gpointer ud, gint fd, struct timeval *timeout, struct event_base *base)
+{
+       rspamd_http_connection_write_message_common (conn, msg, host, mime_type,
+                       ud, fd, timeout, base, TRUE);
+}
+
 struct rspamd_http_message *
 rspamd_http_new_message (enum http_parser_type type)
 {
@@ -1974,6 +2012,37 @@ rspamd_http_message_get_body (struct rspamd_http_message *msg,
        return ret;
 }
 
+static void
+rspamd_http_shname_dtor (void *p)
+{
+       struct _rspamd_storage_shmem_s *n = p;
+
+       shm_unlink (n->shm_name);
+       g_free (n->shm_name);
+       g_slice_free1 (sizeof (*n), n);
+}
+
+void *
+rspamd_http_message_shmem_ref (struct rspamd_http_message *msg)
+{
+       if ((msg->flags & RSPAMD_HTTP_FLAG_SHMEM) && msg->body_buf.c.shared.name) {
+               REF_RETAIN (msg->body_buf.c.shared.name);
+               return msg->body_buf.c.shared.name;
+       }
+
+       return NULL;
+}
+
+void
+rspamd_http_message_shmem_unref (void *p)
+{
+       struct _rspamd_storage_shmem_s *n = p;
+
+       if (n) {
+               REF_RELEASE (n);
+       }
+}
+
 gboolean
 rspamd_http_message_set_body (struct rspamd_http_message *msg,
                const gchar *data, gsize len)
@@ -1984,8 +2053,10 @@ rspamd_http_message_set_body (struct rspamd_http_message *msg,
        rspamd_http_message_storage_cleanup (msg);
 
        if (msg->flags & RSPAMD_HTTP_FLAG_SHMEM) {
-               storage->shared.shm_name = g_strdup ("/rhm.XXXXXXXXXXXXXXXXXXXX");
-               storage->shared.shm_fd = rspamd_shmem_mkstemp (storage->shared.shm_name);
+               storage->shared.name = g_slice_alloc (sizeof (*storage->shared.name));
+               REF_INIT_RETAIN (storage->shared.name, rspamd_http_shname_dtor);
+               storage->shared.name->shm_name = g_strdup ("/rhm.XXXXXXXXXXXXXXXXXXXX");
+               storage->shared.shm_fd = rspamd_shmem_mkstemp (storage->shared.name->shm_name);
 
                if (storage->shared.shm_fd == -1) {
                        return FALSE;
@@ -2192,9 +2263,8 @@ rspamd_http_message_storage_cleanup (struct rspamd_http_message *msg)
                                close (storage->shared.shm_fd);
                        }
 
-                       if (storage->shared.shm_name != NULL) {
-                               shm_unlink (storage->shared.shm_name);
-                               g_free (storage->shared.shm_name);
+                       if (storage->shared.name != NULL) {
+                               REF_RELEASE (storage->shared.name);
                        }
 
                        storage->shared.shm_fd = -1;

diff --git a/src/libutil/http.h b/src/libutil/http.h
index 1e4ffbbc02cc11835474d502b88ba936ae577bca..4cbcaf5fb11eea2add05b2c2ee1af6f07071b5b6 100644 (file)
--- a/src/libutil/http.h
+++ b/src/libutil/http.h
@@ -196,6 +196,16 @@ void rspamd_http_connection_write_message (
                struct timeval *timeout,
                struct event_base *base);
 
+void rspamd_http_connection_write_message_shared (
+               struct rspamd_http_connection *conn,
+               struct rspamd_http_message *msg,
+               const gchar *host,
+               const gchar *mime_type,
+               gpointer ud,
+               gint fd,
+               struct timeval *timeout,
+               struct event_base *base);
+
 /**
  * Free connection structure
  * @param conn
@@ -352,6 +362,17 @@ gboolean rspamd_http_message_remove_header (struct rspamd_http_message *msg,
  */
 void rspamd_http_message_free (struct rspamd_http_message *msg);
 
+/**
+ * Increase refcount for shared file (if any) to prevent early memory unlinking
+ * @param msg
+ */
+void* rspamd_http_message_shmem_ref (struct rspamd_http_message *msg);
+/**
+ * Decrease external ref for shmem segment associated with a message
+ * @param msg
+ */
+void rspamd_http_message_shmem_unref (void *p);
+
 /**
  * Parse HTTP date header and return it as time_t
  * @param header HTTP date header

diff --git a/src/libutil/http_private.h b/src/libutil/http_private.h
index a944cb31b60a90c1e116cad726d09be1dff6171a..38fbec742fdf67559bc37a661c4e479174710e30 100644 (file)
--- a/src/libutil/http_private.h
+++ b/src/libutil/http_private.h
@@ -18,6 +18,7 @@
 
 #include "http.h"
 #include "str_util.h"
+#include "ref.h"
 #include "../../contrib/mumhash/mum.h"
 #define HASH_CASELESS
 #include "uthash_strcase.h"
@@ -52,8 +53,11 @@ struct rspamd_http_message {
                /* Internal storage */
                union _rspamd_storage_u {
                        rspamd_fstring_t *normal;
-                       struct {
-                               gchar *shm_name;
+                       struct _rspamd_storage_shared_s {
+                               struct _rspamd_storage_shmem_s {
+                                       gchar *shm_name;
+                                       ref_entry_t ref;
+                               } *name;
                                gint shm_fd;
                        } shared;
                } c;

diff --git a/src/rspamd_proxy.c b/src/rspamd_proxy.c
index 9d70306e9f6953ad9a105fb97858c60dea351630..6e0c1c2547b14c725ae11ac2959578b75cd3d7e0 100644 (file)
--- a/src/rspamd_proxy.c
+++ b/src/rspamd_proxy.c
@@ -142,9 +142,10 @@ struct rspamd_proxy_session {
        rspamd_inet_addr_t *client_addr;
        struct rspamd_http_connection *client_conn;
        gpointer map;
-       gsize map_len;
+       gpointer shmem_ref;
        struct rspamd_proxy_backend_connection *master_conn;
        GPtrArray *mirror_conns;
+       gsize map_len;
        gint client_sock;
        gboolean is_spamc;
        ref_entry_t ref;
@@ -823,6 +824,7 @@ proxy_session_dtor (struct rspamd_proxy_session *session)
        }
 
        g_ptr_array_free (session->mirror_conns, TRUE);
+       rspamd_http_message_shmem_unref (session->shmem_ref);
        rspamd_inet_address_destroy (session->client_addr);
        close (session->client_sock);
        rspamd_mempool_delete (session->pool);
@@ -1033,7 +1035,7 @@ proxy_open_mirror_connections (struct rspamd_proxy_session *session)
                                session->ctx->local_key);
                msg->peer_key = rspamd_pubkey_ref (m->key);
 
-               rspamd_http_connection_write_message (bk_conn->backend_conn,
+               rspamd_http_connection_write_message_shared (bk_conn->backend_conn,
                                msg, NULL, NULL, bk_conn,
                                bk_conn->backend_sock,
                                &session->ctx->io_tv, session->ctx->ev_base);
@@ -1200,6 +1202,7 @@ proxy_client_finish_handler (struct rspamd_http_connection *conn,
                        rspamd_http_message_remove_header (msg, "Content-Length");
                        rspamd_http_message_remove_header (msg, "Key");
                        rspamd_http_connection_reset (session->client_conn);
+                       session->shmem_ref = rspamd_http_message_shmem_ref (msg);
 
                        session->master_conn->backend_conn = rspamd_http_connection_new (
                                        NULL,
@@ -1215,10 +1218,11 @@ proxy_client_finish_handler (struct rspamd_http_connection *conn,
                                        session->ctx->local_key);
                        msg->peer_key = rspamd_pubkey_ref (backend->key);
 
-                       rspamd_http_connection_write_message (session->master_conn->backend_conn,
-                               msg, NULL, NULL, session->master_conn,
-                               session->master_conn->backend_sock,
-                               &session->ctx->io_tv, session->ctx->ev_base);
+                       rspamd_http_connection_write_message_shared (
+                                       session->master_conn->backend_conn,
+                                       msg, NULL, NULL, session->master_conn,
+                                       session->master_conn->backend_sock,
+                                       &session->ctx->io_tv, session->ctx->ev_base);
                }
        }
        else {