]> git.ipfire.org Git - thirdparty/rspamd.git/commitdiff
[Fix] Fix allocator mismatches in libucl
authorVsevolod Stakhov <vsevolod@rspamd.com>
Sat, 1 Nov 2025 22:14:07 +0000 (22:14 +0000)
committerVsevolod Stakhov <vsevolod@rspamd.com>
Sat, 1 Nov 2025 22:14:07 +0000 (22:14 +0000)
Resolve crashes caused by mixing jemalloc and system malloc allocators
in libucl. The issue occurred when memory allocated with one allocator
(e.g., strdup using system malloc) was freed with another (e.g., jemalloc's
free), causing segmentation faults.

Changes:
- Add UCL_REALLOC and UCL_STRDUP macros to ucl.h for consistent allocation
- Replace all strdup/malloc/realloc/free calls with UCL_* macros in:
  - Variable and macro registration (ucl_parser.c)
  - Parser state management (ucl_util.c)
  - Object copying and trash stack operations (ucl_util.c)
  - URL fetching - fix critical bug where malloc'd buffers were freed
    with ucl_munmap (munmap) instead of free (ucl_util.c)

This ensures all memory operations use the same allocator throughout libucl,
preventing allocator mismatch crashes on systems using jemalloc.

contrib/libucl/ucl.h
contrib/libucl/ucl_parser.c
contrib/libucl/ucl_util.c

