Merge ~enr0n/ubuntu/+source/systemd:ubuntu-focal into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-focal

Proposed by Nick Rosbrook
Status: Merged
Merge reported by: Lukas Märdian
Merged at revision: 48d4e4635719ca0ac0c72b282300d21b36e29e2f
Proposed branch: ~enr0n/ubuntu/+source/systemd:ubuntu-focal
Merge into: ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-focal
Diff against target: 170 lines (+142/-0)
4 files modified
debian/changelog (+11/-0)
debian/patches/lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch (+34/-0)
debian/patches/lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch (+95/-0)
debian/patches/series (+2/-0)
Reviewer Review Type Date Requested Status
Lukas Märdian Approve
Review via email: mp+421860@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Nick Rosbrook (enr0n) wrote :
Revision history for this message
Lukas Märdian (slyon) wrote :

Thank you very much @enr0n!

Both changes look very good and match the upstream fixes. SRU paperwork has been done on the LP bug reports as well. So I'll push the changes into the packaging git, to be staged for whenever we upload the next Focal SRU.

Also, thank you for providing a test-build inside your PPA, this is very helpful. I used it to trigger some autopkgtests against this new version, which are also looking good! https://autopkgtest.ubuntu.com/results/autopkgtest-focal-enr0n-systemd-245/?format=plain

