Hi, I'm getting bad data in the 32 bit section of some of the compiled files in zoneinfo-leaps. The affected time zones are zones that only transition before 1901, usually from LMT to some UTC offset: Indian/Christmas Indian/Cocos Atlantic/South_Georgia Pacific/Port_Moresby Pacific/Chuuk Pacific/Yap Pacific/Wallis Pacific/Funafuti Pacific/Palau Pacific/Ponape Pacific/Wake Pacific/Pohnpei Pacific/Truk Pacific/Tarawa The files in zoneinfo-leaps contain no transitions, and only the offset used before the transition. This causes applications that only read the 32 bit section to use LMT for these zones. The 64 bit section, and both sections in the zoneinfo version of these files, contain >=2 transitions and offsets (e.g. one real transition and one in 2038). For example, zdump with a break added after /old file/ in localtime.c: $ ./zdump $PWD/TZ/usr/share/zoneinfo*/Atlantic/South_Georgia .../TZ/usr/share/zoneinfo/Atlantic/South_Georgia Tue Oct 23 12:31:33 2018 -02 .../TZ/usr/share/zoneinfo-leaps/Atlantic/South_Georgia Tue Oct 23 12:04:58 2018 LMT $ ./zdump -V $PWD/TZ/usr/share/zoneinfo-leaps/Atlantic/South_Georgia .../TZ/usr/share/zoneinfo-leaps/Atlantic/South_Georgia Sat Jul 1 00:00:00 1972 UT = Fri Jun 30 21:33:52 1972 LMT isdst=0 gmtoff=-8768 .../TZ/usr/share/zoneinfo-leaps/Atlantic/South_Georgia Sat Jul 1 00:00:01 1972 UT = Fri Jun 30 21:33:52 1972 LMT isdst=0 gmtoff=-8768 [..] .../TZ/usr/share/zoneinfo-leaps/Atlantic/South_Georgia Sun Jan 1 00:00:27 2017 UT = Sat Dec 31 21:33:52 2016 LMT isdst=0 gmtoff=-8768 I found that before commit 83c119f4d ("Remove Big Bang hack"), the one offset present in the leaps version was the correct one. The structural difference is older; it just didn't affect my application because the offset was the one currently in use. The cause appears to be this sequence of events starting at zic.c /Work around QTBUG-53071/: 1) y2038_boundary-1 transition is added (== INT32_MAX) 2) leap seconds are added to all times, pushing the 2038 transition above INT32_MAX 3) affected zones again only have transition times outside the 32 bit range => timecnt32 is 0 4) because timecnt32 is 0, timei32 is also 0, and the INT32_MIN transition isn't output either 5) because of omittype[defaulttype] = false, the oldest offset is always output regardless of the above The workaround could probably be moved to after the leap seconds are added, especially if it's supposed to apply to the 32 bit leaps files. This will also fix this bug, because the code that determines the 32 bit limits assumes there's at least one time after 1901, and the workaround provides one. (This is why the non-leaps version is correct.) I'm unfortunately not familiar enough with the tz database to suggest a complete fix. Danny -- Daniel Fischer ORACLE Deutschland B.V. & Co. KG, Riesstr. 25, 80992 München - HRA MUC 95603 Komplementaer: ORACLE DE Verw.B.V., Hertogswetering 163/167, 3543 AS Utrecht Geschaeftsfuehrer: Alexander van der Ven, Jan Schultheiss, Val Maher NL
Thanks for the heads-up. I confirmed that tzcode 2015f (and earlier) when compiled for 32-bit time_t misreads the 32-bit binary (TZif) files generated by tzdb 2018f. I'll try to come up with a fix soon, most likely by having zic insert a dummy transition or two in the 32-bit data. As a temporary workaround you can continue to use 2018e zic to compile from .zi to TZif. Recommended practice nowadays is for 32-bit client code to ignore 32-bit binary data entirely, and to just use the 64-bit data, discarding any timestamps outside the 32-bit range. This is why tests with 2018f didn't catch the problem. However, we should support 32-bit clients that don't follow the recommended practice.
Please try the attached patch, which I installed into the development version on GitHub. Although this fixes the problem for my tests, I'd also like to know whether it fixes the problem for yours.
Paul,
Please try the attached patch, which I installed into the development version on GitHub.
The patch does fix my problem. But if the Qt workaround is disabled, there's a buffer overrun now.
+ while (0 < timecnt32 && INT32_MAX < ats[timecnt32 - 1]) --timecnt32; + while (0 < timecnt32 && ats[timei32] < INT32_MIN) { --timecnt32; ++timei32; }
For a zone like Atlantic/South_Georgia, one transition before 1901, no fake 2038 transition: timei32 == 1 timecnt32 == 0 Later, this read from ats[1] is out of bounds:
if (timei32 > 0 && ats[timei32] > INT32_MIN) {
For a quick test, something like this should trigger asan in a fresh clone from github: sed -ibak 's/\(WORK_AROUND_QTBUG_53071 = \)true/\1false/' zic.c make CFLAGS=-fsanitize=address echo -e "Zone foo 0:0 - -0 1890\n0:0 - -0" | ./zic -d foo - NB, the allocation of ats as nats * 9 byte at the start of writezone() might not be as intended:
zic_t *ats = emalloc(size_product(nats, sizeof *ats + 1));
Danny -- Daniel Fischer ORACLE Deutschland B.V. & Co. KG, Riesstr. 25, 80992 München - HRA MUC 95603 Komplementaer: ORACLE DE Verw.B.V., Hertogswetering 163/167, 3543 AS Utrecht Geschaeftsfuehrer: Alexander van der Ven, Jan Schultheiss, Val Maher NL
On 10/25/18 8:38 AM, Daniel Fischer wrote:
But if the Qt workaround is disabled, there's a buffer overrun now.
Thanks for catching that; proposed patch attached (and installed into GitHub). Please give it a try. The fix is in the patch's last hunk.
sed -ibak 's/\(WORK_AROUND_QTBUG_53071 = \)true/\1false/' zic.c
That's awkward. The attached patch surrounds the constant's definition with an ifndef so that you don't need to modify the source code to disable the Qt bug workaround.
NB, the allocation of ats as nats * 9 byte at the start of writezone() might not be as intended:
It's intended, though now that you mention it I see that there could be an issue if malloc (N) assumes that an odd-aligned pointer can be returned when N is odd. The C Standard allows this behavior. The attached patch should fix this bug too. Thanks again for your careful analysis of the code.
participants (2)
-
Daniel Fischer -
Paul Eggert