]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib-storage: Fix vsize-hdr corruption bug
authorMichael M Slusarz <michael.slusarz@open-xchange.com>
Thu, 28 May 2026 16:45:05 +0000 (10:45 -0600)
committerMichael M Slusarz <michael.slusarz@open-xchange.com>
Fri, 29 May 2026 16:01:45 +0000 (10:01 -0600)
If the vsize-hdr in the mail index becomes corrupted (i.e., has a
size that is not zero but is incorrect, or a message-count that is
too low), the system fails to automatically correct it. The function
responsible for updating this header for new emails,
index_mailbox_vsize_update_appends(), misinterprets a corrupted header
as a non-existent one and intentionally avoids creating it. This
leads to a persistent state of corruption, causing incorrect reports
of mailbox size until a manual, full recalculation is triggered.

Add a hdr_corrupted field to the update struct, set when either
vsize_header_refresh() detects an invalid header size or
index_mailbox_vsize_check_rebuild() detects an invalid message
count.

The rebuild guards in index_mailbox_get_virtual_size() and
index_mailbox_vsize_update_appends() (added in the preceding
commit) ensure the rebuild path is always taken when the header
is in an invalid state, preventing stale data from being returned
or lock acquisition being skipped due to edge-case uidnext values.

src/lib-storage/Makefile.am
src/lib-storage/index/Makefile.am
src/lib-storage/index/index-mailbox-size.c
src/lib-storage/index/test-index.c [new file with mode: 0644]

index b8c33634e1586c99c45aa5b7622e688236642255..5b6262b2b9d786e6363e84a2555b762b5d117f81 100644 (file)
@@ -1,5 +1,7 @@
 include $(top_srcdir)/Makefile.test.include
 
+AUTOMAKE_OPTIONS = subdir-objects
+
 SUBDIRS = list index
 
 noinst_LTLIBRARIES = libstorage.la
@@ -135,7 +137,8 @@ test_programs = \
        test-mail \
        test-mail-storage \
        test-mailbox-get \
-       test-mailbox-list
+       test-mailbox-list \
+       test-index
 
 test_libs = \
        $(top_builddir)/src/lib-var-expand/libvar_expand.la \
@@ -166,6 +169,10 @@ test_mailbox_list_SOURCES = test-mailbox-list.c
 test_mailbox_list_LDADD = libstorage.la $(LIBDOVECOT)
 test_mailbox_list_DEPENDENCIES = libstorage.la $(LIBDOVECOT_DEPS)
 
+test_index_SOURCES = index/test-index.c
+test_index_LDADD = libstorage.la $(LIBDOVECOT)
+test_index_DEPENDENCIES = libstorage.la $(LIBDOVECOT_DEPS)
+
 pkginc_libdir=$(pkgincludedir)
 pkginc_lib_HEADERS = $(headers)
 noinst_HEADERS = $(test_headers)
index 4593f10defac362a47b788aaae25a1260fb5cfdb..7f3080e8e2118c817a59e434c6526618d0408ffc 100644 (file)
@@ -57,5 +57,10 @@ headers = \
        index-sync-private.h \
        index-thread-private.h
 
+# test-index is built in the parent directory (src/lib-storage/Makefile.am)
+# because it depends on libstorage.la, and building it here causes
+# circular dependencies or ordering issues during parallel make.
+test_programs =
+
 pkginc_libdir=$(pkgincludedir)
 pkginc_lib_HEADERS = $(headers)
index aaa266da0f02e987e27987c8c6df61fbee1a4ba7..f468ca62547f092fd618ae3972fbf28097053614 100644 (file)
@@ -48,6 +48,7 @@ struct mailbox_vsize_update {
        bool lock_failed;
        bool skip_write;
        bool rebuild;
+       bool hdr_corrupted;
        bool written;
        bool finish_in_background;
 };
