unexpunge segfaults with -l on some mailboxes

[Bron Gondwana]

On Wed, Mar 11, 2009 at 12:40:28PM -0300, Patrick Boutilier wrote:
> Darn. This is caused by mailbox corruption again. My script to detect  
> corruption wasn't working properly. Any idea how we can track down what  
> is causing the corruption?

Yeah, I was going to say.

It's corrupted cache file offset pointers.  Assuming you upgraded
this from an earlier version of Cyrus at some point, cyrus.expunge
and cyrus.index files had issues maintaining cache pointer
consistency.  You pretty much have to reconstruct all your mailboxes
to guarantee consistency I think.

I'm tempted to protect the code from crashing though... we don't
use unexpunge at FastMail, which is probably why I haven't already
done so.

Something like the attached should do it.  I'll test it more
completely and commit it to CVS for 2.3.14 (since Ken hasn't
cut a release candidate yet!)

Bron.
-------------- next part --------------
diff --git a/imap/mailbox.c b/imap/mailbox.c
index 39896a4..4044b3c 100644
--- a/imap/mailbox.c
+++ b/imap/mailbox.c
@@ -297,19 +297,11 @@ unsigned mailbox_cached_header_inline(const char *text)
     return BIT32_MAX;
 }
 
-unsigned long mailbox_cache_size(struct mailbox *mailbox, unsigned msgno)
+unsigned long mailbox_cache_size_offset(struct mailbox *mailbox, unsigned cache_offset)
 {
-    const char *p;
-    unsigned long cache_offset;
     unsigned int cache_ent;
     const char *cacheitem, *cacheitembegin;
 
-    assert((msgno > 0) && (msgno <= mailbox->exists));
-
-    p = (mailbox->index_base + mailbox->start_offset +
-         ((msgno-1) * mailbox->record_size));
-    
-    cache_offset = ntohl(*((bit32 *)(p+OFFSET_CACHE_OFFSET)));
     if (cache_offset > mailbox->cache_size) {
 	return 0;
     }
@@ -321,13 +313,27 @@ unsigned long mailbox_cache_size(struct mailbox *mailbox, unsigned msgno)
     for (cache_ent = 0; cache_ent < NUM_CACHE_FIELDS; cache_ent++) {
 	cacheitem = CACHE_ITEM_NEXT(cacheitem);
 	if (cacheitem < cacheitembegin ||
-	    cacheitem > cacheitembegin + mailbox->cache_size) {
+	    cacheitem > mailbox->cache_base + mailbox->cache_size) {
 	    return 0; /* clearly bogus */
 	}
     }
     return (cacheitem - cacheitembegin);
 }
 
+unsigned long mailbox_cache_size(struct mailbox *mailbox, unsigned msgno)
+{
+    const char *p;
+
+    assert((msgno > 0) && (msgno <= mailbox->exists));
+
+    p = (mailbox->index_base + mailbox->start_offset +
+         ((msgno-1) * mailbox->record_size));
+    
+    cache_offset = ntohl(*((bit32 *)(p+OFFSET_CACHE_OFFSET)));
+
+    return mailbox_cache_size_offset(mailbox, cache_offset);
+}
+
 /* function to be used for notification of mailbox changes/updates */
 static mailbox_notifyproc_t *updatenotifier = NULL;
 
diff --git a/imap/mailbox.h b/imap/mailbox.h
index 06b43b5..19c0662 100644
--- a/imap/mailbox.h
+++ b/imap/mailbox.h
@@ -293,6 +293,7 @@ enum {
 unsigned mailbox_cached_header(const char *s);
 unsigned mailbox_cached_header_inline(const char *text);
 
+unsigned long mailbox_cache_size_offset(struct mailbox *mailbox, unsigned cache_offset);
 unsigned long mailbox_cache_size(struct mailbox *mailbox, unsigned msgno);
 
 typedef unsigned mailbox_decideproc_t(struct mailbox *mailbox, void *rock,
diff --git a/imap/unexpunge.c b/imap/unexpunge.c
index 37f62cc..b7816e2 100644
--- a/imap/unexpunge.c
+++ b/imap/unexpunge.c
@@ -123,7 +123,7 @@ void list_expunged(struct mailbox *mailbox,
 		   const char *expunge_index_base)
 {
     const char *rec;
-    unsigned msgno;
+    unsigned msgno, cache_ent;
     unsigned long uid, size, cache_offset;
     time_t internaldate, sentdate, last_updated;
     const char *cacheitem;
@@ -146,6 +146,11 @@ void list_expunged(struct mailbox *mailbox,
 	printf("\tRecv: %s", ctime(&internaldate));
 	printf("\tExpg: %s", ctime(&last_updated));
 
+	if (!mailbox_cache_size_offset(mailbox, cache_offset)) {
+	    printf("\tERROR: cache record missing or corrupt, not printing cache details\n");
+	    continue;
+	}
+
 	cacheitem = mailbox->cache_base + cache_offset;
 	cacheitem = CACHE_ITEM_NEXT(cacheitem); /* skip envelope */
 	cacheitem = CACHE_ITEM_NEXT(cacheitem); /* skip body structure */