So we have a couple of other things as well - you'll remember
the crashes in cyr_expire when it hit bogus cache offsets in
cyrus.expunge files.
Then there are things like this:
[EMAIL PROTECTED] hm]$ sudo -u cyrus /usr/cyrus/bin/reconstruct -C
/etc/imapd-slot401.conf -rf user.b-----
Segmentation fault
Caused by the fact that we mmap files on disk and then randomly
trust offsets in them far too easily.
So my plan for the next couple of steps is:
/* parse the cache record for "msgno" and check that all the
* offsets are sane and it fits within the mmaped cache file.
*
* records - if passed it must be an array of NUM_CACHE_FIELDS
* size, which will be filled in with direct pointers
* to each field of the cache.
* size - if passed it will be filled with the total size
* in bytes of the cache record.
*
* RETURNS: 0 on success (valid cache record), cyrus error code
* on failure.
*/
int index_cache_record(struct mailbox *mailbox, unsigned msgno,
unsigned *size, char **records);
Then change everything which currently uses direct offset macros
to use this function and then jump directly to the fields it
wants within the cache record using records[ENV_TO], etc. If it
just wants to stream the record to another file, then records[0]
will give a base offset, and size will give the number of bytes
to copy.
This function will also get a CRC32 check added very simply when
it comes time.
There will also be a non-mailbox version of the above when you only
have an mmaped file to deal with. This will need the base of the
buffer, the length of the buffer and the offset within the buffer
where this record starts. Obviously, index_cache_record will be
implemented in terms of this.
int buffer_cache_record(const char *base, unsigned length, unsigned offset,
unsigned *size, char **records);
and if you want the crc32 check you're going to have to do it
yourself using the offset and size logic above since this function
won't know the crc32 going in!
Moving further along, there's going to be a need to replace all the
UID(msgno), INTERNALDATE(msgno), SIZE(msgno), etc macros with a two
stage process instead:
mailbox_read_index_record(mailbox, msgno, &record);
record.uid
record.internaldate
record.size
This will allow integrity checks in one place. The parsing cost is
trivial for the additional safety, but it means changing a _LOT_ of
code! I suspect I'll be releasing an initial crc32 patch that doesn't
do all the checks yet, just does the setting correctly on all paths
and has the option of checks wherever you want them.
Bron ( rewriting this software piece by piece! )
--
Bron Gondwana
[EMAIL PROTECTED]
