Cyrus 2.3.13 RC1

John Capo jc at irbs.com
Thu Sep 25 15:59:04 EDT 2008


Quoting Ken Murchison (murch at andrew.cmu.edu):
> I just put together a release candidate for Cyrus 2.3.13.  I'd
> appreciate any independent testing before I release this to the masses.
> 
> http://www.contrib.andrew.cmu.edu/~murch/cyrus-imapd-2.3.13rc1.tar.gz
> 
> 
> Noteworthy changes:
> 
> * Added an experimental "sql" backend for cyrusdb.  Currently MySQL,
>   PostgreSQL, and SQLite are supported.
> * Added support for IMAP [CAPABILITY] response code to client-side
>   of Murder proxies.
> * Added support for ManageSieve auto-capability response after
>   STARTTLS and after AUTH with a SASL security layer.
> * Made MAXWORD and MAXQUOTED sizes configurable via imapd.conf
> * Rewrote cyrusdb_quotalegacy.c to use readir()
>   rather than glob.c.  This avoids a potential crash due to
>   conflicts between glibc and Heimdal implementations of glob().
> * Added support for fulldirhash to 'ctl_mboxlist -v'
> * Several skiplist bugfixes.
> 
> Check doc/changes.html for a complete list of changes.
> 
> If there are any outstanding issues that you believe still need to be
> addressed in 2.3.13, please let me know.


statuscache_db.c assumes an off_t is 32bits.

    if (p < dend) scdata->index_size = strtoul(p, &p, 10);

and in the printf that generates the string for the DB.

Attached is an autoconf patch that Lorenzo Catucci posted to the
mailing list in May.  The patch applies and re-running autoconf
does figure out that off_t is 64bits on BSD.  Autoconf fails badly
in other ways but it always does for me.

Here is my original post and the details I sent to Rudy Gevaert
about how the bug bites sometimes.

> Found a bug handling the off_t index_size value in the statuscache
> code.  The code assumes off_t is a 32 bits and the message count gets
> >written as 0.
> 
> (gdb) p *scdata
> $22 = {statusitems = 63, index_mtime = 1208893338, index_ino = 1974639,
> index_size = 6581528, messages = 74789, recent = 174, uidnext = 1547527,
>   uidvalidity = 1125360596, unseen = 74743, highestmodseq = 0}
> (gdb) n
> 247         r = DB->store(statuscachedb, key, keylen, data, datalen, NULL);
> (gdb) p data
> $23 = "3 63 1208893338 1974639 6581528 0 74789 174 1547527 1125360596
> 74743\000
> 
> The attached patch assumes off_t is 64 bits.  Are there any 32 bit
> off_t systems left?
> 

Here's the reason why testing didn't turn up this bug.

I suspected that this might be something to do with newer compilers
mucking with arguments to printf and friends and fixing argument
sizes based on printf paramaters.  This turns out not to be the
case and I would be astonished if the compiler did not do what it
was told to do with respect to printf arguments.

statuscache_db.c:

202         /* Sanity check the data */
203         if (!scdata->statusitems || !scdata->index_mtime || !scdata->index_ino ||
204             !scdata->index_size || !scdata->uidnext || !scdata->uidvalidity) {
205             return IMAP_NO_NOSUCHMSG;
206         }
207
(gdb) p *scdata
$11 = {statusitems = 63, index_mtime = 1208964427, index_ino = 48227,
   index_size = 1856, messages = 0, recent = 20, uidnext = 0,
   uidvalidity = 216164, unseen = 1166134651, highestmodseq = 17}

The test above will fail and the status cache data will not be used
if the bogus data in the status cache is 'just right'.  In the case
above the uidnext is 0 because the argument that was used to store
that value was not the right one due to printf argument alignment,
the 64 bit off_t when a 32 bit value was expected by printf.

My tests are with folders with many thousands of messages.  When
printf generates the string to store in the cache, bogus values are
stored that pass the test above. The bogus status cache data will
be used resulting in the wrong status.

