Thanks - that looks sensible I think. I'll roll it into master and 2.5 and 2.4.
Bron.
On Sun, Jul 19, 2015, at 07:06 AM, Thomas Jarosch wrote:
> mailbox_release_resources() closes the file descriptor
> to the index. This implicitly releases any lock on the file.
>
> Unfortunately the function does not reset the index_locktype state.
> Therefore we didn't notice that a mailbox rename
> operated on an unlocked index of the new mailbox. Yikes.
>
> Fix it by resetting the lock state and also adapt
> API users to properly lock the index. This change
> might uncover more API abuse / break "working" code.
>
> Note: mailbox_index_lock() implies a mailbox_read_index_header().
> But it also does more work than just read the index header,
> so there might be side effects.
>
> Patch developed against 2.5 and is easily ported to 2.4 or git HEAD.
> I'll resubmit patches for each branch once the general
> concept of this change is approved.
>
> ---
> imap/mailbox.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/imap/mailbox.c b/imap/mailbox.c
> index 51b8c9d..5b7bd9c 100644
> --- a/imap/mailbox.c
> +++ b/imap/mailbox.c
> @@ -833,6 +833,7 @@ static void mailbox_release_resources(struct mailbox
> *mailbox)
>
> /* release and unmap index */
> xclose(mailbox->index_fd);
> + mailbox->index_locktype = 0;
> if (mailbox->index_base)
> map_free(&mailbox->index_base, &mailbox->index_len);
>
> @@ -4052,7 +4053,7 @@ HIDDEN int mailbox_rename_copy(struct mailbox
> *oldmailbox,
> if (r) goto fail;
>
> /* Re-open header file */
> - r = mailbox_read_index_header(newmailbox);
> + r = mailbox_lock_index(newmailbox, LOCK_EXCLUSIVE);
> if (r) goto fail;
>
> /* read in the flags */
> @@ -4370,7 +4371,7 @@ static int mailbox_reconstruct_create(const char *name,
> struct mailbox **mbptr)
>
> /* Attempt to open index */
> r = mailbox_open_index(mailbox);
> - if (!r) r = mailbox_read_index_header(mailbox);
> + if (!r) r = mailbox_lock_index(mailbox, LOCK_EXCLUSIVE);
> if (r) {
> printf("%s: failed to read index header\n", mailbox->name);
> syslog(LOG_ERR, "failed to read index header for %s", mailbox->name);
> --
> 1.9.3
>
--
Bron Gondwana
br...@fastmail.fm
