]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] fuzzy_storage: drop per-command state caching from TCP session (issue #6001)
authorVsevolod Stakhov <vsevolod@rspamd.com>
Sat, 25 Apr 2026 09:27:23 +0000 (10:27 +0100)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Sat, 25 Apr 2026 09:27:23 +0000 (10:27 +0100)
Each TCP frame allocates a per-frame `cmd_session` whose key, extensions
buffer, ip_stat, and decrypt nm are owned and released by
`fuzzy_session_destroy`. The TCP I/O loop also pre-populated those fields
from a `fuzzy_common_session` cache and "transferred ownership" back at
the end of every frame — but the transfer overwrites without releasing,
so on a persistent TCP connection every frame after the first leaked:

  - one g_malloc'd extensions buffer (sent by every scanning request),
  - one fuzzy_key REF_RETAIN from the pre-populate path,
  - and, for encrypted frames, a second fuzzy_key REF_RETAIN from
    rspamd_fuzzy_decrypt_command's own unguarded `s->key = key`.

Stop caching per-command state on the TCP session entirely. cmd_session
starts zeroed each frame, decrypt populates `key`/`nm`, the wire parser
allocates `extensions`, process_command retains `ip_stat`, and
fuzzy_session_destroy frees the lot when the frame is done. The decrypt
overwrite is also no longer a leak because cmd_session->key starts NULL.

Also delete the never-instantiated `fuzzy_udp_session` /
`fuzzy_udp_session_destroy` / `rspamd_fuzzy_udp_write_reply` — UDP has
been using the legacy `fuzzy_session` path all along.

src/fuzzy_storage.c

index 6bffdc681c3ab41620804cda9414b52e5be55bb2..c87bd86f29099cb9453a43d0789e359eb497a1bd 100644 (file)
@@ -93,6 +93,12 @@ struct fuzzy_tcp_reply_queue_elt {
        struct fuzzy_tcp_reply_queue_elt *prev, *next; /* Link */
 };
 
+/*
+ * Connection-level state for a TCP fuzzy session. Per-command state
+ * (parsed cmd, key retain, extensions buffer, decrypt nm, ip_stat) lives
+ * on the per-frame `struct fuzzy_session` and dies with it — never copy
+ * it onto the long-lived TCP session.
+ */
 struct fuzzy_common_session {
        struct rspamd_fuzzy_storage_ctx *ctx;
        int fd;
@@ -100,14 +106,6 @@ struct fuzzy_common_session {
        ev_tstamp timestamp;
        struct rspamd_worker *worker;
        rspamd_inet_addr_t *addr;
-
-       enum rspamd_fuzzy_epoch epoch;
-       enum fuzzy_cmd_type cmd_type;
-       struct rspamd_fuzzy_shingle_cmd cmd;
-       struct fuzzy_key *key;
-       struct rspamd_fuzzy_cmd_extension *extensions;
-       struct fuzzy_key_stat *ip_stat;
-       unsigned char nm[rspamd_cryptobox_MAX_NMBYTES];
 };
 
 struct fuzzy_tcp_session {
@@ -123,7 +121,6 @@ struct fuzzy_tcp_session {
        uint16_t cur_frame_state;
        uint16_t bytes_unprocessed;
 
-       /* Common with UDP session */
        struct fuzzy_common_session common;
        ref_entry_t ref;
 
@@ -131,13 +128,6 @@ struct fuzzy_tcp_session {
        unsigned char input_buf[FUZZY_TCP_BUFFER_LENGTH];
 };
 
-struct fuzzy_udp_session {
-       /* Common fields with TCP session */
-       struct fuzzy_common_session common;
-       struct rspamd_fuzzy_encrypted_reply reply; /* Again: contains everything */
-       ref_entry_t ref;
-};
-
 struct fuzzy_peer_request {
        ev_io io_ev;
        struct fuzzy_peer_cmd cmd;
@@ -151,7 +141,6 @@ struct rspamd_updates_cbdata {
 };
 
 static void rspamd_fuzzy_write_reply(struct fuzzy_session *session);
-static void rspamd_fuzzy_udp_write_reply(struct fuzzy_udp_session *session);
 static bool rspamd_fuzzy_tcp_write_reply(struct fuzzy_tcp_session *session,
                                                                                 struct fuzzy_tcp_reply_queue_elt *reply);
 static gboolean rspamd_fuzzy_process_updates_queue(struct rspamd_fuzzy_storage_ctx *ctx,
@@ -1923,45 +1912,8 @@ fuzzy_tcp_session_destroy(gpointer d)
 
        close(session->common.fd);
        rspamd_inet_address_free(session->common.addr);
-       rspamd_explicit_memzero(session->common.nm, sizeof(session->common.nm));
        session->common.worker->nconns--;
 
-       if (session->common.ip_stat) {
-               REF_RELEASE(session->common.ip_stat);
-       }
-
-       if (session->common.extensions) {
-               g_free(session->common.extensions);
-       }
-
-       if (session->common.key) {
-               REF_RELEASE(session->common.key);
-       }
-
-       g_free(session);
-}
-
-static void
-fuzzy_udp_session_destroy(gpointer d)
-{
-       struct fuzzy_udp_session *session = d;
-
-       rspamd_inet_address_free(session->common.addr);
-       rspamd_explicit_memzero(session->common.nm, sizeof(session->common.nm));
-       session->common.worker->nconns--;
-
-       if (session->common.ip_stat) {
-               REF_RELEASE(session->common.ip_stat);
-       }
-
-       if (session->common.extensions) {
-               g_free(session->common.extensions);
-       }
-
-       if (session->common.key) {
-               REF_RELEASE(session->common.key);
-       }
-
        g_free(session);
 }
 
@@ -2240,49 +2192,29 @@ rspamd_fuzzy_tcp_io(EV_P_ ev_io *w, int revents)
 
                        /* Check if we have complete frame */
                        if (session->bytes_unprocessed - processed_offset >= frame_len) {
-                               /* Create heap-allocated session for async processing */
+                               /*
+                                * Per-frame command session. Owns its own key retain,
+                                * extensions buffer, ip_stat retain, and decrypt nm —
+                                * fuzzy_session_destroy releases all of that when this
+                                * cmd_session is dropped. Do NOT cache any of it on the
+                                * long-lived TCP session: that turned the per-frame
+                                * allocations into per-frame leaks.
+                                */
                                struct fuzzy_session *cmd_session = g_malloc0(sizeof(*cmd_session));
                                REF_INIT_RETAIN(cmd_session, fuzzy_session_destroy);
 
-                               /* Copy data from TCP session to command session */
                                cmd_session->worker = session->common.worker;
                                cmd_session->addr = rspamd_inet_address_copy(session->common.addr, NULL);
                                cmd_session->ctx = session->common.ctx;
                                cmd_session->fd = session->common.fd;
                                cmd_session->timestamp = session->common.timestamp;
-                               cmd_session->key = session->common.key;
-                               cmd_session->ip_stat = session->common.ip_stat;
-                               memcpy(cmd_session->nm, session->common.nm, sizeof(cmd_session->nm));
-
-                               /* Retain references to shared objects */
-                               if (cmd_session->key) {
-                                       REF_RETAIN(cmd_session->key);
-                               }
-                               if (cmd_session->ip_stat) {
-                                       REF_RETAIN(cmd_session->ip_stat);
-                               }
-                               /* Don't increment nconns here - TCP session already counted the connection */
 
-                               /* Set TCP session pointer so replies go to TCP queue */
+                               /* Replies go to the TCP queue, not the socket directly */
                                cmd_session->tcp_session = session;
-                               REF_RETAIN(session); /* TCP session must live until command is processed */
+                               REF_RETAIN(session); /* released by fuzzy_session_destroy */
 
                                if (rspamd_fuzzy_cmd_from_wire(session->input_buf + processed_offset,
                                                                                           frame_len, cmd_session)) {
-                                       /* Copy parsed data back to TCP session for tracking */
-                                       session->common.epoch = cmd_session->epoch;
-                                       session->common.cmd_type = cmd_session->cmd_type;
-                                       memcpy(&session->common.cmd, &cmd_session->cmd, sizeof(session->common.cmd));
-
-                                       /* Transfer ownership of key and extensions to TCP session */
-                                       /* Clear cmd_session pointers to avoid double-free */
-                                       session->common.key = cmd_session->key;
-                                       session->common.extensions = cmd_session->extensions;
-                                       cmd_session->key = NULL;
-                                       cmd_session->extensions = NULL;
-                                       memcpy(session->common.nm, cmd_session->nm, sizeof(session->common.nm));
-
-                                       /* Process command - replies will go to TCP queue via tcp_session pointer */
                                        rspamd_fuzzy_process_command(cmd_session);
                                }
                                else {
@@ -2290,10 +2222,8 @@ rspamd_fuzzy_tcp_io(EV_P_ ev_io *w, int revents)
                                        msg_debug_fuzzy_storage("invalid TCP fuzzy command of size %d received from %s",
                                                                                        (int) frame_len,
                                                                                        rspamd_inet_address_to_string(session->common.addr));
-                                       /* Note: Don't release session here - cmd_session holds a reference and will release it */
                                }
 
-                               /* Release our reference - session will be freed when all callbacks complete */
                                REF_RELEASE(cmd_session);
 
                                processed_offset += frame_len;