Repost: Stack corruption on platform with 64-bit time_t

[ellie timoney]

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/li-
> bcyrus_imap.so.0(search_expr_evaluate+0x16c)[0x7fbf166a848c] /usr/lib-
> 64/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

>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20160921/8842af21/attachment-0001.html>