From d8add91444769387ef3ce1054fdb2b7101ffc60d Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 02:10:37 -0700 Subject: [PATCH 01/12] replace assert() with error handling in writer, sorter, and zlib decompression --- mtbl/block.c | 18 ++++++++++-------- mtbl/compression.c | 16 ++++++++++++++-- mtbl/sorter.c | 28 ++++++++++++++++++++++------ mtbl/writer.c | 14 ++++++++++++-- 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/mtbl/block.c b/mtbl/block.c index 131033b..2957117 100644 --- a/mtbl/block.c +++ b/mtbl/block.c @@ -16,11 +16,11 @@ */ // Copyright (c) 2011 The LevelDB Authors. All rights reserved. -// +// // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are // met: -// +// // * Redistributions of source code must retain the above copyright // notice, this list of conditions and the following disclaimer. // @@ -32,7 +32,7 @@ // * Neither the name of Google Inc. nor the names of its // contributors may be used to endorse or promote products derived from // this software without specific prior written permission. -// +// // THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS // "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT // LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR @@ -202,7 +202,9 @@ static bool parse_next_key(struct block_iter *bi) { bi->current = next_entry_offset(bi); - uint8_t *p = bi->data + bi->current; uint8_t *limit = bi->data + bi->restarts; + uint8_t *p = bi->data + bi->current; + uint8_t *limit = bi->data + bi->restarts; + if (p >= limit) { /* no more entries to return, mark as invalid */ bi->current = bi->restarts; @@ -214,7 +216,7 @@ parse_next_key(struct block_iter *bi) uint32_t shared, non_shared, value_length; p = decode_entry(p, limit, &shared, &non_shared, &value_length); assert(!(p == NULL || ubuf_size(bi->key) < shared)); - + ubuf_clip(bi->key, shared); ubuf_append(bi->key, p, non_shared); bi->next = p + non_shared + value_length; @@ -241,7 +243,7 @@ block_iter_seek_to_first(struct block_iter *bi) parse_next_key(bi); } -void +void block_iter_seek_to_last(struct block_iter *bi) { seek_to_restart_point(bi, bi->num_restarts - 1); @@ -268,7 +270,7 @@ block_iter_seek(struct block_iter *bi, const uint8_t *target, size_t target_len) uint32_t start_ri = bi->restart_index; /* Current key is in this restart-block. */ bool from_start = true; /* Search from start of restart-block? */ - /* + /* * If the restart_index is not zero and not equal to the number of * restarts, then begin with galloping search in the restart array to find * the first restart point with a key >= target, otherwise just do binary @@ -352,7 +354,7 @@ block_iter_next(struct block_iter *bi) return (block_iter_valid(bi)); } -void +void block_iter_prev(struct block_iter *bi) { assert(block_iter_valid(bi)); diff --git a/mtbl/compression.c b/mtbl/compression.c index f4d76c6..a9e42de 100644 --- a/mtbl/compression.c +++ b/mtbl/compression.c @@ -446,6 +446,7 @@ _mtbl_decompress_zlib( size_t *output_size) { int zret; + z_stream zs = { .avail_in = 0, .next_in = Z_NULL, @@ -479,8 +480,14 @@ _mtbl_decompress_zlib( do { zret = inflate(&zs, Z_FINISH); - assert(zret == Z_STREAM_END || zret == Z_BUF_ERROR); + if (zret != Z_STREAM_END && zret != Z_BUF_ERROR) { + goto fail; + } + if (zret != Z_STREAM_END) { + if (*output_size > SIZE_MAX / 2) { + goto fail; + } *output = my_realloc(*output, *output_size * 2); zs.next_out = *output + *output_size; zs.avail_out = *output_size; @@ -490,6 +497,11 @@ _mtbl_decompress_zlib( *output_size = zs.total_out; inflateEnd(&zs); - return (mtbl_res_success); +fail: + free(*output); + *output = NULL; + *output_size = 0; + inflateEnd(&zs); + return (mtbl_res_failure); } diff --git a/mtbl/sorter.c b/mtbl/sorter.c index f034bdf..34616ae 100644 --- a/mtbl/sorter.c +++ b/mtbl/sorter.c @@ -184,21 +184,39 @@ _mtbl_sorter_write_chunk(struct entry_batch *b) char template[64]; /* Temporary file creation: */ - sprintf(template, "/.mtbl.%ld.XXXXXX", (long)getpid()); + (void) snprintf(template, sizeof(template), "/.mtbl.%ld.XXXXXX", (long)getpid()); + ubuf *tmp_fname = ubuf_init(strlen(s->opt.tmp_dname) + strlen(template) + 1); ubuf_append(tmp_fname, (uint8_t *) s->opt.tmp_dname, strlen(s->opt.tmp_dname)); ubuf_append(tmp_fname, (uint8_t *) template, strlen(template)); ubuf_append(tmp_fname, (const uint8_t *) "\x00", 1); int fd = mkstemp((char *) ubuf_data(tmp_fname)); - assert(fd >= 0); + if (fd < 0) { + ubuf_destroy(&tmp_fname); + return (NULL); + } + int unlink_ret = unlink((char *) ubuf_data(tmp_fname)); - assert(unlink_ret == 0); + if (unlink_ret == -1) { + ubuf_destroy(&tmp_fname); + close(fd); + return (NULL); + } + ubuf_destroy(&tmp_fname); struct mtbl_writer_options *wopt = mtbl_writer_options_init(); mtbl_writer_options_set_compression(wopt, MTBL_COMPRESSION_SNAPPY); + struct mtbl_writer *w = mtbl_writer_init_fd(fd, wopt); + if (w == NULL) { + close(fd); + entry_vec_destroy(&b->entries); + free(b); + return (NULL); + } + mtbl_writer_options_destroy(&wopt); /* Sort and add sorter entries to the temporary file writer. */ @@ -241,9 +259,7 @@ _mtbl_sorter_write_chunk(struct entry_batch *b) } } - res = mtbl_writer_add(w, - entry_key(ent), ent->len_key, - entry_val(ent), ent->len_val); + res = mtbl_writer_add(w, entry_key(ent), ent->len_key, entry_val(ent), ent->len_val); free(ent); if (res != mtbl_res_success) break; diff --git a/mtbl/writer.c b/mtbl/writer.c index 74a2cac..6b02825 100644 --- a/mtbl/writer.c +++ b/mtbl/writer.c @@ -146,7 +146,10 @@ mtbl_writer_init_fd(int orig_fd, const struct mtbl_writer_options *opt) int fd; fd = dup(orig_fd); - assert(fd >= 0); + if (fd < 0) { + return (NULL); + } + w = my_calloc(1, sizeof(*w)); if (opt == NULL) { w->opt.compression_type = DEFAULT_COMPRESSION_TYPE; @@ -162,7 +165,14 @@ mtbl_writer_init_fd(int orig_fd, const struct mtbl_writer_options *opt) * Start writing from the current offset. This allows mtbl's callers * to reserve some initial bytes in the file. */ - w->last_offset = lseek(fd, 0, SEEK_CUR); + off_t offset = lseek(fd, 0, SEEK_CUR); + if (offset == (off_t)-1) { + close(fd); + free(w); + return (NULL); + } + w->last_offset = (uint64_t)offset; + w->pending_offset = w->last_offset; w->last_key = ubuf_init(256); w->m.file_version = MTBL_FORMAT_V2; From 81c3d25fc4ad22d9e56110477e5073b90696f0ae Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 07:45:30 +0000 Subject: [PATCH 02/12] fix zstd decompression sentinel check and compression buffer size guard --- mtbl/compression.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/mtbl/compression.c b/mtbl/compression.c index a9e42de..9f27b81 100644 --- a/mtbl/compression.c +++ b/mtbl/compression.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2026 DomainTools LLC * Copyright (c) 2012, 2014-2017, 2021 by Farsight Security, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -258,7 +259,7 @@ _mtbl_compress_zstd( compression_level = ZSTD_maxCLevel(); zstd_size = ZSTD_compressBound(input_size); - if (zstd_size < INT_MAX/2) { + if (zstd_size < SIZE_MAX/2) { /** * "Compression runs faster if `dstCapacity` >= * `ZSTD_compressBound(srcSize)`." @@ -390,13 +391,14 @@ _mtbl_decompress_zstd( { size_t ret = 0; - if (input_size > INT_MAX) + unsigned long long decompressed_size = ZSTD_getFrameContentSize(input, input_size); + if (decompressed_size == ZSTD_CONTENTSIZE_UNKNOWN || decompressed_size == ZSTD_CONTENTSIZE_ERROR) { return (mtbl_res_failure); - - *output_size = (size_t) ZSTD_getFrameContentSize(input, input_size); - if (*output_size <= 0) + } else if (decompressed_size > SIZE_MAX) { return (mtbl_res_failure); + } + *output_size = (size_t) decompressed_size; *output = my_malloc(*output_size); ret = ZSTD_decompress( From a5f4837f76d27c0de524050e477187d6023e3583 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 07:45:30 +0000 Subject: [PATCH 03/12] fix memory leak in sorter when merge callback returns NULL --- mtbl/sorter.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mtbl/sorter.c b/mtbl/sorter.c index 34616ae..be7b05d 100644 --- a/mtbl/sorter.c +++ b/mtbl/sorter.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2026 DomainTools LLC * Copyright (c) 2012-2016 by Farsight Security, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -239,6 +240,10 @@ _mtbl_sorter_write_chunk(struct entry_batch *b) entry_val(next_ent), next_ent->len_val, &merge_val, &len_merge_val); if (merge_val == NULL) { + for (size_t j = i; j < entry_vec_size(b->entries); j++) { + free(entry_vec_value(b->entries, j)); + } + entry_vec_destroy(&b->entries); free(b); mtbl_writer_destroy(&w); return (NULL); From 5c71572fa1bae294378b568ff75d8fc0ea5f4649 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 07:45:30 +0000 Subject: [PATCH 04/12] check return values of get_block_at_index and pthread_create --- mtbl/reader.c | 6 +++++- mtbl/sorter.c | 5 +++++ mtbl/threadpool.c | 29 ++++++++++++++++++++++++++--- mtbl/writer.c | 5 +++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/mtbl/reader.c b/mtbl/reader.c index fa2f858..10ab03f 100644 --- a/mtbl/reader.c +++ b/mtbl/reader.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 DomainTools LLC + * Copyright (c) 2022, 2026 DomainTools LLC * Copyright (c) 2012-2018 by Farsight Security, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -515,7 +515,11 @@ reader_iter_next(void *v, block_iter_destroy(&it->bi); if (!block_iter_next(it->index_iter)) return (mtbl_res_failure); + it->b = get_block_at_index(it->r, it->index_iter); + if (it->b == NULL) + return (mtbl_res_failure); + it->bi = block_iter_init(it->b); block_iter_seek_to_first(it->bi); it->valid = block_iter_get(it->bi, key, len_key, val, len_val); diff --git a/mtbl/sorter.c b/mtbl/sorter.c index be7b05d..c9bf34d 100644 --- a/mtbl/sorter.c +++ b/mtbl/sorter.c @@ -140,6 +140,11 @@ mtbl_sorter_init(const struct mtbl_sorter_options *opt) if (s->opt.pool != NULL) { s->pool = s->opt.pool->pool; s->rhandler = result_handler_init(_collect_readers_cb, s); + if (s->rhandler == NULL) { + entry_vec_destroy(&s->vec); + reader_vec_destroy(&s->readers); + return (NULL); + } } return (s); diff --git a/mtbl/threadpool.c b/mtbl/threadpool.c index c79c10b..b2c419c 100644 --- a/mtbl/threadpool.c +++ b/mtbl/threadpool.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024 DomainTools LLC + * Copyright (c) 2024, 2026 DomainTools LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,9 +15,11 @@ */ #include +#include #include #include #include +#include #include #include "threadpool.h" @@ -165,6 +167,7 @@ static struct thread * threadpool_next(struct threadpool *pool) { struct thread *thr = NULL; + int ret; pthread_mutex_lock(&pool->m); @@ -190,7 +193,18 @@ threadpool_next(struct threadpool *pool) thr->pool = pool; pthread_mutex_init(&thr->m, NULL); pthread_cond_init(&thr->c, NULL); - pthread_create(&thr->t, NULL, thread_worker, thr); + + ret = pthread_create(&thr->t, NULL, thread_worker, thr); + if (ret != 0) { + fprintf(stderr, "%s: pthread_create() failed: %s\n", __func__, strerror(ret)); + pthread_cond_destroy(&thr->c); + pthread_mutex_destroy(&thr->m); + free(thr); + thr = NULL; + pthread_mutex_lock(&pool->m); + pool->count--; + pthread_mutex_unlock(&pool->m); + } } return thr; @@ -213,6 +227,7 @@ threadpool_dispatch(struct threadpool *pool, struct resultq *rq = rh->rq; struct thread *thr = threadpool_next(pool); + assert(thr != NULL); assert(!thr->running); assert(thr->next == NULL); @@ -378,11 +393,19 @@ struct result_handler * result_handler_init(result_cb cb, void *cbdata) { struct result_handler *rh = calloc(1, sizeof(*rh)); + int ret; rh->rq = resultq_init(); rh->cb = cb; rh->cbdata = cbdata; - pthread_create(&rh->thread, NULL, result_worker, rh); + + ret = pthread_create(&rh->thread, NULL, result_worker, rh); + if (ret != 0) { + fprintf(stderr, "%s: pthread_create() failed: %s\n", __func__, strerror(ret)); + resultq_destroy(&rh->rq); + free(rh); + return (NULL); + } return rh; } diff --git a/mtbl/writer.c b/mtbl/writer.c index 6b02825..6a8dcc5 100644 --- a/mtbl/writer.c +++ b/mtbl/writer.c @@ -185,6 +185,11 @@ mtbl_writer_init_fd(int orig_fd, const struct mtbl_writer_options *opt) if (w->opt.pool != NULL) { w->pool = w->opt.pool->pool; w->rhandler = result_handler_init(_write_data_block_wrapper, w); + if (w->rhandler == NULL) { + close(fd); + free(w); + return (NULL); + } } return (w); From e571c81d2d9a52c8857f878b7f9d6f15d5a83016 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 07:45:30 +0000 Subject: [PATCH 05/12] report error and exit when user merge function returns NULL --- src/mtbl_merge.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/mtbl_merge.c b/src/mtbl_merge.c index 9e46587..62dc34b 100644 --- a/src/mtbl_merge.c +++ b/src/mtbl_merge.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2026 DomainTools LLC * Copyright (c) 2012, 2014-2016, 2019, 2021 by Farsight Security, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -135,6 +136,13 @@ print_stats(void) ); } +static void +fprint_hex(FILE *f, const uint8_t *buf, size_t len) +{ + for (size_t i = 0; i < len; i++) + fprintf(f, "%02x", buf[i]); +} + static void merge_func(void *clos, const uint8_t *key, size_t len_key, @@ -147,6 +155,23 @@ merge_func(void *clos, val0, len_val0, val1, len_val1, merged_val, len_merged_val); + + /* + * A NULL *merged_val indicates a merge failure and causes the merger to return mtbl_res_failure. + * This is indistinguishable from end of data at the call site in merge(), so we have to catch it here. + */ + if (*merged_val == NULL) { + fprintf(stderr, "%s: merge function returned NULL\n", program_name); + fprintf(stderr, " key (%zu bytes): ", len_key); + fprint_hex(stderr, key, len_key); + fprintf(stderr, "\n val0 (%zu bytes): ", len_val0); + fprint_hex(stderr, val0, len_val0); + fprintf(stderr, "\n val1 (%zu bytes): ", len_val1); + fprint_hex(stderr, val1, len_val1); + fputc('\n', stderr); + exit(EXIT_FAILURE); + } + count_merged += 1; } From 3544ba8a3f09f37b30b61954c2424786c7da1281 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 07:45:30 +0000 Subject: [PATCH 06/12] fix output stream, argument parsing, and bounds check in CLI tools --- src/mtbl_dump.c | 35 ++++++++++++++++------------------- src/mtbl_verify.c | 10 +++++++++- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/mtbl_dump.c b/src/mtbl_dump.c index a46e005..944664a 100644 --- a/src/mtbl_dump.c +++ b/src/mtbl_dump.c @@ -1,4 +1,5 @@ /* + * Copyright (c) 2026 DomainTools LLC * Copyright (c) 2012, 2014-2015, 2021 by Farsight Security, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -36,7 +37,7 @@ static void print_hex_string(const void *data, size_t len, FILE *out) unsigned c = *(str++); fprintf(out, "%02x", c); if (len > 0) - fputc('-', stdout); + fputc('-', out); } } @@ -142,31 +143,27 @@ main(int argc, char **argv) } break; case 'K': - if (strlen(optarg) == 0) { - fprintf(stderr, "Need a non-empty argument to -K\n"); - return (EXIT_FAILURE); - } - - key_min_len = atoi(optarg); - - if (key_min_len < 1) { - fprintf(stderr, "Bad value of minimum key length: %s\n", optarg); + { + char *endptr; + long val = strtol(optarg, &endptr, 10); + if (endptr == optarg || *endptr != '\0' || val < 1) { + fprintf(stderr, "Invalid minimum key length: %s\n", optarg); return (EXIT_FAILURE); } + key_min_len = (size_t) val; break; + } case 'V': - if (strlen(optarg) == 0) { - fprintf(stderr, "Need a non-empty argument to -K\n"); - return (EXIT_FAILURE); - } - - val_min_len = atoi(optarg); - - if (val_min_len < 1) { - fprintf(stderr, "Bad value of minimum val length: %s\n", optarg); + { + char *endptr; + long val = strtol(optarg, &endptr, 10); + if (endptr == optarg || *endptr != '\0' || val < 1) { + fprintf(stderr, "Invalid minimum val length: %s\n", optarg); return (EXIT_FAILURE); } + val_min_len = (size_t) val; break; + } default: usage(); } diff --git a/src/mtbl_verify.c b/src/mtbl_verify.c index 6be8dd3..b0a5d47 100644 --- a/src/mtbl_verify.c +++ b/src/mtbl_verify.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 DomainTools LLC + * Copyright (c) 2022, 2026 DomainTools LLC * Copyright (c) 2015, 2017, 2019 by Farsight Security, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -191,6 +191,14 @@ verify_file(const char *fname) uint64_t bytes_data_blocks = mtbl_metadata_bytes_data_blocks(m); uint64_t index_offset = mtbl_metadata_index_block_offset(m); + if (bytes_data_blocks > index_offset) { + fprintf(stderr, "%s: metadata: bytes_data_blocks (%" PRIu64 ") > index_block_offset (%" PRIu64 ")\n", + fname, bytes_data_blocks, index_offset); + mtbl_reader_destroy(&r); + close(fd); + return false; + } + uint64_t data_offset = index_offset - bytes_data_blocks; if (verify_data_blocks(fd, fname, data_offset, bytes_data_blocks, count_data_blocks, mtbl_metadata_file_version(m))) { From 4d640bb886513ed1477c26d61a0b35255c880924 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 08:02:43 +0000 Subject: [PATCH 07/12] detect early termination in mtbl_merge --- .gitignore | 2 ++ src/mtbl_dump.c | 12 ++++++++++++ src/mtbl_merge.c | 13 +++++++++++++ 3 files changed, 27 insertions(+) diff --git a/.gitignore b/.gitignore index 286997f..ecaef8d 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,5 @@ Makefile.in report.coverage TAGS t/test-merge.sh +cscope.out +tags diff --git a/src/mtbl_dump.c b/src/mtbl_dump.c index 944664a..f878652 100644 --- a/src/mtbl_dump.c +++ b/src/mtbl_dump.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include "libmy/print_string.h" @@ -50,6 +51,7 @@ dump(const char *fname, const bool silent, bool hex, { const uint8_t *key, *val; size_t key_len, val_len; + uint64_t count = 0, expected; struct mtbl_reader *r; struct mtbl_iter *it; @@ -59,8 +61,10 @@ dump(const char *fname, const bool silent, bool hex, return (false); } + expected = mtbl_metadata_count_entries(mtbl_reader_metadata(r)); it = mtbl_source_iter(mtbl_reader_source(r)); while (mtbl_iter_next(it, &key, &key_len, &val, &val_len)) { + count++; if (key_prefix != 0 && (key_len < key_prefix_len || 0 != bcmp(key, key_prefix, key_prefix_len))) @@ -87,6 +91,14 @@ dump(const char *fname, const bool silent, bool hex, } mtbl_iter_destroy(&it); + + if (count != expected) { + fprintf(stderr, "%s: error: read %" PRIu64 " of %" PRIu64 " expected entries;" + " file may be truncated or corrupt\n", fname, count, expected); + mtbl_reader_destroy(&r); + return (false); + } + mtbl_reader_destroy(&r); return (true); diff --git a/src/mtbl_merge.c b/src/mtbl_merge.c index 62dc34b..922a220 100644 --- a/src/mtbl_merge.c +++ b/src/mtbl_merge.c @@ -62,6 +62,7 @@ static struct mtbl_writer *writer; static struct timespec start_time; static uint64_t count; static uint64_t count_merged; +static uint64_t total_input_entries; static void usage(void) @@ -193,6 +194,17 @@ merge(void) mtbl_iter_destroy(&it); mtbl_merger_destroy(&merger); mtbl_writer_destroy(&writer); + + /* + * mtbl_iter_next() returns the same value for end-of-data and error, so we check that the count equals the + * sum of all input entries minus count_merged. Any shortfall means iteration stopped early. + */ + uint64_t expected = total_input_entries - count_merged; + if (count != expected) { + fprintf(stderr, "%s: error: wrote %" PRIu64 " of %" PRIu64 " expected entries; input may be truncated or corrupt\n", + program_name, count, expected); + exit(EXIT_FAILURE); + } } static void @@ -448,6 +460,7 @@ main(int argc, char **argv) usage(); } mtbl_merger_add_source(merger, mtbl_reader_source(readers[i])); + total_input_entries += mtbl_metadata_count_entries(mtbl_reader_metadata(readers[i])); } /* do merge */ From be8cb715de5c94cfcca98072a2055299e2c4fe07 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Fri, 14 Aug 2026 08:32:02 +0000 Subject: [PATCH 08/12] add error messages to mtbl_reader_init and mtbl_merge on open failure --- mtbl/reader.c | 10 ++++++++-- src/mtbl_merge.c | 5 +++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/mtbl/reader.c b/mtbl/reader.c index 10ab03f..e36510c 100644 --- a/mtbl/reader.c +++ b/mtbl/reader.c @@ -224,11 +224,17 @@ mtbl_reader_init(const char *fname, const struct mtbl_reader_options *opt) int fd; fd = open(fname, O_RDONLY); - if (fd < 0) + if (fd < 0) { + fprintf(stderr, "%s: ERROR: failed to open(2) '%s': %s\n", __func__, fname, strerror(errno)); return (NULL); + } + r = mtbl_reader_init_fd(fd, opt); - close(fd); + if (r == NULL) { + fprintf(stderr, "%s: ERROR: failed to init mtbl fd for '%s'\n", __func__, fname); + } + close(fd); return (r); } diff --git a/src/mtbl_merge.c b/src/mtbl_merge.c index 922a220..064352c 100644 --- a/src/mtbl_merge.c +++ b/src/mtbl_merge.c @@ -456,8 +456,9 @@ main(int argc, char **argv) fprintf(stderr, "%s: opening input file %s\n", program_name, fname); readers[i] = mtbl_reader_init(fname, NULL); if (readers[i] == NULL) { - fprintf(stderr, "Error: mtbl_reader_init() failed.\n\n"); - usage(); + fprintf(stderr, "%s: ERROR: mtbl_reader_init() failed to open file %s\n\n", + program_name, fname); + exit(EXIT_FAILURE); } mtbl_merger_add_source(merger, mtbl_reader_source(readers[i])); total_input_entries += mtbl_metadata_count_entries(mtbl_reader_metadata(readers[i])); From be5cc476d3087b77656eec08da317ee7a59b1edf Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Sat, 15 Aug 2026 13:31:21 -0700 Subject: [PATCH 09/12] test-merge.sh should exit, not return --- t/test-merge.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-merge.sh.in b/t/test-merge.sh.in index e4020d8..2ca8c1f 100755 --- a/t/test-merge.sh.in +++ b/t/test-merge.sh.in @@ -129,4 +129,4 @@ cmp -s $tmp_data $full_data print_result_of_last_cmd rm -f $tmp_data -return $exitcode +exit $exitcode From df70249d56e1a395e215dc92d267ee1bdd669a40 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Sat, 15 Aug 2026 20:44:37 +0000 Subject: [PATCH 10/12] fix mem leak in _mtbl_sorter_write_chunk() --- mtbl/sorter.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mtbl/sorter.c b/mtbl/sorter.c index c9bf34d..33afd40 100644 --- a/mtbl/sorter.c +++ b/mtbl/sorter.c @@ -217,6 +217,7 @@ _mtbl_sorter_write_chunk(struct entry_batch *b) struct mtbl_writer *w = mtbl_writer_init_fd(fd, wopt); if (w == NULL) { + mtbl_writer_options_destroy(&wopt); close(fd); entry_vec_destroy(&b->entries); free(b); From bdfbc95459ca734e2a72b3a78817bf0a32b09fe3 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Wed, 19 Aug 2026 15:10:15 -0700 Subject: [PATCH 11/12] revert s/assert/exit changes --- mtbl/compression.c | 15 ++------------- mtbl/reader.c | 7 +++---- mtbl/sorter.c | 27 ++++----------------------- mtbl/threadpool.c | 24 +++++------------------- mtbl/writer.c | 20 ++++---------------- src/mtbl_merge.c | 5 +++++ 6 files changed, 23 insertions(+), 75 deletions(-) diff --git a/mtbl/compression.c b/mtbl/compression.c index 9f27b81..5770195 100644 --- a/mtbl/compression.c +++ b/mtbl/compression.c @@ -482,14 +482,8 @@ _mtbl_decompress_zlib( do { zret = inflate(&zs, Z_FINISH); - if (zret != Z_STREAM_END && zret != Z_BUF_ERROR) { - goto fail; - } - + assert(zret == Z_STREAM_END || zret == Z_BUF_ERROR); if (zret != Z_STREAM_END) { - if (*output_size > SIZE_MAX / 2) { - goto fail; - } *output = my_realloc(*output, *output_size * 2); zs.next_out = *output + *output_size; zs.avail_out = *output_size; @@ -499,11 +493,6 @@ _mtbl_decompress_zlib( *output_size = zs.total_out; inflateEnd(&zs); + return (mtbl_res_success); -fail: - free(*output); - *output = NULL; - *output_size = 0; - inflateEnd(&zs); - return (mtbl_res_failure); } diff --git a/mtbl/reader.c b/mtbl/reader.c index e36510c..f704de9 100644 --- a/mtbl/reader.c +++ b/mtbl/reader.c @@ -461,7 +461,7 @@ reader_iter_seek(void *v, const uint8_t *key, size_t len_key) { struct reader_iter *it = (struct reader_iter *) v; - + const uint8_t *ikey, *ival; size_t len_ikey, len_ival; uint64_t new_offset; @@ -481,7 +481,7 @@ reader_iter_seek(void *v, mtbl_varint_decode64(ival, &new_offset); /* We can skip decoding a new block if our new key is within the - * currently-decoded block. */ + * currently-decoded block. */ if (it->b == NULL || it->block_offset != new_offset) { block_destroy(&it->b); block_iter_destroy(&it->bi); @@ -523,8 +523,7 @@ reader_iter_next(void *v, return (mtbl_res_failure); it->b = get_block_at_index(it->r, it->index_iter); - if (it->b == NULL) - return (mtbl_res_failure); + assert(it->b != NULL); it->bi = block_iter_init(it->b); block_iter_seek_to_first(it->bi); diff --git a/mtbl/sorter.c b/mtbl/sorter.c index 33afd40..f8ee6ce 100644 --- a/mtbl/sorter.c +++ b/mtbl/sorter.c @@ -140,11 +140,7 @@ mtbl_sorter_init(const struct mtbl_sorter_options *opt) if (s->opt.pool != NULL) { s->pool = s->opt.pool->pool; s->rhandler = result_handler_init(_collect_readers_cb, s); - if (s->rhandler == NULL) { - entry_vec_destroy(&s->vec); - reader_vec_destroy(&s->readers); - return (NULL); - } + assert(s->rhandler != NULL); } return (s); @@ -198,31 +194,16 @@ _mtbl_sorter_write_chunk(struct entry_batch *b) ubuf_append(tmp_fname, (const uint8_t *) "\x00", 1); int fd = mkstemp((char *) ubuf_data(tmp_fname)); - if (fd < 0) { - ubuf_destroy(&tmp_fname); - return (NULL); - } - + assert(fd >= 0); int unlink_ret = unlink((char *) ubuf_data(tmp_fname)); - if (unlink_ret == -1) { - ubuf_destroy(&tmp_fname); - close(fd); - return (NULL); - } - + assert(unlink_ret == 0); ubuf_destroy(&tmp_fname); struct mtbl_writer_options *wopt = mtbl_writer_options_init(); mtbl_writer_options_set_compression(wopt, MTBL_COMPRESSION_SNAPPY); struct mtbl_writer *w = mtbl_writer_init_fd(fd, wopt); - if (w == NULL) { - mtbl_writer_options_destroy(&wopt); - close(fd); - entry_vec_destroy(&b->entries); - free(b); - return (NULL); - } + assert(w != NULL); mtbl_writer_options_destroy(&wopt); diff --git a/mtbl/threadpool.c b/mtbl/threadpool.c index b2c419c..72fd0ad 100644 --- a/mtbl/threadpool.c +++ b/mtbl/threadpool.c @@ -15,11 +15,9 @@ */ #include -#include #include #include #include -#include #include #include "threadpool.h" @@ -195,16 +193,7 @@ threadpool_next(struct threadpool *pool) pthread_cond_init(&thr->c, NULL); ret = pthread_create(&thr->t, NULL, thread_worker, thr); - if (ret != 0) { - fprintf(stderr, "%s: pthread_create() failed: %s\n", __func__, strerror(ret)); - pthread_cond_destroy(&thr->c); - pthread_mutex_destroy(&thr->m); - free(thr); - thr = NULL; - pthread_mutex_lock(&pool->m); - pool->count--; - pthread_mutex_unlock(&pool->m); - } + assert(ret == 0); } return thr; @@ -400,12 +389,7 @@ result_handler_init(result_cb cb, void *cbdata) rh->cbdata = cbdata; ret = pthread_create(&rh->thread, NULL, result_worker, rh); - if (ret != 0) { - fprintf(stderr, "%s: pthread_create() failed: %s\n", __func__, strerror(ret)); - resultq_destroy(&rh->rq); - free(rh); - return (NULL); - } + assert(ret == 0); return rh; } @@ -414,7 +398,9 @@ void result_handler_destroy(struct result_handler **prh) { struct result_handler *rh = *prh; - if (rh == NULL) return; + if (rh == NULL) + return; + resultq_finish(rh->rq); pthread_join(rh->thread, NULL); free(rh); diff --git a/mtbl/writer.c b/mtbl/writer.c index 6a8dcc5..c8f9208 100644 --- a/mtbl/writer.c +++ b/mtbl/writer.c @@ -146,10 +146,7 @@ mtbl_writer_init_fd(int orig_fd, const struct mtbl_writer_options *opt) int fd; fd = dup(orig_fd); - if (fd < 0) { - return (NULL); - } - + assert(fd >= 0); w = my_calloc(1, sizeof(*w)); if (opt == NULL) { w->opt.compression_type = DEFAULT_COMPRESSION_TYPE; @@ -166,14 +163,9 @@ mtbl_writer_init_fd(int orig_fd, const struct mtbl_writer_options *opt) * to reserve some initial bytes in the file. */ off_t offset = lseek(fd, 0, SEEK_CUR); - if (offset == (off_t)-1) { - close(fd); - free(w); - return (NULL); - } - w->last_offset = (uint64_t)offset; + assert(offset != (off_t)-1); - w->pending_offset = w->last_offset; + w->pending_offset = (uint64_t)offset; w->last_key = ubuf_init(256); w->m.file_version = MTBL_FORMAT_V2; w->m.compression_algorithm = w->opt.compression_type; @@ -185,11 +177,7 @@ mtbl_writer_init_fd(int orig_fd, const struct mtbl_writer_options *opt) if (w->opt.pool != NULL) { w->pool = w->opt.pool->pool; w->rhandler = result_handler_init(_write_data_block_wrapper, w); - if (w->rhandler == NULL) { - close(fd); - free(w); - return (NULL); - } + assert(w->rhandler != NULL); } return (w); diff --git a/src/mtbl_merge.c b/src/mtbl_merge.c index 064352c..07df51f 100644 --- a/src/mtbl_merge.c +++ b/src/mtbl_merge.c @@ -199,6 +199,11 @@ merge(void) * mtbl_iter_next() returns the same value for end-of-data and error, so we check that the count equals the * sum of all input entries minus count_merged. Any shortfall means iteration stopped early. */ + if (count_merged > total_input_entries) { + fprintf(stderr, "%s: error: count_merged (%" PRIu64 ") exceeds total_input_entries (%" PRIu64 ")\n", + program_name, count_merged, total_input_entries); + exit(EXIT_FAILURE); + } uint64_t expected = total_input_entries - count_merged; if (count != expected) { fprintf(stderr, "%s: error: wrote %" PRIu64 " of %" PRIu64 " expected entries; input may be truncated or corrupt\n", From bb9fa3d6a1090cec924d9e7580da1de6f6cebc51 Mon Sep 17 00:00:00 2001 From: Rafael Vanoni Date: Wed, 19 Aug 2026 16:11:21 -0700 Subject: [PATCH 12/12] remove fprintf(3) calls in mtbl_reader_init --- mtbl/reader.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mtbl/reader.c b/mtbl/reader.c index f704de9..ff0050d 100644 --- a/mtbl/reader.c +++ b/mtbl/reader.c @@ -225,14 +225,10 @@ mtbl_reader_init(const char *fname, const struct mtbl_reader_options *opt) fd = open(fname, O_RDONLY); if (fd < 0) { - fprintf(stderr, "%s: ERROR: failed to open(2) '%s': %s\n", __func__, fname, strerror(errno)); return (NULL); } r = mtbl_reader_init_fd(fd, opt); - if (r == NULL) { - fprintf(stderr, "%s: ERROR: failed to init mtbl fd for '%s'\n", __func__, fname); - } close(fd); return (r);