[RFC PATCH] Fix index lock breakage in mailbox_release_resources()

Bron Gondwana brong at fastmail.fm
Sun Jul 19 12:54:28 EDT 2015


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
  brong at fastmail.fm


More information about the Cyrus-devel mailing list