From: Timo Sirainen Date: Mon, 28 Nov 2016 11:52:40 +0000 (+0200) Subject: lib-index: mail_transaction_log_file_sync(): Don't mix I/O errors and corruption X-Git-Tag: 2.2.27~30 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2d4bf7547754e325caf2ce0c10e8340ff754ec3d;p=thirdparty%2Fdovecot%2Fcore.git lib-index: mail_transaction_log_file_sync(): Don't mix I/O errors and corruption --- diff --git a/src/lib-index/mail-transaction-log-file.c b/src/lib-index/mail-transaction-log-file.c index 92c7496ade..5acb62165c 100644 --- a/src/lib-index/mail-transaction-log-file.c +++ b/src/lib-index/mail-transaction-log-file.c @@ -17,7 +17,8 @@ #define LOG_NEW_DOTLOCK_SUFFIX ".newlock" static int -mail_transaction_log_file_sync(struct mail_transaction_log_file *file); +mail_transaction_log_file_sync(struct mail_transaction_log_file *file, + bool *retry_r); static void log_file_set_syscall_error(struct mail_transaction_log_file *file, @@ -189,6 +190,7 @@ static void mail_transaction_log_file_add_to_list(struct mail_transaction_log_file *file) { struct mail_transaction_log_file **p; + bool retry; file->sync_offset = file->hdr.hdr_size; file->sync_highest_modseq = file->hdr.initial_modseq; @@ -207,7 +209,7 @@ mail_transaction_log_file_add_to_list(struct mail_transaction_log_file *file) if (file->buffer != NULL) { /* if we read any unfinished data, make sure the buffer gets truncated. */ - (void)mail_transaction_log_file_sync(file); + (void)mail_transaction_log_file_sync(file, &retry); buffer_set_used_size(file->buffer, file->sync_offset - file->buffer_offset); } @@ -1356,7 +1358,8 @@ log_file_track_sync(struct mail_transaction_log_file *file, } static int -mail_transaction_log_file_sync(struct mail_transaction_log_file *file) +mail_transaction_log_file_sync(struct mail_transaction_log_file *file, + bool *retry_r) { const struct mail_transaction_header *hdr; const void *data; @@ -1367,6 +1370,8 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) i_assert(file->sync_offset >= file->buffer_offset); + *retry_r = FALSE; + data = buffer_get_data(file->buffer, &size); if (file->buffer_offset + size < file->sync_offset) { mail_transaction_log_file_set_corrupted(file, @@ -1374,7 +1379,7 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) file->buffer_offset + (uoff_t)size, file->sync_offset); /* fix the sync_offset to avoid crashes later on */ file->sync_offset = file->buffer_offset + size; - return -1; + return 0; } while (file->sync_offset - file->buffer_offset + sizeof(*hdr) <= size) { hdr = CONST_PTR_OFFSET(data, file->sync_offset - @@ -1387,7 +1392,7 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) if (trans_size < sizeof(*hdr)) { mail_transaction_log_file_set_corrupted(file, "hdr.size too small (%u)", trans_size); - return -1; + return 0; } if (file->sync_offset - file->buffer_offset + trans_size > size) @@ -1396,7 +1401,7 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) /* transaction has been fully written */ if ((ret = log_file_track_sync(file, hdr, trans_size)) <= 0) { if (ret < 0) - return -1; + return 0; break; } @@ -1419,6 +1424,7 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) } if ((uoff_t)st.st_size != file->last_size) { file->last_size = st.st_size; + *retry_r = TRUE; return 0; } } @@ -1431,7 +1437,7 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) if (file->locked) { mail_transaction_log_file_set_corrupted(file, "Unexpected garbage at EOF"); - return -1; + return 0; } /* The size field will be updated soon */ mail_index_flush_read_cache(file->log->index, file->filepath, @@ -1445,7 +1451,7 @@ mail_transaction_log_file_sync(struct mail_transaction_log_file *file) "Invalid transaction log size " "(%"PRIuUOFF_T" vs %u): %s", file->sync_offset, file->log->head->hdr.prev_file_offset, file->filepath); - return -1; + return 0; } return 1; @@ -1544,6 +1550,7 @@ static int mail_transaction_log_file_read(struct mail_transaction_log_file *file, uoff_t start_offset, bool nfs_flush) { + bool retry; int ret; i_assert(file->mmap_base == NULL); @@ -1579,11 +1586,10 @@ mail_transaction_log_file_read(struct mail_transaction_log_file *file, mail_transaction_log_file_need_nfs_flush(file)) { /* we didn't read enough data. flush and try again. */ return mail_transaction_log_file_read(file, start_offset, TRUE); - } else if ((ret = mail_transaction_log_file_sync(file)) <= 0) { - i_assert(ret != 0); /* ret=0 happens only with mmap */ - } else { - i_assert(file->sync_offset >= file->buffer_offset); + } else if ((ret = mail_transaction_log_file_sync(file, &retry)) == 0) { + i_assert(!retry); /* retry happens only with mmap */ } + i_assert(file->sync_offset >= file->buffer_offset); buffer_set_used_size(file->buffer, file->sync_offset - file->buffer_offset); return ret; @@ -1665,6 +1671,7 @@ mail_transaction_log_file_map_mmap(struct mail_transaction_log_file *file, uoff_t start_offset) { struct stat st; + bool retry; int ret; /* we are going to mmap() this file, but it's not necessarily @@ -1688,10 +1695,9 @@ mail_transaction_log_file_map_mmap(struct mail_transaction_log_file *file, if (file->buffer != NULL && file->buffer_offset <= start_offset && (uoff_t)st.st_size == file->buffer_offset + file->buffer->used) { /* we already have the whole file mapped */ - if ((ret = mail_transaction_log_file_sync(file)) < 0) - return 0; - if (ret > 0) - return 1; + if ((ret = mail_transaction_log_file_sync(file, &retry)) != 0 || + !retry) + return ret; /* size changed, re-mmap */ } @@ -1707,11 +1713,10 @@ mail_transaction_log_file_map_mmap(struct mail_transaction_log_file *file, if (mail_transaction_log_file_mmap(file) < 0) return -1; - if ((ret = mail_transaction_log_file_sync(file)) < 0) - return 0; - } while (ret == 0); + ret = mail_transaction_log_file_sync(file, &retry); + } while (retry); - return 1; + return ret; } int mail_transaction_log_file_map(struct mail_transaction_log_file *file,