From 0f2f88a6157250fecb9ce5d8d28b02b99739a2d2 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Wed, 2 Mar 2011 21:44:02 +0300 Subject: [PATCH] Try to fix memory issues. --- src/filter.c | 43 ++++++++++++++----------- src/html.c | 16 +++++++--- src/message.c | 2 ++ src/plugins/regexp.c | 2 +- src/plugins/spf.c | 5 --- src/spf.c | 75 ++++++++++++++++++++++++-------------------- src/url.c | 18 +++++------ 7 files changed, 90 insertions(+), 71 deletions(-) diff --git a/src/filter.c b/src/filter.c index d41c85ff0..68a43d1f9 100644 --- a/src/filter.c +++ b/src/filter.c @@ -84,35 +84,37 @@ insert_metric_result (struct worker_task *task, struct metric *metric, const gch /* Add metric score */ - if ((s = g_hash_table_lookup (metric_res->symbols, symbol)) != NULL) { - if (!single) { - if (s->options && opts && opts != s->options) { - /* Append new options */ - s->options = g_list_concat (s->options, g_list_copy(opts)); - /* - * Note that there is no need to add new destructor of GList as elements of appended - * GList are used directly, so just free initial GList - */ - } - else if (opts) { - s->options = opts; - memory_pool_add_destructor (task->task_pool, (pool_destruct_func) g_list_free, s->options); - } - - s->score += w; - metric_res->score += w; + if (!single && (s = g_hash_table_lookup (metric_res->symbols, symbol)) != NULL) { + if (s->options && opts && opts != s->options) { + /* Append new options */ + s->options = g_list_concat (s->options, g_list_copy(opts)); + /* + * Note that there is no need to add new destructor of GList as elements of appended + * GList are used directly, so just free initial GList + */ } + else if (opts) { + s->options = g_list_copy (opts); + memory_pool_add_destructor (task->task_pool, (pool_destruct_func) g_list_free, s->options); + } + + s->score += w; + metric_res->score += w; } else { s = memory_pool_alloc (task->task_pool, sizeof (struct symbol)); s->score = w; - s->options = opts; + s->name = symbol; metric_res->score += w; if (opts) { + s->options = g_list_copy (opts); memory_pool_add_destructor (task->task_pool, (pool_destruct_func) g_list_free, s->options); } + else { + s->options = NULL; + } g_hash_table_insert (metric_res->symbols, (gpointer) symbol, s); } @@ -165,6 +167,11 @@ insert_result_common (struct worker_task *task, const gchar *symbol, double flag cur = g_list_next (cur); } } + + if (opts != NULL) { + /* XXX: it is not wise to destroy them here */ + g_list_free (opts); + } } /* Insert result that may be increased on next insertions */ diff --git a/src/html.c b/src/html.c index 772f7d33b..2ab441ad3 100644 --- a/src/html.c +++ b/src/html.c @@ -679,14 +679,22 @@ html_strncasestr (const gchar *s, const gchar *find, gsize len) } static void -check_phishing (struct worker_task *task, struct uri *href_url, const gchar *url_text) +check_phishing (struct worker_task *task, struct uri *href_url, const gchar *url_text, gsize remain) { struct uri *new; gchar *url_str; - gsize len; + const gchar *p; + gsize len = 0; gint off, rc; - len = strcspn (url_text, "<>"); + p = url_text; + while (len < remain) { + if (*p == '<' || *p == '>') { + break; + } + len ++; + p ++; + } if (url_try_text (task->task_pool, url_text, len, &off, &url_str) && url_str != NULL) { new = memory_pool_alloc0 (task->task_pool, sizeof (struct uri)); @@ -796,7 +804,7 @@ parse_tag_url (struct worker_task *task, struct mime_text_part *part, tag_id_t i * Check for phishing */ if ((p = strchr (c, '>')) != NULL ) { - check_phishing (task, url, p + 1); + check_phishing (task, url, p + 1, tag_len - (p - tag_text)); } if (part->html_urls && g_tree_lookup (part->html_urls, url_text) == NULL) { g_tree_insert (part->html_urls, url_text, url); diff --git a/src/message.c b/src/message.c index 0a88ab0f7..86e333dc1 100644 --- a/src/message.c +++ b/src/message.c @@ -837,7 +837,9 @@ mime_foreach_callback (GMimeObject * part, gpointer user_data) msg_err ("endless recursion detected: %d", task->parser_recursion); return; } +#ifndef GMIME24 g_object_unref (message); +#endif } else if (GMIME_IS_MESSAGE_PARTIAL (part)) { /* message/partial */ diff --git a/src/plugins/regexp.c b/src/plugins/regexp.c index 8afefb9da..0ab4451e1 100644 --- a/src/plugins/regexp.c +++ b/src/plugins/regexp.c @@ -607,7 +607,7 @@ process_regexp (struct rspamd_regexp *re, struct worker_task *task, const gchar guint8 *ct; gsize clen; gint r, passed = 0, start, end, old; - gboolean matched; + gboolean matched = FALSE; GList *cur, *headerlist; GRegex *regexp; diff --git a/src/plugins/spf.c b/src/plugins/spf.c index f7b0caab3..7b7dac605 100644 --- a/src/plugins/spf.c +++ b/src/plugins/spf.c @@ -177,11 +177,6 @@ spf_plugin_callback (struct spf_record *record, struct worker_task *task) } cur = g_list_previous (cur); } - if (record->addrs != NULL) { - /* Free addresses that we already proceed */ - g_list_free (record->addrs); - record->addrs = NULL; - } } if (task->save.saved == 0) { diff --git a/src/spf.c b/src/spf.c index 212bd0074..f19665e79 100644 --- a/src/spf.c +++ b/src/spf.c @@ -145,6 +145,19 @@ dump_spf_record (GList *addrs) msg_info ("spf record: %s", logbuf); } +/* + * Destructor for spf record + */ +static void +spf_record_destructor (gpointer r) +{ + struct spf_record *rec = r; + + if (rec->addrs) { + g_list_free (rec->addrs); + } +} + static gboolean parse_spf_ipmask (const gchar *begin, struct spf_addr *addr) { @@ -292,6 +305,7 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg) else if (reply->type == DNS_REQUEST_A) { if (cb->addr->addr == 0) { cb->addr->addr = ntohl (elt_data->a.addr[0].s_addr); + cb->addr->mask = 32; } else { /* Insert one more address */ @@ -313,6 +327,7 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg) if (reply->type == DNS_REQUEST_A) { /* XXX: process only one record */ cb->addr->addr = ntohl (elt_data->a.addr[0].s_addr); + cb->addr->mask = 32; } break; case SPF_RESOLVE_PTR: @@ -344,50 +359,32 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg) start_spf_parse (cb->rec, begin); cb->rec->in_include = FALSE; - if (tmp) { + if (tmp && cb->rec->addrs) { tmp1 = g_list_find (tmp, cb->addr); if (tmp1) { /* Insert new list in place of include element */ + cb->rec->addrs->prev = tmp1->prev; last = g_list_last (cb->rec->addrs); - - if (tmp1->prev == NULL && tmp1->next == NULL) { - g_list_free1 (tmp1); + last->next = tmp1; + tmp1->prev = last; + /* If tmp1 is at the beginning of the list, shift list */ + if (tmp == tmp1) { + tmp = cb->rec->addrs; } else { - - if (tmp1->prev) { - tmp1->prev->next = cb->rec->addrs; - } - else { - /* Elt is the first element, so we need to shift temporary list */ - tmp = tmp1->next; - tmp->prev = NULL; - } - if (tmp1->next) { - tmp1->next->prev = last; - if (last != NULL) { - last->next = tmp1->next; - } - } - - if (cb->rec->addrs != NULL) { - cb->rec->addrs->prev = tmp1->prev; - } - - /* Shift temporary list */ - while (tmp->prev) { - tmp = tmp->prev; - } - - cb->rec->addrs = tmp; - g_list_free1 (tmp1); + tmp1->prev->next = cb->rec->addrs; + } + /* Remove self element */ + tmp = g_list_delete_link (tmp, tmp1); #ifdef SPF_DEBUG - msg_info ("after include"); - dump_spf_record (cb->rec->addrs); + msg_info ("after include"); + dump_spf_record (cb->rec->addrs); #endif - } } } + if (tmp) { + cb->rec->addrs = tmp; + } } break; case SPF_RESOLVE_EXP: @@ -410,16 +407,19 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg) if (reply->type == DNS_REQUEST_MX) { msg_info ("cannot find MX record for %s", cb->rec->cur_domain); cb->addr->addr = ntohl (INADDR_NONE); + cb->addr->mask = 32; } else if (reply->type != DNS_REQUEST_MX) { msg_info ("cannot resolve MX record for %s", cb->rec->cur_domain); cb->addr->addr = ntohl (INADDR_NONE); + cb->addr->mask = 32; } break; case SPF_RESOLVE_A: if (reply->type == DNS_REQUEST_A) { /* XXX: process only one record */ cb->addr->addr = ntohl (INADDR_NONE); + cb->addr->mask = 32; } break; case SPF_RESOLVE_PTR: @@ -434,6 +434,7 @@ spf_record_dns_callback (struct rspamd_dns_reply *reply, gpointer arg) break; case SPF_RESOLVE_EXISTS: cb->addr->addr = ntohl (INADDR_NONE); + cb->addr->mask = 32; break; } } @@ -909,6 +910,8 @@ expand_spf_macro (struct worker_task *task, struct spf_record *rec, gchar *begin (x) = memory_pool_alloc (task->task_pool, sizeof (struct spf_addr)); \ (x)->mech = check_spf_mech (rec->cur_elt, &need_shift); \ (x)->spf_string = memory_pool_strdup (task->task_pool, begin); \ + (x)->addr = 0; \ + (x)->mask = 32; \ } while (0); /* Read current element and try to parse record */ @@ -1149,7 +1152,10 @@ resolve_spf (struct worker_task *task, spf_cb_t callback) rec = memory_pool_alloc0 (task->task_pool, sizeof (struct spf_record)); rec->task = task; rec->callback = callback; + /* Add destructor */ + memory_pool_add_destructor (task->task_pool, (pool_destruct_func)spf_record_destructor, rec); + /* Extract from data */ if (task->from && (domain = strchr (task->from, '@')) != NULL && *domain == '@') { rec->sender = task->from; @@ -1170,6 +1176,7 @@ resolve_spf (struct worker_task *task, spf_cb_t callback) } } else { + /* Extract from header */ domains = message_get_header (task->task_pool, task->message, "From", FALSE); if (domains != NULL) { diff --git a/src/url.c b/src/url.c index 8282f9217..83eb30d4d 100644 --- a/src/url.c +++ b/src/url.c @@ -930,7 +930,7 @@ url_file_end (const gchar *begin, const gchar *end, const gchar *pos, url_match_ } } - while (p < end && *p != stop && is_urlsafe (*p)) { + while (p < end - 1 && *p != stop && is_urlsafe (*p)) { p ++; } @@ -979,14 +979,14 @@ url_web_end (const gchar *begin, const gchar *end, const gchar *pos, url_match_t if (is_atom (*p)) { /* might be a domain or user@domain */ c = p; - while (p < end) { + while (p < end - 1) { if (!is_atom (*p)) { break; } p++; - while (p < end && is_atom (*p)) { + while (p < end - 1 && is_atom (*p)) { p++; } @@ -1006,18 +1006,18 @@ url_web_end (const gchar *begin, const gchar *end, const gchar *pos, url_match_t } else if (is_domain (*p)) { domain: - while (p < end) { + while (p < end - 1) { if (!is_domain (*p)) { break; } p++; - while (p < end && is_domain (*p)) { + while (p < end - 1 && is_domain (*p)) { p++; } - if ((p + 1) < end && *p == '.' && (is_domain (*(p + 1)) || *(p + 1) == '/')) { + if ((p + 1) < end - 1 && *p == '.' && (is_domain (*(p + 1)) || *(p + 1) == '/')) { p++; } } @@ -1034,7 +1034,7 @@ domain: if (is_digit (*p) || passwd) { port = (*p++ - '0'); - while (p < end && is_digit (*p) && port < 65536) { + while (p < end - 1 && is_digit (*p) && port < 65536) { port = (port * 10) + (*p++ - '0'); } @@ -1052,7 +1052,7 @@ domain: passwd = TRUE; c = p; - while (p < end && is_atom (*p)) { + while (p < end - 1 && is_atom (*p)) { p++; } @@ -1076,7 +1076,7 @@ domain: case '/': /* we've detected a path component to our url */ p++; case '?': - while (p < end && is_urlsafe (*p)) { + while (p < end - 1 && is_urlsafe (*p)) { if (*p == open_brace) { brace_stack++; } -- 2.39.5