index 8c2ac59a498dec5ebc53fffed1f73e4db6adab85..a91e5fb014bf19307777c75f4a755d3cacaa2f5e 100644 (file)
@@ -89,6 +89,19 @@ extern "C" {
 #ifndef UCL_FREE
 #define UCL_FREE(size, ptr) free(ptr)
 #endif
+#ifndef UCL_REALLOC
+#define UCL_REALLOC(ptr, size) realloc(ptr, size)
+#endif
+#ifndef UCL_STRDUP
+static inline char *ucl_strdup_impl(const char *s)
+{
+       size_t len = strlen(s) + 1;
+       char *p = (char *) UCL_ALLOC(len);
+       if (p) memcpy(p, s, len);
+       return p;
+}
+#define UCL_STRDUP(str) ucl_strdup_impl(str)
+#endif
 
 #if    __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 4)
 #define UCL_WARN_UNUSED_RESULT               \
index 11dd991cb974018831dab199465d77dc0570267c..107706b457f478b76ebfc073d5ddd89d263b1f48 100644 (file)
@@ -2865,7 +2865,7 @@ bool ucl_parser_register_macro(struct ucl_parser *parser, const char *macro,
 
        memset(new, 0, sizeof(struct ucl_macro));
        new->h.handler = handler;
-       new->name = strdup(macro);
+       new->name = UCL_STRDUP(macro);
        if (new->name == NULL) {
                UCL_FREE(sizeof(struct ucl_macro), new);
                return false;
@@ -2891,7 +2891,7 @@ bool ucl_parser_register_context_macro(struct ucl_parser *parser, const char *ma
 
        memset(new, 0, sizeof(struct ucl_macro));
        new->h.context_handler = handler;
-       new->name = strdup(macro);
+       new->name = UCL_STRDUP(macro);
        if (new->name == NULL) {
                UCL_FREE(sizeof(struct ucl_macro), new);
                return false;
@@ -2925,8 +2925,8 @@ void ucl_parser_register_variable(struct ucl_parser *parser, const char *var,
                if (new != NULL) {
                        /* Remove variable */
                        DL_DELETE(parser->variables, new);
-                       free(new->var);
-                       free(new->value);
+                       UCL_FREE(new->var_len + 1, new->var);
+                       UCL_FREE(new->value_len + 1, new->value);
                        UCL_FREE(sizeof(struct ucl_variable), new);
                }
                else {
@@ -2941,16 +2941,16 @@ void ucl_parser_register_variable(struct ucl_parser *parser, const char *var,
                                return;
                        }
                        memset(new, 0, sizeof(struct ucl_variable));
-                       new->var = strdup(var);
+                       new->var = UCL_STRDUP(var);
                        new->var_len = strlen(var);
-                       new->value = strdup(value);
+                       new->value = UCL_STRDUP(value);
                        new->value_len = strlen(value);
 
                        DL_APPEND(parser->variables, new);
                }
                else {
-                       free(new->value);
-                       new->value = strdup(value);
+                       UCL_FREE(new->value_len + 1, new->value);
+                       new->value = UCL_STRDUP(value);
                        new->value_len = strlen(value);
                }
        }
@@ -3043,7 +3043,7 @@ bool ucl_parser_add_chunk_full(struct ucl_parser *parser, const unsigned char *d
                chunk->parse_type = parse_type;
 
                if (parser->cur_file) {
-                       chunk->fname = strdup(parser->cur_file);
+                       chunk->fname = UCL_STRDUP(parser->cur_file);
                }
 
                LL_PREPEND(parser->chunks, chunk);
index d5b84f6a53fabf96de97136bab0e84a51baab784..5ea9f89ffc6062d8919dd01d5bbb0e64202a199e 100644 (file)
@@ -504,7 +504,7 @@ ucl_copy_key_trash (const ucl_object_t *obj)
        }
        if (obj->trash_stack[UCL_TRASH_KEY] == NULL && obj->key != NULL) {
                deconst = __DECONST (ucl_object_t *, obj);
-               deconst->trash_stack[UCL_TRASH_KEY] = malloc (obj->keylen + 1);
+               deconst->trash_stack[UCL_TRASH_KEY] = UCL_ALLOC(obj->keylen + 1);
                if (deconst->trash_stack[UCL_TRASH_KEY] != NULL) {
                        memcpy (deconst->trash_stack[UCL_TRASH_KEY], obj->key, obj->keylen);
                        deconst->trash_stack[UCL_TRASH_KEY][obj->keylen] = '\0';
@@ -538,7 +538,7 @@ ucl_chunk_free (struct ucl_chunk *chunk)
                chunk->special_handlers = NULL;
 
                if (chunk->fname) {
-                       free (chunk->fname);
+                       UCL_FREE(strlen(chunk->fname) + 1, chunk->fname);
                }
 
                UCL_FREE (sizeof (*chunk), chunk);
@@ -559,7 +559,7 @@ ucl_copy_value_trash (const ucl_object_t *obj)
 
                        /* Special case for strings */
                        if (obj->flags & UCL_OBJECT_BINARY) {
-                               deconst->trash_stack[UCL_TRASH_VALUE] = malloc (obj->len);
+                               deconst->trash_stack[UCL_TRASH_VALUE] = UCL_ALLOC(obj->len);
                                if (deconst->trash_stack[UCL_TRASH_VALUE] != NULL) {
                                        memcpy (deconst->trash_stack[UCL_TRASH_VALUE],
                                                        obj->value.sv,
@@ -568,7 +568,7 @@ ucl_copy_value_trash (const ucl_object_t *obj)
                                }
                        }
                        else {
-                               deconst->trash_stack[UCL_TRASH_VALUE] = malloc (obj->len + 1);
+                               deconst->trash_stack[UCL_TRASH_VALUE] = UCL_ALLOC(obj->len + 1);
                                if (deconst->trash_stack[UCL_TRASH_VALUE] != NULL) {
                                        memcpy (deconst->trash_stack[UCL_TRASH_VALUE],
                                                        obj->value.sv,
@@ -622,10 +622,10 @@ ucl_parser_free (struct ucl_parser *parser)
        }
 
        LL_FOREACH_SAFE (parser->stack, stack, stmp) {
-               free (stack);
+               UCL_FREE(sizeof(struct ucl_stack), stack);
        }
        HASH_ITER (hh, parser->macroes, macro, mtmp) {
-               free (macro->name);
+               UCL_FREE(strlen(macro->name) + 1, macro->name);
                HASH_DEL (parser->macroes, macro);
                UCL_FREE (sizeof (struct ucl_macro), macro);
        }
@@ -636,8 +636,8 @@ ucl_parser_free (struct ucl_parser *parser)
                UCL_FREE (sizeof (struct ucl_pubkey), key);
        }
        LL_FOREACH_SAFE (parser->variables, var, vtmp) {
-               free (var->value);
-               free (var->var);
+               UCL_FREE(var->value_len + 1, var->value);
+               UCL_FREE(var->var_len + 1, var->var);
                UCL_FREE (sizeof (struct ucl_variable), var);
        }
        LL_FOREACH_SAFE (parser->trash_objs, tr, trtmp) {
@@ -649,7 +649,7 @@ ucl_parser_free (struct ucl_parser *parser)
        }
 
        if (parser->cur_file) {
-               free (parser->cur_file);
+               UCL_FREE(strlen(parser->cur_file) + 1, parser->cur_file);
        }
 
        if (parser->comments) {
@@ -765,7 +765,7 @@ ucl_curl_write_callback (void* contents, size_t size, size_t nmemb, void* ud)
        struct ucl_curl_cbdata *cbdata = ud;
        size_t realsize = size * nmemb;
 
-       cbdata->buf = realloc (cbdata->buf, cbdata->buflen + realsize + 1);
+       cbdata->buf = UCL_REALLOC(cbdata->buf, cbdata->buflen + realsize + 1);
        if (cbdata->buf == NULL) {
                return 0;
        }
@@ -812,7 +812,7 @@ ucl_fetch_url (const unsigned char *url, unsigned char **buf, size_t *buflen,
        }
 
        *buflen = us.size;
-       *buf = malloc (*buflen);
+       *buf = UCL_ALLOC(*buflen);
        if (*buf == NULL) {
                ucl_create_err (err, "cannot allocate buffer for URL %s: %s",
                                url, strerror (errno));
@@ -859,7 +859,7 @@ ucl_fetch_url (const unsigned char *url, unsigned char **buf, size_t *buflen,
                }
                curl_easy_cleanup (curl);
                if (cbdata.buf) {
-                       free (cbdata.buf);
+                       UCL_FREE(cbdata.buflen, cbdata.buf);
                }
                return false;
        }
@@ -1042,12 +1042,12 @@ ucl_include_url (const unsigned char *data, size_t len,
                                                        urlbuf,
                                                        ERR_error_string (ERR_get_error (), NULL));
                        if (siglen > 0) {
-                               ucl_munmap (sigbuf, siglen);
+                               UCL_FREE(siglen, sigbuf);
                        }
                        return false;
                }
                if (siglen > 0) {
-                       ucl_munmap (sigbuf, siglen);
+                       UCL_FREE(siglen, sigbuf);
                }
 #endif
        }
@@ -1067,7 +1067,7 @@ ucl_include_url (const unsigned char *data, size_t len,
        }
 
        parser->state = prev_state;
-       free (buf);
+       UCL_FREE(buflen, buf);
 
        return res;
 }
@@ -1390,13 +1390,13 @@ ucl_include_file_single (const unsigned char *data, size_t len,
                DL_FOREACH_SAFE (parser->variables, cur_var, tmp_var) {
                        if (strcmp (cur_var->var, "CURDIR") == 0 && old_curdir) {
                                DL_DELETE (parser->variables, cur_var);
-                               free (cur_var->var);
-                               free (cur_var->value);
+                               UCL_FREE(cur_var->var_len + 1, cur_var->var);
+                               UCL_FREE(cur_var->value_len + 1, cur_var->value);
                                UCL_FREE (sizeof (struct ucl_variable), cur_var);
                        } else if (strcmp (cur_var->var, "FILENAME") == 0 && old_filename) {
                                DL_DELETE (parser->variables, cur_var);
-                               free (cur_var->var);
-                               free (cur_var->value);
+                               UCL_FREE(cur_var->var_len + 1, cur_var->var);
+                               UCL_FREE(cur_var->value_len + 1, cur_var->value);
                                UCL_FREE (sizeof (struct ucl_variable), cur_var);
                        }
                }
@@ -1995,10 +1995,10 @@ ucl_parser_set_filevars (struct ucl_parser *parser, const char *filename, bool n
                }
 
                if (parser->cur_file) {
-                       free (parser->cur_file);
+                       UCL_FREE(strlen(parser->cur_file) + 1, parser->cur_file);
                }
 
-               parser->cur_file = strdup (realbuf);
+               parser->cur_file = UCL_STRDUP(realbuf);
 
                /* Define variables */
                ucl_parser_register_variable (parser, "FILENAME", realbuf);
@@ -2097,7 +2097,7 @@ ucl_parser_add_fd_full (struct ucl_parser *parser, int fd,
        }
 
        if (parser->cur_file) {
-               free (parser->cur_file);
+               UCL_FREE(strlen(parser->cur_file) + 1, parser->cur_file);
        }
        parser->cur_file = NULL;
        len = st.st_size;
@@ -3651,7 +3651,7 @@ ucl_object_copy_internal (const ucl_object_t *other, bool allow_array)
        if (other->type == UCL_USERDATA) {
                sz = sizeof (struct ucl_object_userdata);
        }
-       new = malloc (sz);
+       new = UCL_ALLOC(sz);
 
        if (new != NULL) {
                memcpy (new, other, sz);
@@ -3668,7 +3668,7 @@ ucl_object_copy_internal (const ucl_object_t *other, bool allow_array)
                if (other->trash_stack[UCL_TRASH_KEY] != NULL) {
                        new->trash_stack[UCL_TRASH_KEY] = NULL;
                        if (other->key == (const char *)other->trash_stack[UCL_TRASH_KEY]) {
-                               new->trash_stack[UCL_TRASH_KEY] = malloc(other->keylen + 1);
+                               new->trash_stack[UCL_TRASH_KEY] = UCL_ALLOC(other->keylen + 1);
                                memcpy(new->trash_stack[UCL_TRASH_KEY], other->trash_stack[UCL_TRASH_KEY], other->keylen);
                                new->trash_stack[UCL_TRASH_KEY][other->keylen] = '\0';
                                new->key = new->trash_stack[UCL_TRASH_KEY];
@@ -3676,7 +3676,7 @@ ucl_object_copy_internal (const ucl_object_t *other, bool allow_array)
                }
                if (other->trash_stack[UCL_TRASH_VALUE] != NULL) {
                        new->trash_stack[UCL_TRASH_VALUE] =
-                                       strdup (other->trash_stack[UCL_TRASH_VALUE]);
+                               UCL_STRDUP(other->trash_stack[UCL_TRASH_VALUE]);
                        if (new->type == UCL_STRING) {
                                new->value.sv = new->trash_stack[UCL_TRASH_VALUE];
                        }