<!DOCTYPE html>
<html>
<head>
<title></title>
</head>
<body><div>Hi Richard,<br></div>
<div><br></div>
<div>Thanks for bumping this, looks like it got forgotten about. I've put this into the github issues tracker as <a href="https://github.com/cyrusimap/cyrus-imapd/issues/27">https://github.com/cyrusimap/cyrus-imapd/issues/27</a> so at least it won't get forgotten again.<br></div>
<div><br></div>
<div>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!)<br></div>
<div><br></div>
<div>We do already have autoconf testing for sizeof time_t -- e.g. my config.h (x86_64) gets:<br></div>
<div><br></div>
<div>> /* The size of `time_t', as computed by sizeof. */<br></div>
<div>> #define SIZEOF_TIME_T 8<br></div>
<div><br></div>
<div>... 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?<br></div>
<div><br></div>
<div>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.<br></div>
<div><br></div>
<div>Cheers,<br></div>
<div><br></div>
<div>ellie</div>
<div><br></div>
<div>On Tue, Sep 20, 2016, at 11:51 PM, Richard Garnish via Cyrus-devel wrote:<br></div>
<blockquote type="cite"><div>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.<br></div>
<div> <br></div>
<div> Richard<br></div>
<div> <br></div>
<div><div><br></div>
<div>-------- Forwarded Message --------<br></div>
<table cellspacing="0" cellpadding="0" border="0"><tbody><tr><th nowrap="nowrap">Subject:<br></th><td>Stack corruption on platform with 64-bit time_t<br></td></tr><tr><th nowrap="nowrap">Date:<br></th><td>Fri, 29 Apr 2016 20:32:42 +0100<br></td></tr><tr><th nowrap="nowrap">From:<br></th><td>Richard Garnish <a href="mailto:richard@cinesite.com"><richard@cinesite.com></a><br></td></tr><tr><th nowrap="nowrap">To:<br></th><td><a href="mailto:cyrus-devel@lists.andrew.cmu.edu">cyrus-devel@lists.andrew.cmu.edu</a><br></td></tr></tbody></table><div><br></div>
<div><br></div>
<pre>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/libcyrus_imap.so.0(search_expr_evaluate+0x16c)[0x7fbf166a848c]
/usr/lib64/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
<br></pre></div>
</blockquote><div><br></div>
</body>
</html>