From ec64510fc496ca6e01576a53bb7862b7a9308cd3 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Fri, 10 Jun 2016 18:47:25 +0100 Subject: [PATCH] [Fix] Fix race condition with shared memory by refcounts --- src/libutil/http.c | 90 +++++++++++++++++++++++++++++++++----- src/libutil/http.h | 21 +++++++++ src/libutil/http_private.h | 8 +++- src/rspamd_proxy.c | 16 ++++--- 4 files changed, 117 insertions(+), 18 deletions(-) diff --git a/src/libutil/http.c b/src/libutil/http.c index cedf394ae..ad47b1b20 100644 --- 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 1e4ffbbc0..4cbcaf5fb 100644 --- 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 a944cb31b..38fbec742 100644 --- 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 9d70306e9..6e0c1c254 100644 --- 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 { -- 2.39.5