I did a tiny rebase of your commits locally, just to improve the "gbp dch" formatting a little: I moved the (LP: #XXX) reference into the 2nd line of the DCH comment / GIT commit, in order to keep the comment line below 80 chars (tho, that does not work for the automatic gbp FIle: and git URL lines).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index ec7e5c6..9ef5043 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,14 @@
6+systemd (245.4-4ubuntu3.18) UNRELEASED; urgency=medium
7+
8+ * core: make sure we don't get confused when setting TERM for a tty fd (LP: #1959475)
9+ File: debian/patches/lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch
10+ https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=df151f69018f1a040d0b4a7000659e727c2103b5
11+ * shared/calendarspec: when mktime() moves us backwards, jump forward (LP: #1966800)
12+ File: debian/patches/lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch
13+ https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=40471de215fcd95876540990409dcaf78d0b9821
14+
15+ -- Nick Rosbrook <nick.rosbrook@canonical.com> Fri, 29 Apr 2022 15:03:31 -0400
16+
17 systemd (245.4-4ubuntu3.17) focal; urgency=medium
18
19 [ Andy Chi ]
20diff --git a/debian/patches/lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch b/debian/patches/lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch
21new file mode 100644
22index 0000000..ca09348
23--- /dev/null
24+++ b/debian/patches/lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch
25@@ -0,0 +1,34 @@
26+From: Lennart Poettering <lennart@poettering.net>
27+Date: Wed, 22 Apr 2020 21:52:22 +0200
28+Subject: core: make sure we don't get confused when setting TERM for a tty fd
29+
30+Origin: upstream, https://github.com/systemd/systemd/commit/e8cf09b2a2ad0d48e5493050d54251d5f512d9b6
31+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1959475
32+
33+Fixes: #15344
34+---
35+ src/core/execute.c | 9 +++++----
36+ 1 file changed, 5 insertions(+), 4 deletions(-)
37+
38+diff --git a/src/core/execute.c b/src/core/execute.c
39+index a99e746..75aca6b 100644
40+--- a/src/core/execute.c
41++++ b/src/core/execute.c
42+@@ -1785,12 +1785,13 @@ static int build_environment(
43+
44+ tty_path = exec_context_tty_path(c);
45+
46+- /* If we are forked off PID 1 and we are supposed to operate on /dev/console, then let's try to inherit
47+- * the $TERM set for PID 1. This is useful for containers so that the $TERM the container manager
48+- * passes to PID 1 ends up all the way in the console login shown. */
49++ /* If we are forked off PID 1 and we are supposed to operate on /dev/console, then let's try
50++ * to inherit the $TERM set for PID 1. This is useful for containers so that the $TERM the
51++ * container manager passes to PID 1 ends up all the way in the console login shown. */
52+
53+- if (path_equal(tty_path, "/dev/console") && getppid() == 1)
54++ if (path_equal_ptr(tty_path, "/dev/console") && getppid() == 1)
55+ term = getenv("TERM");
56++
57+ if (!term)
58+ term = default_term_for_tty(tty_path);
59+
60diff --git a/debian/patches/lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch b/debian/patches/lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch
61new file mode 100644
62index 0000000..e457503
63--- /dev/null
64+++ b/debian/patches/lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch
65@@ -0,0 +1,95 @@
66+From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
67+Date: Mon, 22 Mar 2021 12:51:47 +0100
68+Subject: shared/calendarspec: when mktime() moves us backwards, jump forward
69+MIME-Version: 1.0
70+Content-Type: text/plain; charset="utf-8"
71+Content-Transfer-Encoding: 8bit
72+
73+Origin: upstream, https://github.com/systemd/systemd/commit/129cb6e249bef30dc33e08f98f0b27a6de976f6f
74+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1966800
75+
76+When trying to calculate the next firing of 'Sun *-*-* 01:00:00', we'd fall
77+into an infinite loop, because mktime() moves us "backwards":
78+
79+Before this patch:
80+tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
81+tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
82+tm_within_bounds: good=0 2021-03-29 01:00:00 → 2021-03-29 00:00:00
83+...
84+
85+We rely on mktime() normalizing the time. The man page does not say that it'll
86+move the time forward, but our algorithm relies on this. So let's catch this
87+case explicitly.
88+
89+With this patch:
90+$ TZ=Europe/Dublin faketime 2021-03-21 build/systemd-analyze calendar --iterations=5 'Sun *-*-* 01:00:00'
91+Normalized form: Sun *-*-* 01:00:00
92+ Next elapse: Sun 2021-03-21 01:00:00 GMT
93+ (in UTC): Sun 2021-03-21 01:00:00 UTC
94+ From now: 59min left
95+ Iter. #2: Sun 2021-04-04 01:00:00 IST
96+ (in UTC): Sun 2021-04-04 00:00:00 UTC
97+ From now: 1 weeks 6 days left <---- note the 2 week jump here
98+ Iter. #3: Sun 2021-04-11 01:00:00 IST
99+ (in UTC): Sun 2021-04-11 00:00:00 UTC
100+ From now: 2 weeks 6 days left
101+ Iter. #4: Sun 2021-04-18 01:00:00 IST
102+ (in UTC): Sun 2021-04-18 00:00:00 UTC
103+ From now: 3 weeks 6 days left
104+ Iter. #5: Sun 2021-04-25 01:00:00 IST
105+ (in UTC): Sun 2021-04-25 00:00:00 UTC
106+ From now: 1 months 4 days left
107+
108+Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1941335.
109+---
110+Dropped the test/test-functions change to resolve git conflict in v245.
111+---
112+ src/shared/calendarspec.c | 19 +++++++++++--------
113+ src/test/test-calendarspec.c | 3 +++
114+ 2 files changed, 14 insertions(+), 8 deletions(-)
115+
116+diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c
117+index 217ab3f..6fb9b67 100644
118+--- a/src/shared/calendarspec.c
119++++ b/src/shared/calendarspec.c
120+@@ -1160,15 +1160,18 @@ static int tm_within_bounds(struct tm *tm, bool utc) {
121+ return negative_errno();
122+
123+ /* Did any normalization take place? If so, it was out of bounds before */
124+- bool good = t.tm_year == tm->tm_year &&
125+- t.tm_mon == tm->tm_mon &&
126+- t.tm_mday == tm->tm_mday &&
127+- t.tm_hour == tm->tm_hour &&
128+- t.tm_min == tm->tm_min &&
129+- t.tm_sec == tm->tm_sec;
130+- if (!good)
131++ int cmp = CMP(t.tm_year, tm->tm_year) ?:
132++ CMP(t.tm_mon, tm->tm_mon) ?:
133++ CMP(t.tm_mday, tm->tm_mday) ?:
134++ CMP(t.tm_hour, tm->tm_hour) ?:
135++ CMP(t.tm_min, tm->tm_min) ?:
136++ CMP(t.tm_sec, tm->tm_sec);
137++
138++ if (cmp < 0)
139++ return -EDEADLK; /* Refuse to go backward */
140++ if (cmp > 0)
141+ *tm = t;
142+- return good;
143++ return cmp == 0;
144+ }
145+
146+ static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
147+diff --git a/src/test/test-calendarspec.c b/src/test/test-calendarspec.c
148+index 9c2be7f..9e96cdb 100644
149+--- a/src/test/test-calendarspec.c
150++++ b/src/test/test-calendarspec.c
151+@@ -216,6 +216,9 @@ int main(int argc, char* argv[]) {
152+ // Confirm that timezones in the Spec work regardless of current timezone
153+ test_next("2017-09-09 20:42:00 Pacific/Auckland", "", 12345, 1504946520000000);
154+ test_next("2017-09-09 20:42:00 Pacific/Auckland", "EET", 12345, 1504946520000000);
155++ /* Check that we don't start looping if mktime() moves us backwards */
156++ test_next("Sun *-*-* 01:00:00 Europe/Dublin", "", 1616412478000000, 1617494400000000);
157++ test_next("Sun *-*-* 01:00:00 Europe/Dublin", "IST", 1616412478000000, 1617494400000000);
158+
159+ assert_se(calendar_spec_from_string("test", &c) < 0);
160+ assert_se(calendar_spec_from_string(" utc", &c) < 0);
161diff --git a/debian/patches/series b/debian/patches/series
162index 8718b3b..6441bb3 100644
163--- a/debian/patches/series
164+++ b/debian/patches/series
165@@ -175,3 +175,5 @@ lp1958284-core-move-reset_arguments-to-the-end-of-main-s-finish.patch
166 pid1-set-SYSTEMD_NSS_DYNAMIC_BYPASS-1-env-var-for-dbus-da.patch
167 hwdb-Add-mic-mute-key-mapping-for-HP-Elite-x360.patch
168 lp1966179-add-more-hp-dmi-to-unblock-intel-hid-event.patch
169+lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch
170+lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch

Subscribers

People subscribed via source and target branches