shared/calendarspec: fix normalization when DST is negative

kmeaw · bluca · commit 8c08ad735d67 · 2025-05-18T22:16:44.000+01:00
When trying to calculate the next firing of 'hourly', we'd lose the tm_isdst value on the next iteration. On most systems in Europe/Dublin it would cause a 100% cpu hang due to timers restarting. This happens in Europe/Dublin because Ireland defines the Irish Standard Time as UTC+1, so winter time is encoded in tzdata as negative 1 hour of daylight saving. Before this patch: $ env TZ=IST-1GMT-0,M10.5.0/1,M3.5.0/1 systemd-analyze calendar --base-time='Sat 2025-03-29 22:00:00 UTC' --iterations=5 'hourly' Original form: hourly Normalized form: *-*-* *:00:00 Next elapse: Sat 2025-03-29 23:00:00 GMT (in UTC): Sat 2025-03-29 23:00:00 UTC From now: 13h ago Iteration #2: Sun 2025-03-30 00:00:00 GMT (in UTC): Sun 2025-03-30 00:00:00 UTC From now: 12h ago Iteration #3: Sun 2025-03-30 00:00:00 GMT <-- note every next iteration having the same firing time (in UTC): Sun 2025-03-30 00:00:00 UTC From now: 12h ago ... With this patch: $ env TZ=IST-1GMT-0,M10.5.0/1,M3.5.0/1 systemd-analyze calendar --base-time='Sat 2025-03-29 22:00:00 UTC' --iterations=5 'hourly' Original form: hourly Normalized form: *-*-* *:00:00 Next elapse: Sat 2025-03-29 23:00:00 GMT (in UTC): Sat 2025-03-29 23:00:00 UTC From now: 13h ago Iteration #2: Sun 2025-03-30 00:00:00 GMT (in UTC): Sun 2025-03-30 00:00:00 UTC From now: 12h ago Iteration #3: Sun 2025-03-30 02:00:00 IST <-- the expected 1 hour jump (in UTC): Sun 2025-03-30 01:00:00 UTC From now: 11h ago ... This bug isn't reproduced on Debian and Ubuntu because they mitigate it by using the rearguard version of tzdata. ArchLinux and NixOS don't, so it would cause pid1 to spin during DST transition. This is how the affected tzdata looks like: $ zdump -V -c 2024,2025 Europe/Dublin Europe/Dublin Sun Mar 31 00:59:59 2024 UT = Sun Mar 31 00:59:59 2024 GMT isdst=1 gmtoff=0 Europe/Dublin Sun Mar 31 01:00:00 2024 UT = Sun Mar 31 02:00:00 2024 IST isdst=0 gmtoff=3600 Europe/Dublin Sun Oct 27 00:59:59 2024 UT = Sun Oct 27 01:59:59 2024 IST isdst=0 gmtoff=3600 Europe/Dublin Sun Oct 27 01:00:00 2024 UT = Sun Oct 27 01:00:00 2024 GMT isdst=1 gmtoff=0 Compare it to Europe/London: $ zdump -V -c 2024,2025 Europe/London Europe/London Sun Mar 31 00:59:59 2024 UT = Sun Mar 31 00:59:59 2024 GMT isdst=0 gmtoff=0 Europe/London Sun Mar 31 01:00:00 2024 UT = Sun Mar 31 02:00:00 2024 BST isdst=1 gmtoff=3600 Europe/London Sun Oct 27 00:59:59 2024 UT = Sun Oct 27 01:59:59 2024 BST isdst=1 gmtoff=3600 Europe/London Sun Oct 27 01:00:00 2024 UT = Sun Oct 27 01:00:00 2024 GMT isdst=0 gmtoff=0 Fixes #32039. (cherry picked from commit e4bb033) (cherry picked from commit 07c01efc82d4a239ef0d14da54d36053294ad203) There were some conflicts related to the skipping of 6f5cf41, but the tests pass with and the example output above also looks good, so I think the backport is correct. (cherry picked from commit 1568dea89ebb84ed2c9cf8c45aaf90c07858cbc0) (cherry picked from commit f3dc34e) (cherry picked from commit 2230a5d)
diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c
@@ -1239,14 +1239,43 @@ static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
         return (weekdays_bits & (1 << k));
 }
 
