[dovecot-cvs] dovecot/src/lib-index mail-transaction-log-append.c, 1.19, 1.20 mail-transaction-log.c, 1.117, 1.118
tss at dovecot.org
tss at dovecot.org
Tue Jan 16 21:58:56 UTC 2007
Update of /var/lib/cvs/dovecot/src/lib-index
In directory talvi:/tmp/cvs-serv1074
Modified Files:
mail-transaction-log-append.c mail-transaction-log.c
Log Message:
Fixed a race condition in transaction log rotation, which could have caused
log data to be overwritten.
Index: mail-transaction-log-append.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-index/mail-transaction-log-append.c,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -d -r1.19 -r1.20
--- mail-transaction-log-append.c 28 Jun 2006 13:10:38 -0000 1.19
+++ mail-transaction-log-append.c 16 Jan 2007 21:58:52 -0000 1.20
@@ -360,9 +360,10 @@
(log)->head->sync_offset == (idx_hdr)->log_file_int_offset && \
(log)->head->sync_offset == (idx_hdr)->log_file_ext_offset)
-int mail_transaction_log_append(struct mail_index_transaction *t,
- uint32_t *log_file_seq_r,
- uoff_t *log_file_offset_r)
+static int
+mail_transaction_log_append_locked(struct mail_index_transaction *t,
+ uint32_t *log_file_seq_r,
+ uoff_t *log_file_offset_r)
{
struct mail_index_view *view = t->view;
struct mail_index *index;
@@ -377,26 +378,12 @@
index = mail_index_view_get_index(view);
log = index->log;
- if (!t->log_updates) {
- /* nothing to append */
- *log_file_seq_r = 0;
- *log_file_offset_r = 0;
- return 0;
- }
-
- if (log->index->log_locked) {
- i_assert(t->external);
- } else {
- if (mail_transaction_log_lock_head(log) < 0)
- return -1;
-
+ if (!index->log_locked) {
/* update sync_offset */
if (mail_transaction_log_file_map(log->head,
log->head->sync_offset,
- (uoff_t)-1) < 0) {
- mail_transaction_log_file_unlock(log->head);
+ (uoff_t)-1) < 0)
return -1;
- }
}
if (log->head->sync_offset > MAIL_TRANSACTION_LOG_ROTATE_SIZE &&
@@ -405,28 +392,21 @@
ARE_ALL_TRANSACTIONS_IN_INDEX(log, index->hdr)) {
/* we might want to rotate, but check first that everything is
synced in index. */
- if (mail_index_lock_shared(log->index, TRUE, &lock_id) < 0) {
- if (!log->index->log_locked)
- mail_transaction_log_file_unlock(log->head);
+ if (mail_index_lock_shared(index, TRUE, &lock_id) < 0)
return -1;
- }
/* we need the latest log_file_*_offsets. It's important to
use this function instead of mail_index_map() as it may
have generated them by reading log files. */
if (mail_index_get_latest_header(index, &idx_hdr) <= 0) {
mail_index_unlock(index, lock_id);
- if (!log->index->log_locked)
- mail_transaction_log_file_unlock(log->head);
return -1;
}
- mail_index_unlock(log->index, lock_id);
+ mail_index_unlock(index, lock_id);
if (ARE_ALL_TRANSACTIONS_IN_INDEX(log, &idx_hdr)) {
- if (mail_transaction_log_rotate(log, TRUE) < 0) {
- /* that didn't work. well, try to continue
- anyway */
- }
+ if (mail_transaction_log_rotate(log, TRUE) < 0)
+ return -1;
}
}
@@ -502,7 +482,7 @@
ret = pwrite_full(file->fd, &file->first_append_size,
sizeof(uint32_t), append_offset);
if (ret < 0) {
- mail_index_file_set_syscall_error(log->index,
+ mail_index_file_set_syscall_error(index,
file->filepath, "pwrite()");
}
} else {
@@ -527,9 +507,36 @@
*log_file_seq_r = file->hdr.file_seq;
*log_file_offset_r = file->sync_offset;
-
- if (!log->index->log_locked)
- mail_transaction_log_file_unlock(file);
return ret;
}
+int mail_transaction_log_append(struct mail_index_transaction *t,
+ uint32_t *log_file_seq_r,
+ uoff_t *log_file_offset_r)
+{
+ struct mail_index *index;
+ int ret;
+
+ if (!t->log_updates) {
+ /* nothing to append */
+ *log_file_seq_r = 0;
+ *log_file_offset_r = 0;
+ return 0;
+ }
+
+ index = mail_index_view_get_index(t->view);
+
+ if (index->log_locked) {
+ i_assert(t->external);
+ } else {
+ if (mail_transaction_log_lock_head(index->log) < 0)
+ return -1;
+ }
+
+ ret = mail_transaction_log_append_locked(t, log_file_seq_r,
+ log_file_offset_r);
+
+ if (!index->log_locked)
+ mail_transaction_log_file_unlock(index->log->head);
+ return ret;
+}
Index: mail-transaction-log.c
===================================================================
RCS file: /var/lib/cvs/dovecot/src/lib-index/mail-transaction-log.c,v
retrieving revision 1.117
retrieving revision 1.118
diff -u -d -r1.117 -r1.118
--- mail-transaction-log.c 28 Dec 2006 16:28:29 -0000 1.117
+++ mail-transaction-log.c 16 Jan 2007 21:58:53 -0000 1.118
@@ -486,31 +486,37 @@
}
static int
-mail_transaction_log_file_create2(struct mail_transaction_log *log,
- const char *path, int new_fd,
+mail_transaction_log_file_create2(struct mail_transaction_log_file *file,
+ bool lock, int new_fd,
struct dotlock **dotlock,
dev_t dev, ino_t ino, uoff_t file_size)
{
- struct mail_index *index = log->index;
+ struct mail_index *index = file->log->index;
struct mail_transaction_log_header hdr;
struct stat st;
const char *path2;
int old_fd, ret;
- bool found;
+ bool rename_existing;
+
+ i_assert(!lock || file->log->head->locked);
/* log creation is locked now - see if someone already created it */
- old_fd = nfs_safe_open(path, O_RDWR);
- if (old_fd != -1) {
+ if (lock) {
+ /* don't even bother checking the existing file, but rename it
+ if it exists */
+ rename_existing = TRUE;
+ } else if ((old_fd = nfs_safe_open(file->filepath, O_RDWR)) != -1) {
if ((ret = fstat(old_fd, &st)) < 0) {
- mail_index_file_set_syscall_error(index, path,
+ mail_index_file_set_syscall_error(index, file->filepath,
"fstat()");
} else if (st.st_ino == ino && CMP_DEV_T(st.st_dev, dev) &&
(uoff_t)st.st_size == file_size) {
/* same file, still broken */
} else {
/* file changed, use the new file */
- (void)file_dotlock_delete(dotlock);
- return old_fd;
+ (void)file_dotlock_delete(dotlock);
+ file->fd = old_fd;
+ return 0;
}
(void)close(old_fd);
@@ -520,29 +526,35 @@
/* fstat() failure, return after closing fd.. */
return -1;
}
- found = TRUE;
+ rename_existing = TRUE;
} else if (errno != ENOENT) {
- mail_index_file_set_syscall_error(index, path, "open()");
+ mail_index_file_set_syscall_error(index, file->filepath,
+ "open()");
return -1;
} else {
- found = FALSE;
+ rename_existing = FALSE;
}
- if (mail_transaction_log_init_hdr(log, &hdr) < 0)
+ if (mail_transaction_log_init_hdr(file->log, &hdr) < 0)
return -1;
if (write_full(new_fd, &hdr, sizeof(hdr)) < 0) {
- mail_index_file_set_syscall_error(index, path,
+ mail_index_file_set_syscall_error(index, file->filepath,
"write_full()");
return -1;
}
+ if (lock) {
+ if (mail_transaction_log_file_lock(file) < 0)
+ return -1;
+ }
+
/* keep two log files */
- if (found) {
- path2 = t_strconcat(path, ".2", NULL);
- if (rename(path, path2) < 0) {
- mail_index_set_error(index,
- "rename(%s, %s) failed: %m", path, path2);
+ if (rename_existing) {
+ path2 = t_strconcat(file->filepath, ".2", NULL);
+ if (rename(file->filepath, path2) < 0 && errno != ENOENT) {
+ mail_index_set_error(index, "rename(%s, %s) failed: %m",
+ file->filepath, path2);
/* ignore the error. we don't care that much about the
second log file and we're going to overwrite this
first one. */
@@ -554,63 +566,65 @@
return -1;
/* success */
- return new_fd;
+ file->fd = new_fd;
+ return 0;
}
static int
-mail_transaction_log_file_create(struct mail_transaction_log *log,
- const char *path,
- dev_t dev, ino_t ino, uoff_t file_size)
+mail_transaction_log_file_create(struct mail_transaction_log_file *file,
+ bool lock, dev_t dev, ino_t ino,
+ uoff_t file_size)
{
+ struct mail_index *index = file->log->index;
struct dotlock *dotlock;
struct stat st;
mode_t old_mask;
int fd;
- i_assert(!MAIL_INDEX_IS_IN_MEMORY(log->index));
+ i_assert(!MAIL_INDEX_IS_IN_MEMORY(index));
- if (stat(log->index->dir, &st) < 0) {
+ if (stat(index->dir, &st) < 0) {
if (ENOTFOUND(errno)) {
/* the whole index directory was deleted, which means
the mailbox was deleted by another process.
fail silently. */
- mail_index_mark_corrupted(log->index);
+ mail_index_mark_corrupted(index);
return -1;
}
- mail_index_file_set_syscall_error(log->index, log->index->dir,
- "stat()");
+ mail_index_file_set_syscall_error(index, index->dir, "stat()");
return -1;
}
/* With dotlocking we might already have path.lock created, so this
filename has to be different. */
- old_mask = umask(log->index->mode ^ 0666);
- fd = file_dotlock_open(&log->new_dotlock_settings, path, 0, &dotlock);
+ old_mask = umask(index->mode ^ 0666);
+ fd = file_dotlock_open(&file->log->new_dotlock_settings,
+ file->filepath, 0, &dotlock);
umask(old_mask);
if (fd == -1) {
- mail_index_file_set_syscall_error(log->index, path,
+ mail_index_file_set_syscall_error(index, file->filepath,
"file_dotlock_open()");
return -1;
}
- if (log->index->gid != (gid_t)-1 &&
- fchown(fd, (uid_t)-1, log->index->gid) < 0) {
- mail_index_file_set_syscall_error(log->index, path, "fchown()");
+ if (index->gid != (gid_t)-1 &&
+ fchown(fd, (uid_t)-1, index->gid) < 0) {
+ mail_index_file_set_syscall_error(index, file->filepath,
+ "fchown()");
(void)file_dotlock_delete(&dotlock);
return -1;
}
/* either fd gets used or the dotlock gets deleted and returned fd
is for the existing file */
- fd = mail_transaction_log_file_create2(log, path, fd, &dotlock,
- dev, ino, file_size);
- if (fd < 0) {
+ if (mail_transaction_log_file_create2(file, lock, fd, &dotlock,
+ dev, ino, file_size) < 0) {
if (dotlock != NULL)
(void)file_dotlock_delete(&dotlock);
return -1;
}
- return fd;
+ return 0;
}
static void
@@ -655,104 +669,72 @@
*p = file;
}
+static struct mail_transaction_log_file *
+mail_transaction_log_file_alloc(struct mail_transaction_log *log,
+ const char *path)
+{
+ struct mail_transaction_log_file *file;
+
+ file = i_new(struct mail_transaction_log_file, 1);
+ file->log = log;
+ file->filepath = i_strdup(path);
+ file->fd = -1;
+ return file;
+}
+
static int
-mail_transaction_log_file_fd_open(struct mail_transaction_log *log,
- struct mail_transaction_log_file **file_r,
- const char *path, int fd, bool head,
- bool ignore_estale)
+mail_transaction_log_file_fd_open(struct mail_transaction_log_file *file,
+ bool head, bool ignore_estale)
{
- struct mail_transaction_log_file *file;
struct stat st;
- int ret;
-
- i_assert(!MAIL_INDEX_IS_IN_MEMORY(log->index));
- *file_r = NULL;
+ i_assert(!MAIL_INDEX_IS_IN_MEMORY(file->log->index));
- if (fstat(fd, &st) < 0) {
+ if (fstat(file->fd, &st) < 0) {
if (errno != ESTALE || !ignore_estale) {
- mail_index_file_set_syscall_error(log->index, path,
- "fstat()");
+ mail_index_file_set_syscall_error(file->log->index,
+ file->filepath,
+ "fstat()");
}
- close_keep_errno(fd);
return -1;
}
- file = i_new(struct mail_transaction_log_file, 1);
- file->log = log;
- file->filepath = i_strdup(path);
- file->fd = fd;
file->st_dev = st.st_dev;
file->st_ino = st.st_ino;
file->last_mtime = st.st_mtime;
file->last_size = st.st_size;
- ret = mail_transaction_log_file_read_hdr(file, head, ignore_estale);
- if (ret < 0) {
- mail_transaction_log_file_free(file);
- return -1;
- }
-
- *file_r = file;
- return ret;
+ return mail_transaction_log_file_read_hdr(file, head, ignore_estale);
}
-
-static struct mail_transaction_log_file *
-mail_transaction_log_file_fd_open_or_create(struct mail_transaction_log *log,
- const char *path, int fd,
- bool *retry_r, bool try_retry)
+static int
+mail_transaction_log_file_fd_open_or_create(struct mail_transaction_log_file
+ *file, bool try_retry)
{
- struct mail_transaction_log_file *file;
- struct stat st;
int ret;
- *retry_r = FALSE;
-
- ret = mail_transaction_log_file_fd_open(log, &file, path, fd, TRUE,
- !try_retry);
- if (ret < 0) {
- *retry_r = errno == ESTALE && try_retry;
- return NULL;
- }
-
+ ret = mail_transaction_log_file_fd_open(file, TRUE, !try_retry);
if (ret == 0) {
- /* corrupted header */
- fd = mail_transaction_log_file_create(log, path, file->st_dev,
- file->st_ino,
- file->last_size);
- if (fd == -1)
- ret = -1;
- else if (fstat(fd, &st) < 0) {
- mail_index_file_set_syscall_error(log->index, path,
- "fstat()");
- (void)close(fd);
- fd = -1;
+ /* corrupted header, recreate the file */
+ if (mail_transaction_log_file_create(file, FALSE,
+ file->st_dev,
+ file->st_ino,
+ file->last_size) < 0)
ret = -1;
+ else {
+ ret = mail_transaction_log_file_fd_open(file, TRUE,
+ FALSE);
+ if (ret == 0) {
+ /* newly created transaction log corrupted */
+ return -1;
+ }
}
-
- if (fd != -1) {
- (void)close(file->fd);
- file->fd = fd;
-
- file->st_dev = st.st_dev;
- file->st_ino = st.st_ino;
- file->last_mtime = st.st_mtime;
- file->last_size = st.st_size;
-
- memset(&file->hdr, 0, sizeof(file->hdr));
- ret = mail_transaction_log_file_read_hdr(file, TRUE,
- !try_retry);
- }
- }
- if (ret <= 0) {
- *retry_r = errno == ESTALE && try_retry;
- mail_transaction_log_file_free(file);
- return NULL;
}
+ if (ret < 0)
+ return errno == ENOENT && try_retry ? 0 : -1;
mail_transaction_log_file_add_to_head(file);
- return file;
+ return 1;
}
static struct mail_transaction_log_file *
@@ -783,36 +765,41 @@
{
struct mail_transaction_log_file *file;
unsigned int i;
- bool retry;
- int fd;
+ int ret;
if (MAIL_INDEX_IS_IN_MEMORY(log->index))
return mail_transaction_log_file_alloc_in_memory(log);
+ file = mail_transaction_log_file_alloc(log, path);
+
for (i = 0; ; i++) {
- fd = nfs_safe_open(path, O_RDWR);
- if (fd == -1) {
+ file->fd = nfs_safe_open(path, O_RDWR);
+ if (file->fd == -1) {
if (errno != ENOENT) {
- mail_index_file_set_syscall_error(log->index,
+ mail_index_file_set_syscall_error(log->index,
path,
"open()");
- return NULL;
+ break;
}
/* doesn't exist, try creating it */
- fd = mail_transaction_log_file_create(log, path,
- 0, 0, 0);
- if (fd == -1)
- return NULL;
+ if (mail_transaction_log_file_create(file, FALSE,
+ 0, 0, 0) < 0)
+ break;
}
- file = mail_transaction_log_file_fd_open_or_create(log, path,
- fd, &retry, i == MAIL_INDEX_ESTALE_RETRY_COUNT);
- if (file != NULL || !retry)
- return file;
+ ret = mail_transaction_log_file_fd_open_or_create(file,
+ i == MAIL_INDEX_ESTALE_RETRY_COUNT);
+ if (ret > 0)
+ return file;
+ if (ret < 0)
+ break;
/* ESTALE - retry */
- }
+ }
+
+ mail_transaction_log_file_free(file);
+ return NULL;
}
static struct mail_transaction_log_file *
@@ -821,36 +808,40 @@
{
struct mail_transaction_log_file *file;
unsigned int i;
- int fd, ret;
+ int ret;
+ file = mail_transaction_log_file_alloc(log, path);
for (i = 0;; i++) {
- fd = nfs_safe_open(path, O_RDWR);
- if (fd == -1) {
+ file->fd = nfs_safe_open(path, O_RDWR);
+ if (file->fd == -1) {
mail_index_file_set_syscall_error(log->index, path,
"open()");
- return NULL;
+ break;
}
- ret = mail_transaction_log_file_fd_open(log, &file, path,
- fd, TRUE, i < MAIL_INDEX_ESTALE_RETRY_COUNT);
- if (ret > 0)
- break;
+ ret = mail_transaction_log_file_fd_open(file,
+ TRUE, i < MAIL_INDEX_ESTALE_RETRY_COUNT);
+ if (ret > 0) {
+ /* success */
+ mail_transaction_log_file_add_to_head(file);
+ return file;
+ }
- /* error / corrupted */
- if (ret == 0)
- mail_transaction_log_file_free(file);
- else {
- if (errno == ESTALE &&
- i < MAIL_INDEX_ESTALE_RETRY_COUNT) {
- /* try again */
- continue;
- }
- }
- return NULL;
+ if (ret == 0) {
+ /* corrupted */
+ break;
+ }
+ if (errno != ESTALE ||
+ i == MAIL_INDEX_ESTALE_RETRY_COUNT) {
+ /* syscall error */
+ break;
+ }
+
+ /* ESTALE - try again */
}
- mail_transaction_log_file_add_to_head(file);
- return file;
+ mail_transaction_log_file_free(file);
+ return NULL;
}
void mail_transaction_logs_clean(struct mail_transaction_log *log)
@@ -871,8 +862,7 @@
struct mail_transaction_log_file *file;
const char *path = log->head->filepath;
struct stat st;
- bool retry;
- int fd;
+ int ret;
i_assert(log->head->locked);
@@ -887,26 +877,22 @@
return -1;
}
- fd = mail_transaction_log_file_create(log, path,
- st.st_dev, st.st_ino,
- st.st_size);
- if (fd == -1)
+ file = mail_transaction_log_file_alloc(log, path);
+ if (mail_transaction_log_file_create(file, lock, st.st_dev,
+ st.st_ino,
+ st.st_size) < 0) {
+ mail_transaction_log_file_free(file);
return -1;
+ }
- file = mail_transaction_log_file_fd_open_or_create(log, path,
- fd, &retry, FALSE);
- if (file == NULL) {
- i_assert(!retry);
- return -1;
- }
- }
-
- if (lock) {
- if (mail_transaction_log_file_lock(file) < 0) {
- mail_transaction_logs_clean(log);
+ ret = mail_transaction_log_file_fd_open_or_create(file, FALSE);
+ if (ret <= 0) {
+ i_assert(ret != 0);
+ mail_transaction_log_file_free(file);
return -1;
}
}
+
i_assert(file->locked == lock);
if (--log->head->refcount == 0)
@@ -1022,8 +1008,11 @@
}
}
- ret = mail_transaction_log_file_fd_open(log, &file, path, fd,
- FALSE, TRUE);
+
+ file = mail_transaction_log_file_alloc(log, path);
+ file->fd = fd;
+
+ ret = mail_transaction_log_file_fd_open(file, FALSE, TRUE);
if (ret <= 0) {
bool stale = errno == ESTALE;
More information about the dovecot-cvs
mailing list