* STATUS BK (MESSAGES 75904 RECENT 1620 UIDNEXT 1549555 UIDVALIDITY 1125360596 UNSEEN 75858)
* STATUS BK (MESSAGES 0 RECENT 75904 UIDNEXT 1620 UIDVALIDITY 1549555 UNSEEN 1125360596)

John Capo


-------------- next part --------------
diff -r f1c01f49ce58 configure.in
--- a/configure.in	Mon May 26 13:45:47 2008 +0200
+++ b/configure.in	Tue May 27 13:31:29 2008 +0200
@@ -49,7 +49,7 @@
 AC_INIT(imap/imapd.c)
 AC_PREREQ([2.54])
 AC_CONFIG_HEADER(config.h)
-AC_CANONICAL_HOST
+AC_CANONICAL_SYSTEM
 
 dnl Useful hook for distributions
 AC_ARG_WITH(extraident,[  --with-extraident=STRING   use STRING as extra version information],
@@ -110,6 +110,9 @@
   AC_DEFINE(HAVE_LONG_LONG_INT,[],[Does the compiler support long long int?])
   AC_C_BIGENDIAN
 fi
+
+dnl Check off_t size
+AC_CHECK_SIZEOF(off_t)
 
 CMU_C___ATTRIBUTE__
 CMU_C_FPIC
@@ -989,9 +992,17 @@
 dnl add perl cccdlflags when building libraries -- this ensures that the
 dnl libraries will be compiled as PIC if perl requires PIC objects
 dnl -- this is needed on NetBSD, but seems to cause problems on atleast Solaris --
-dnl    eval `${PERL} -V:cccdlflags`
-    PERL_CCCDLFLAGS="$cccdlflags"
-    AC_SUBST(PERL_CCCDLFLAGS)
+    case "${target_os}" in
+        linux*|netbsd*)
+            AC_MSG_CHECKING(for perl cccdlflags needed on "${target_os}")
+            eval `${PERL} -V:cccdlflags`
+            PERL_CCCDLFLAGS="$cccdlflags"
+            AC_SUBST(PERL_CCCDLFLAGS)
+            AC_MSG_RESULT($PERL_CCCDLFLAGS)
+            ;;
+        *)
+            AC_MSG_WARN(skipping check for perl cccdlflags on "${target_os}")
+    esac
 fi
 
 dnl for timsieved
diff -r f1c01f49ce58 imap/statuscache_db.c
--- a/imap/statuscache_db.c	Mon May 26 13:45:47 2008 +0200
+++ b/imap/statuscache_db.c	Tue May 27 13:31:29 2008 +0200
@@ -187,7 +187,11 @@
     if (p < dend) scdata->statusitems = (unsigned) strtol(p, &p, 10);
     if (p < dend) scdata->index_mtime = strtol(p, &p, 10);
     if (p < dend) scdata->index_ino = strtoul(p, &p, 10);
+#if SIZEOF_OFF_T == 8
+    if (p < dend) scdata->index_size = strtoull(p, &p, 10);
+#else
     if (p < dend) scdata->index_size = strtoul(p, &p, 10);
+#endif
     if (p < dend) scdata->messages = strtoul(p, &p, 10);
     if (p < dend) scdata->recent = (unsigned) strtoul(p, &p, 10);
     if (p < dend) scdata->uidnext = strtoul(p, &p, 10);
@@ -233,7 +237,11 @@
     char *key = statuscache_buildkey(mboxname, userid, &keylen);
 
     datalen = snprintf(data, sizeof(data),
+#if SIZEOF_OFF_T == 8
+		       "%u %u %ld %lu %llu %lu %u %lu %lu %u " MODSEQ_FMT,
+#else
 		       "%u %u %ld %lu %lu %lu %u %lu %lu %u " MODSEQ_FMT,
+#endif
 		       STATUSCACHE_VERSION, scdata->statusitems,
 		       scdata->index_mtime, scdata->index_ino,
 		       scdata->index_size, scdata->messages,


More information about the Cyrus-devel mailing list