@@ -75,6 +76,7 @@ static void vsize_header_refresh(struct mailbox_vsize_update *update)
                        mailbox_set_critical(update->box,
                                "vsize-hdr has invalid size: %zu",
                                size);
+                       update->hdr_corrupted = TRUE;
                }
                update->rebuild = TRUE;
                i_zero(&update->vsize_hdr);
@@ -98,6 +100,7 @@ index_mailbox_vsize_check_rebuild(struct mailbox_vsize_update *update)
                        mailbox_set_critical(update->box,
                                "vsize-hdr has invalid message-count (%u < %u)",
                                update->vsize_hdr.message_count, seq2);
+                       update->hdr_corrupted = TRUE;
                } else {
                        /* some messages have been expunged, rescan */
                }
@@ -162,7 +165,8 @@ bool index_mailbox_vsize_update_wait_lock(struct mailbox_vsize_update *update)
 
 bool index_mailbox_vsize_want_updates(struct mailbox_vsize_update *update)
 {
-       return update->vsize_hdr.highest_uid > 0;
+       return update->hdr_corrupted ||
+               update->vsize_hdr.highest_uid > 0;
 }
 
 static void
@@ -172,6 +176,9 @@ index_mailbox_vsize_update_write_to_index(struct mailbox_vsize_update *update)
 
        trans = mail_index_transaction_begin(update->view,
                                MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL);
+       if (update->hdr_corrupted)
+               mail_index_ext_resize_hdr(trans, update->box->vsize_hdr_ext_id,
+                                 sizeof(update->vsize_hdr));
        mail_index_update_header_ext(trans, update->box->vsize_hdr_ext_id,
                                     0, &update->vsize_hdr,
                                     sizeof(update->vsize_hdr));
@@ -476,7 +483,7 @@ void index_mailbox_vsize_update_appends(struct mailbox *box)
        struct mailbox_status status;
 
        update = index_mailbox_vsize_update_init(box);
