Skip to content

Commit

Permalink
shared/calendarspec: when mktime() moves us backwards, jump forward
Browse files Browse the repository at this point in the history
When trying to calculate the next firing of 'Sun *-*-* 01:00:00', we'd fall
into an infinite loop, because mktime() moves us "backwards":

Before this patch:
tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
...

We rely on mktime() normalizing the time. The man page does not say that it'll
move the time forward, but our algorithm relies on this. So let's catch this
case explicitly.

With this patch:
$ TZ=Europe/Dublin faketime 2021-03-21 build/systemd-analyze calendar --iterations=5 'Sun *-*-* 01:00:00'
Normalized form: Sun *-*-* 01:00:00
    Next elapse: Sun 2021-03-21 01:00:00 GMT
       (in UTC): Sun 2021-03-21 01:00:00 UTC
       From now: 59min left
       Iter. #2: Sun 2021-04-04 01:00:00 IST
       (in UTC): Sun 2021-04-04 00:00:00 UTC
       From now: 1 weeks 6 days left           <---- note the 2 week jump here
       Iter. #3: Sun 2021-04-11 01:00:00 IST
       (in UTC): Sun 2021-04-11 00:00:00 UTC
       From now: 2 weeks 6 days left
       Iter. #4: Sun 2021-04-18 01:00:00 IST
       (in UTC): Sun 2021-04-18 00:00:00 UTC
       From now: 3 weeks 6 days left
       Iter. #5: Sun 2021-04-25 01:00:00 IST
       (in UTC): Sun 2021-04-25 00:00:00 UTC
       From now: 1 months 4 days left

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1941335.

(cherry picked from commit 129cb6e)
(cherry picked from commit e5cf86f)
  • Loading branch information
keszybz committed Mar 23, 2021
1 parent 822d39c commit a8b66ca
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
19 changes: 11 additions & 8 deletions src/shared/calendarspec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1185,15 +1185,18 @@ static int tm_within_bounds(struct tm *tm, bool utc) {
return negative_errno();

/* Did any normalization take place? If so, it was out of bounds before */
bool good = t.tm_year == tm->tm_year &&
t.tm_mon == tm->tm_mon &&
t.tm_mday == tm->tm_mday &&
t.tm_hour == tm->tm_hour &&
t.tm_min == tm->tm_min &&
t.tm_sec == tm->tm_sec;
if (!good)
int cmp = CMP(t.tm_year, tm->tm_year) ?:
CMP(t.tm_mon, tm->tm_mon) ?:
CMP(t.tm_mday, tm->tm_mday) ?:
CMP(t.tm_hour, tm->tm_hour) ?:
CMP(t.tm_min, tm->tm_min) ?:
CMP(t.tm_sec, tm->tm_sec);

if (cmp < 0)
return -EDEADLK; /* Refuse to go backward */
if (cmp > 0)
*tm = t;
return good;
return cmp == 0;
}

static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
Expand Down
3 changes: 3 additions & 0 deletions src/test/test-calendarspec.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ int main(int argc, char* argv[]) {
// Confirm that timezones in the Spec work regardless of current timezone
test_next("2017-09-09 20:42:00 Pacific/Auckland", "", 12345, 1504946520000000);
test_next("2017-09-09 20:42:00 Pacific/Auckland", "EET", 12345, 1504946520000000);
/* 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);

assert_se(calendar_spec_from_string("test", &c) < 0);
assert_se(calendar_spec_from_string(" utc", &c) < 0);
Expand Down
1 change: 1 addition & 0 deletions test/test-functions
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,7 @@ install_zoneinfo() {
inst_any /usr/share/zoneinfo/Asia/Vladivostok
inst_any /usr/share/zoneinfo/Australia/Sydney
inst_any /usr/share/zoneinfo/Europe/Berlin
inst_any /usr/share/zoneinfo/Europe/Dublin
inst_any /usr/share/zoneinfo/Europe/Kiev
inst_any /usr/share/zoneinfo/Pacific/Auckland
inst_any /usr/share/zoneinfo/Pacific/Honolulu
Expand Down

0 comments on commit a8b66ca

Please sign in to comment.