Repost: Stack corruption on platform with 64-bit time_t

Bron Gondwana brong at fastmail.fm
Wed Sep 21 20:05:37 EDT 2016


The problem is entirely that there's no search_time_t_cmp or similar for
time values.  That and the fact that these things operate on pointers to
memory that doesn't have enough type information to ensure we aren't
doing something stupid.

We have a problem with time_t that many of the values we're comparing
against (internaldate, sentdate, etc) are stored in 32 bit fields in
cyrus.index, which is probably why the 32 bit function was chosen.

Bron.


On Wed, 21 Sep 2016, at 12:43, ellie timoney via Cyrus-devel wrote:
> Hi Richard,
>
> Thanks for bumping this, looks like it got forgotten about.  I've put
> this into the github issues tracker as
> https://github.com/cyrusimap/cyrus-imapd/issues/27 so at least it
> won't get forgotten again.
>
> It looks from your patch like "internaldate" and "sentdate" are the
> specific fields causing problems? (Based on line numbers, since the
> patch doesn't have *quite* enough useful context, doh!)
>
> We do already have autoconf testing for sizeof time_t -- e.g. my
> config.h (x86_64) gets:
>
> > /* The size of `time_t', as computed by sizeof. */
> > #define SIZEOF_TIME_T 8
>
> ... but not much code actually looks for that.  And my thinking is
> the same as yours: trivial to patch in this one spot, but what else
> needs doing?
>
> I don't  know search well/at all, so maybe someone who does can chime
> in in more detail here.  Otherwise, I'll experiment and read up a bit
> and see if I can reproduce/understand it.
>
> Cheers,
>
> ellie
>
> On Tue, Sep 20, 2016, at 11:51 PM, Richard Garnish via Cyrus-
> devel wrote:
>> Since sending this report I haven't seen any correspondence on the
>> issue mentioned.  Has the discrepancy of time_t size been addressed
>> in some way yet?  A glance at search_expr.c in git suggests not,
>> unless the problem has been attacked from the other direction by
>> forcing time_t to 32-bit.
>>
>> Richard
>>
>>
>> -------- Forwarded Message -------- Subject: Stack corruption on
>> platform with 64-bit time_t Date: Fri, 29 Apr 2016 20:32:42 +0100
>> From: Richard Garnish <richard at cinesite.com> To: cyrus-
>> devel at lists.andrew.cmu.edu
>>
>>
>> I've been testing 3.0.0-beta2 on CentOS 7.2 for deployment, and have
>> come across a crash on date searches.  For example, many Android
>> clients will start by requesting a list of UIDs from the last 2
>> weeks:  1601 Select {5+} INBOX * 152 EXISTS * 151 RECENT * FLAGS
>> (\Answered \Flagged \Draft \Deleted \Seen NonJunk) * OK
>> [PERMANENTFLAGS (\Answered \Flagged \Draft \Deleted \Seen NonJunk
>> \*)] Ok * OK [UNSEEN 1] Ok * OK [UIDVALIDITY 1461950888] Ok * OK
>> [UIDNEXT 154] Ok * OK [HIGHESTMODSEQ 6] Ok * OK [URLMECH INTERNAL] Ok
>> * OK [ANNOTATIONS 65536] Ok 1601 OK [READ-WRITE] Completed 1602 UID
>> Search all since 15-Apr-2016 *** stack smashing detected ***: imapd
>> terminated ======= Backtrace: =========
>> /usr/lib64/libc.so.6(__fortify_fail+0x37)[0x7fbf13e6ab37]
>> /usr/lib64/libc.so.6(__fortify_fail+0x0)[0x7fbf13e6ab00]
>> /usr/lib64/libcyrus_imap.so.0(+0x6bbc6)[0x7fbf166a5bc6] /usr/lib64/l-
>> ibcyrus_imap.so.0(search_expr_evaluate+0x16c)[0x7fbf166a848c] /usr/l-
>> ib64/libcyrus_imap.so.0(search_expr_evaluate+0x4c)[0x7fbf166a836c]
>> /usr/lib64/libcyrus_imap.so.0(+0x38d64)[0x7fbf16672d64]
>> /usr/lib64/libcyrus_imap.so.0(+0x6f3b8)[0x7fbf166a93b8]
>> /usr/lib64/libcyrus_imap.so.0(+0x6f566)[0x7fbf166a9566]
>> /usr/lib64/libcyrus_imap.so.0(search_query_run+0x72)[0x7fbf166a9f82]
>> /usr/lib64/libcyrus_imap.so.0(index_search+0x69)[0x7fbf16675899]
>> imapd(+0x2849b)[0x7fbf16d6349b] imapd(+0x2a9e8)[0x7fbf16d659e8]
>> imapd(+0x1a699)[0x7fbf16d55699]
>> /usr/lib64/libc.so.6(__libc_start_main+0xf5)[0x7fbf13d7eb15]
>> imapd(+0x1ada9)[0x7fbf16d55da9]  The crash immediate follows
>> search_uint32_cmp operating on m->record.internaldate (of type
>> time_t.)  On CentOS 7 64-bit, time_t (and size_t, off_t, long) are
>> 64-bit types by default. Because it's Friday evening, I've quickly
>> fixed the problem by replacing the search functions for
>> "internaldate" and "sentdate" with the 64-bit equivalents in attrs[]
>> within search_attr_init() and this resolves the crash observed above.
>> However, I'm sure the right way to handle this properly is to test
>> for sizeof(time_t) in autoconf and select the search functions
>> accordingly. I'm also unsure whether this type mismatch impacts
>> anything else -- for instance cyrus.index uses 32-bit fields for
>> dates, and mailbox.c feeds internaldate, sentdate, gmtime and
>> last_updated through htonl() and ntohl() to populate these fields,
>> without explicit typecasting.  Richard  --- cyrus-imapd-3.0.0-
>> beta2/imap/search_expr.c.orig     2016-04-29 19:19:23.453052354 +0100
>> +++ cyrus-imapd-3.0.0-beta2/imap/search_expr.c  2016-04-29
>> 19:20:20.074296701 +0100 @@ -2277,10 +2277,10 @@ EXPORTED void
>> search_attr_init(void) SEARCH_PART_NONE, SEARCH_COST_INDEX,
>> /*internalise*/NULL, -            search_uint32_cmp, -
>> search_uint32_match, -            search_uint32_serialise, -
>> search_uint32_unserialise, +            search_uint64_cmp, +
>> search_uint64_match, +            search_uint64_serialise, +
>> search_uint64_unserialise, /*get_countability*/NULL,
>> /*duplicate*/NULL, /*free*/NULL, @@ -2291,10 +2291,10 @@ EXPORTED
>> void search_attr_init(void) SEARCH_PART_NONE, SEARCH_COST_INDEX,
>> /*internalise*/NULL, -            search_uint32_cmp, -
>> search_uint32_match, -            search_uint32_serialise, -
>> search_uint32_unserialise, +            search_uint64_cmp, +
>> search_uint64_match, +            search_uint64_serialise, +
>> search_uint64_unserialise, /*get_countability*/NULL,
>> /*duplicate*/NULL, /*free*/NULL,  -- Richard Garnish Senior Systems
>> Administrator Cinesite VFX Limited

>>
>

--
  Bron Gondwana
  brong at fastmail.fm

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20160922/88873bc0/attachment.html>


More information about the Cyrus-devel mailing list