-       if (update->rebuild) {
+       if (update->rebuild && !update->hdr_corrupted) {
                /* The vsize header doesn't exist. Don't create it. */
                update->skip_write = TRUE;
        }
diff --git a/src/lib-storage/index/test-index.c b/src/lib-storage/index/test-index.c
new file mode 100644 (file)
index 0000000..84e532a
--- /dev/null
@@ -0,0 +1,219 @@
+/* Copyright Dovecot authors, see the included COPYING file */
+
+#include "lib.h"
+#include "ioloop.h"
+#include "istream.h"
+#include "test-common.h"
+#include "test-dir.h"
+#include "master-service.h"
+#include "test-mail-storage-common.h"
+#include "mail-storage-private.h"
+#include "index/index-storage.h"
+#include "index/index-mailbox-size.h"
+
+static void test_mail_save(struct mailbox *box, const char *mail_input)
+{
+       struct mailbox_transaction_context *trans;
+       struct mail_save_context *save_ctx;
+       struct istream *input;
+       int ret;
+
+       input = i_stream_create_from_data(mail_input, strlen(mail_input));
+       trans = mailbox_transaction_begin(box,
+                       MAILBOX_TRANSACTION_FLAG_EXTERNAL, __func__);
+       save_ctx = mailbox_save_alloc(trans);
+       test_assert(mailbox_save_begin(&save_ctx, input) == 0);
+       while ((ret = i_stream_read(input)) > 0) ;
+       test_assert(ret == -1);
+       test_assert(mailbox_save_finish(&save_ctx) == 0);
+       i_stream_unref(&input);
+       test_assert(mailbox_transaction_commit(&trans) == 0);
+       test_assert(mailbox_sync(box, 0) == 0);
+}
+
+static void test_vsize_hdr_corruption_fix(void)
+{
+       struct test_mail_storage_ctx *ctx;
+       const struct mail_namespace *ns;
+       struct mailbox *box;
+       struct mail_index_view *view;
+       const void *data;
+       size_t size;
+
+       test_begin("vsize header corruption fix");
+
+       ctx = test_mail_storage_init();
+       const struct test_mail_storage_settings set = {
+               .driver = "maildir",
+               .hierarchy_sep = "/",
+       };
+       test_mail_storage_init_user(ctx, &set);
+
+       ns = mail_namespace_find_inbox(ctx->user->namespaces);
+       box = mailbox_alloc(ns->list, "vsize-test", 0);
+       test_assert(mailbox_create(box, NULL, FALSE) == 0);
+       test_assert(mailbox_open(box) == 0);
+       test_assert(mailbox_sync(box, MAILBOX_SYNC_FLAG_FULL_READ) == 0);
+
+       /* write corrupted header */
+       struct mail_index_transaction *trans =
+               mail_index_transaction_begin(box->view,
+                                            MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL);
+       char corrupted_hdr[44];
+       i_zero(&corrupted_hdr);
+       memcpy(corrupted_hdr, "corrupted", 9);
+       mail_index_update_header_ext(trans, box->vsize_hdr_ext_id, 0,
+                                    corrupted_hdr, sizeof(corrupted_hdr));
+       test_assert(mail_index_transaction_commit(&trans) == 0);
+
+       /* close and reopen the box to trigger the fix */
+       test_expect_errors(2);
+       mailbox_free(&box);
+       box = mailbox_alloc(ns->list, "vsize-test", 0);
+       test_assert(mailbox_open(box) == 0);
+       index_mailbox_vsize_update_appends(box);
+       mailbox_free(&box);
+
+       /* reopen one last time to verify fix on disk */
+       box = mailbox_alloc(ns->list, "vsize-test", 0);
+       test_assert(mailbox_open(box) == 0);
+
+       /* verify the fix */
+       (void)mail_index_refresh(box->index);
+       view = mail_index_view_open(box->index);
+       mail_index_get_header_ext(view, box->vsize_hdr_ext_id, &data, &size);
+
+       test_assert(size == sizeof(struct mailbox_index_vsize));
+       struct mailbox_index_vsize empty_hdr;
+       i_zero(&empty_hdr);
+       test_assert(memcmp(data, &empty_hdr, sizeof(empty_hdr)) == 0);
+
+       mail_index_view_close(&view);
+       mailbox_free(&box);
+       test_mail_storage_deinit_user(ctx);
+       test_mail_storage_deinit(&ctx);
+
+       test_end();
+}
+
+static void test_vsize_hdr_msg_count_corruption_fix(void)
+{
+       struct test_mail_storage_ctx *ctx;
+       const struct mail_namespace *ns;
+       struct mailbox *box;
+       struct mail_index_view *view;
+       const void *data;
+       size_t size;
+       uint32_t uid2;
+
+       test_begin("vsize header message count corruption fix");
+
+       ctx = test_mail_storage_init();
+       const struct test_mail_storage_settings set = {
+               .driver = "maildir",
+               .hierarchy_sep = "/",
+       };
+       test_mail_storage_init_user(ctx, &set);
+
+       ns = mail_namespace_find_inbox(ctx->user->namespaces);
+       box = mailbox_alloc(ns->list, "vsize-msg-count-test", 0);
+       test_assert(mailbox_create(box, NULL, FALSE) == 0);
+       test_assert(mailbox_open(box) == 0);
+       test_assert(mailbox_sync(box, MAILBOX_SYNC_FLAG_FULL_READ) == 0);
+
+       test_mail_save(box, "From: foo\n\nbar\n");
+       test_mail_save(box, "From: bar\n\nbaz\n");
+
+       struct mailbox_status status;
+       mailbox_get_open_status(box, STATUS_UIDNEXT, &status);
+       uid2 = status.uidnext - 1;
+
+       /* Create a valid vsize header with message_count == 2 */
+       {
+               struct mail_index_transaction *trans;
+               struct mailbox_index_vsize valid_hdr;
+               i_zero(&valid_hdr);
+               valid_hdr.vsize = 100;
+               valid_hdr.highest_uid = uid2;
+               valid_hdr.message_count = 2;
+               trans = mail_index_transaction_begin(box->view,
+                                            MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL);
+               mail_index_update_header_ext(trans, box->vsize_hdr_ext_id, 0,
+                                            &valid_hdr, sizeof(valid_hdr));
+               test_assert(mail_index_transaction_commit(&trans) == 0);
+       }
+
+       /* Verify we have 2 messages in vsize header */
+       (void)mail_index_refresh(box->index);
+       view = mail_index_view_open(box->index);
+       mail_index_get_header_ext(view, box->vsize_hdr_ext_id, &data, &size);
+       test_assert(size == sizeof(struct mailbox_index_vsize));
+       const struct mailbox_index_vsize *vsize_hdr = data;
+       test_assert(vsize_hdr->message_count == 2);
+       test_assert(vsize_hdr->highest_uid == uid2);
+       mail_index_view_close(&view);
+
+       /* Write corrupted header: message_count = 1, but highest_uid is still uid2 */
+       struct mail_index_transaction *trans =
+               mail_index_transaction_begin(box->view,
+                                            MAIL_INDEX_TRANSACTION_FLAG_EXTERNAL);
+       struct mailbox_index_vsize corrupted_vsize;
+       corrupted_vsize.vsize = 100; /* dummy */
+       corrupted_vsize.highest_uid = uid2;
+       corrupted_vsize.message_count = 1;
+       mail_index_update_header_ext(trans, box->vsize_hdr_ext_id, 0,
+                                    &corrupted_vsize, sizeof(corrupted_vsize));
+       test_assert(mail_index_transaction_commit(&trans) == 0);
+
+       /* Close and reopen to trigger the fix in index_mailbox_vsize_check_rebuild */
+       test_expect_errors(2); /* vsize-hdr has invalid message-count (1 < 2) twice */
+       mailbox_free(&box);
+       box = mailbox_alloc(ns->list, "vsize-msg-count-test", 0);
+       test_assert(mailbox_open(box) == 0);
+       index_mailbox_vsize_update_appends(box);
+       mailbox_free(&box);
+
+       /* Reopen one last time to verify fix on disk */
+       box = mailbox_alloc(ns->list, "vsize-msg-count-test", 0);
+       test_assert(mailbox_open(box) == 0);
+
+       /* verify the fix */
+       (void)mail_index_refresh(box->index);
+       view = mail_index_view_open(box->index);
+       mail_index_get_header_ext(view, box->vsize_hdr_ext_id, &data, &size);
+
+       test_assert(size == sizeof(struct mailbox_index_vsize));
+       vsize_hdr = data;
+       test_assert(vsize_hdr->message_count == 2);
+       test_assert(vsize_hdr->highest_uid == uid2);
+
+       mail_index_view_close(&view);
+       mailbox_free(&box);
+       test_mail_storage_deinit_user(ctx);
+       test_mail_storage_deinit(&ctx);
+
+       test_end();
+}
+
+int main(int argc, char *argv[])
+{
+       static void (* const test_functions[])(void) = {
+               test_vsize_hdr_corruption_fix,
+               test_vsize_hdr_msg_count_corruption_fix,
+               NULL
+       };
+
+       master_service = master_service_init("test-index",
+                                            MASTER_SERVICE_FLAG_STANDALONE |
+                                            MASTER_SERVICE_FLAG_DONT_SEND_STATS |
+                                            MASTER_SERVICE_FLAG_CONFIG_BUILTIN |
+                                            MASTER_SERVICE_FLAG_NO_SSL_INIT |
+                                            MASTER_SERVICE_FLAG_NO_INIT_DATASTACK_FRAME,
+                                            &argc, &argv, "");
+       test_dir_init("test-index");
+       int ret = test_run(test_functions);
+
+       master_service_deinit(&master_service);
+
+       return ret;
+}