+static int tm_compare(const struct tm *t1, const struct tm *t2) {
+        int r;
+
+        assert(t1);
+        assert(t2);
+
+        r = CMP(t1->tm_year, t2->tm_year);
+        if (r != 0)
+                return r;
+
+        r = CMP(t1->tm_mon, t2->tm_mon);
+        if (r != 0)
+                return r;
+
+        r = CMP(t1->tm_mday, t2->tm_mday);
+        if (r != 0)
+                return r;
+
+        r = CMP(t1->tm_hour, t2->tm_hour);
+        if (r != 0)
+                return r;
+
+        r = CMP(t1->tm_min, t2->tm_min);
+        if (r != 0)
+                return r;
+
+        return CMP(t1->tm_sec, t2->tm_sec);
+}
+
 /* A safety valve: if we get stuck in the calculation, return an error.
  * C.f. https://bugzilla.redhat.com/show_bug.cgi?id=1941335. */
 #define MAX_CALENDAR_ITERATIONS 1000
 
 static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) {
         struct tm c;
-        int tm_usec;
-        int r;
+        int tm_usec, r;
+        bool invalidate_dst = false;
 
         /* Returns -ENOENT if the expression is not going to elapse anymore */
 
@@ -1259,7 +1288,8 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) {
         for (unsigned iteration = 0; iteration < MAX_CALENDAR_ITERATIONS; iteration++) {
                 /* Normalize the current date */
                 (void) mktime_or_timegm(&c, spec->utc);
-                c.tm_isdst = spec->dst;
+                if (!invalidate_dst)
+                        c.tm_isdst = spec->dst;
 
                 c.tm_year += 1900;
                 r = find_matching_component(spec, spec->year, &c, &c.tm_year);
@@ -1349,6 +1379,18 @@ static int find_next(const CalendarSpec *spec, struct tm *tm, usec_t *usec) {
                 if (r == 0)
                         continue;
 
+                r = tm_compare(tm, &c);
+                if (r == 0) {
+                        assert(tm_usec + 1 <= 1000000);
+                        r = CMP(*usec, (usec_t) tm_usec + 1);
+                }
+                if (r >= 0) {
+                        /* We're stuck - advance, let mktime determine DST transition and try again. */
+                        invalidate_dst = true;
+                        c.tm_hour++;
+                        continue;
+                }
+
                 *tm = c;
                 *usec = tm_usec;
                 return 0;
diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c
@@ -48,7 +48,7 @@ static void _test_next(int line, const char *input, const char *new_tz, usec_t a
         if (old_tz)
                 old_tz = strdupa_safe(old_tz);
 
-        if (!isempty(new_tz))
+        if (!isempty(new_tz) && !strchr(new_tz, ','))
                 new_tz = strjoina(":", new_tz);
 
         assert_se(set_unset_env("TZ", new_tz, true) == 0);
@@ -225,6 +225,8 @@ TEST(calendar_spec_next) {
         /* Check that we don't start looping if mktime() moves us backwards */
         test_next("Sun *-*-* 01:00:00 Europe/Dublin", "", 1616412478000000, 1617494400000000);
         test_next("Sun *-*-* 01:00:00 Europe/Dublin", "IST", 1616412478000000, 1617494400000000);
+        /* Europe/Dublin TZ that moves DST backwards */
+        test_next("hourly", "IST-1GMT-0,M10.5.0/1,M3.5.0/1", 1743292800000000, 1743296400000000);
 }
 
 TEST(calendar_spec_from_string) {
