Merge ~enr0n/ubuntu/+source/systemd:ubuntu-kinetic-sru into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-kinetic
- Git
- lp:~enr0n/ubuntu/+source/systemd
- ubuntu-kinetic-sru
- Merge into ubuntu-kinetic
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 74ae8a98d5e803a6513ad3bd0ccad4e91b235397 | ||||||||
Proposed branch: | ~enr0n/ubuntu/+source/systemd:ubuntu-kinetic-sru | ||||||||
Merge into: | ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-kinetic | ||||||||
Diff against target: |
983 lines (+913/-0) 11 files modified
debian/changelog (+31/-0) debian/patches/CVE-2022-4415.patch (+380/-0) debian/patches/CVE-2022-45873.patch (+115/-0) debian/patches/backport-for-CVE-2022-45873.patch (+45/-0) debian/patches/lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch (+66/-0) debian/patches/lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch (+54/-0) debian/patches/lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch (+64/-0) debian/patches/lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch (+61/-0) debian/patches/lp2002445/udev-net-allow-new-link-name-as-an-altname-before-renamin.patch (+34/-0) debian/patches/lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch (+54/-0) debian/patches/series (+9/-0) |
||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Lukas Märdian | Approve | ||
Review via email: mp+438555@code.launchpad.net |
Commit message
Description of the change
These autopkgtest results are from a previous build, i.e. before rebasing on the security update. The networkd-test.py failure is already present and will not be considered a regression. The unit-tests failure in armhf is fixed in my latest build, though I did not re-trigger the autopkgtest because it was a known issue, and I just forgot to include the fix in the tested build.
systemd 251.4-1ubuntu7.
systemd 251.4-1ubuntu7.
systemd 251.4-1ubuntu7.
systemd 251.4-1ubuntu7.
systemd 251.4-1ubuntu7.
Lukas Märdian (slyon) wrote : | # |
Nick Rosbrook (enr0n) wrote : | # |
I have opened bug 2009859 to track the issue.
Preview Diff
1 | diff --git a/debian/changelog b/debian/changelog | |||
2 | index c7ddcfc..2fe2841 100644 | |||
3 | --- a/debian/changelog | |||
4 | +++ b/debian/changelog | |||
5 | @@ -1,3 +1,34 @@ | |||
6 | 1 | systemd (251.4-1ubuntu7.2) kinetic; urgency=medium | ||
7 | 2 | |||
8 | 3 | * network/dhcp4: accept local subnet routes from DHCP (LP: #2004478) | ||
9 | 4 | File: debian/patches/lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch | ||
10 | 5 | https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=751bac59b405025964d76c4ef8e0603457a605af | ||
11 | 6 | * udev: avoid NIC renaming race with kernel (LP: #2002445) | ||
12 | 7 | Files: | ||
13 | 8 | - debian/patches/lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch | ||
14 | 9 | - debian/patches/lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch | ||
15 | 10 | - debian/patches/lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch | ||
16 | 11 | - debian/patches/lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch | ||
17 | 12 | - debian/patches/lp2002445/udev-net-allow-new-link-name-as-an-altname-before-renamin.patch | ||
18 | 13 | https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=ffb1e85fdd3f0fe9b158b28a95cfa6d241fcbe70 | ||
19 | 14 | |||
20 | 15 | -- Nick Rosbrook <nick.rosbrook@canonical.com> Wed, 08 Mar 2023 12:07:55 -0500 | ||
21 | 16 | |||
22 | 17 | systemd (251.4-1ubuntu7.1) kinetic-security; urgency=medium | ||
23 | 18 | |||
24 | 19 | * SECURITY UPDATE: information leak vulnerability in systemd-coredump | ||
25 | 20 | - debian/patches/CVE-2022-4415.patch: do not allow user to access | ||
26 | 21 | coredumps with changed uid/gid/capabilities | ||
27 | 22 | - CVE-2022-4415 | ||
28 | 23 | * SECURITY UPDATE: DoS vulnerability in systemd-coredump | ||
29 | 24 | - debian/patches/backport-for-CVE-2022-45873.patch: allow | ||
30 | 25 | json_variant_dump() to return an error | ||
31 | 26 | - debian/patches/CVE-2022-45873.patch: avoid deadlock when passing | ||
32 | 27 | processed backtrace data | ||
33 | 28 | - CVE-2022-45873 | ||
34 | 29 | |||
35 | 30 | -- Nishit Majithia <nishit.majithia@canonical.com> Thu, 02 Mar 2023 18:28:02 +0530 | ||
36 | 31 | |||
37 | 1 | systemd (251.4-1ubuntu7) kinetic; urgency=medium | 32 | systemd (251.4-1ubuntu7) kinetic; urgency=medium |
38 | 2 | 33 | ||
39 | 3 | [ Nick Rosbrook ] | 34 | [ Nick Rosbrook ] |
40 | diff --git a/debian/patches/CVE-2022-4415.patch b/debian/patches/CVE-2022-4415.patch | |||
41 | 4 | new file mode 100644 | 35 | new file mode 100644 |
42 | index 0000000..cc12be9 | |||
43 | --- /dev/null | |||
44 | +++ b/debian/patches/CVE-2022-4415.patch | |||
45 | @@ -0,0 +1,380 @@ | |||
46 | 1 | Origin: backport, https://github.com/systemd/systemd-stable/commit/efca5283dc791a07171f80eef84e14fdb58fad57 | ||
47 | 2 | |||
48 | 3 | From efca5283dc791a07171f80eef84e14fdb58fad57 Mon Sep 17 00:00:00 2001 | ||
49 | 4 | From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> | ||
50 | 5 | Date: Mon, 28 Nov 2022 12:12:55 +0100 | ||
51 | 6 | Subject: [PATCH] coredump: do not allow user to access coredumps with changed | ||
52 | 7 | uid/gid/capabilities | ||
53 | 8 | |||
54 | 9 | When the user starts a program which elevates its permissions via setuid, | ||
55 | 10 | setgid, or capabilities set on the file, it may access additional information | ||
56 | 11 | which would then be visible in the coredump. We shouldn't make the the coredump | ||
57 | 12 | visible to the user in such cases. | ||
58 | 13 | |||
59 | 14 | Reported-by: Matthias Gerstner <mgerstner@suse.de> | ||
60 | 15 | |||
61 | 16 | This reads the /proc/<pid>/auxv file and attaches it to the process metadata as | ||
62 | 17 | PROC_AUXV. Before the coredump is submitted, it is parsed and if either | ||
63 | 18 | at_secure was set (which the kernel will do for processes that are setuid, | ||
64 | 19 | setgid, or setcap), or if the effective uid/gid don't match uid/gid, the file | ||
65 | 20 | is not made accessible to the user. If we can't access this data, we assume the | ||
66 | 21 | file should not be made accessible either. In principle we could also access | ||
67 | 22 | the auxv data from a note in the core file, but that is much more complex and | ||
68 | 23 | it seems better to use the stand-alone file that is provided by the kernel. | ||
69 | 24 | |||
70 | 25 | Attaching auxv is both convient for this patch (because this way it's passed | ||
71 | 26 | between the stages along with other fields), but I think it makes sense to save | ||
72 | 27 | it in general. | ||
73 | 28 | |||
74 | 29 | We use the information early in the core file to figure out if the program was | ||
75 | 30 | 32-bit or 64-bit and its endianness. This way we don't need heuristics to guess | ||
76 | 31 | whether the format of the auxv structure. This test might reject some cases on | ||
77 | 32 | fringe architecutes. But the impact would be limited: we just won't grant the | ||
78 | 33 | user permissions to view the coredump file. If people report that we're missing | ||
79 | 34 | some cases, we can always enhance this to support more architectures. | ||
80 | 35 | |||
81 | 36 | I tested auxv parsing on amd64, 32-bit program on amd64, arm64, arm32, and | ||
82 | 37 | ppc64el, but not the whole coredump handling. | ||
83 | 38 | |||
84 | 39 | (cherry picked from commit 3e4d0f6cf99f8677edd6a237382a65bfe758de03) | ||
85 | 40 | (cherry picked from commit 9b75a3d0502d6741c8ecb7175794345f8eb3827c) | ||
86 | 41 | --- | ||
87 | 42 | src/basic/io-util.h | 9 ++ | ||
88 | 43 | src/coredump/coredump.c | 196 +++++++++++++++++++++++++++++++++++++--- | ||
89 | 44 | 2 files changed, 192 insertions(+), 13 deletions(-) | ||
90 | 45 | |||
91 | 46 | --- systemd-251.4.orig/src/basic/io-util.h | ||
92 | 47 | +++ systemd-251.4/src/basic/io-util.h | ||
93 | 48 | @@ -91,7 +91,16 @@ struct iovec_wrapper *iovw_new(void); | ||
94 | 49 | struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw); | ||
95 | 50 | struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw); | ||
96 | 51 | void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors); | ||
97 | 52 | + | ||
98 | 53 | int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len); | ||
99 | 54 | +static inline int iovw_consume(struct iovec_wrapper *iovw, void *data, size_t len) { | ||
100 | 55 | + /* Move data into iovw or free on error */ | ||
101 | 56 | + int r = iovw_put(iovw, data, len); | ||
102 | 57 | + if (r < 0) | ||
103 | 58 | + free(data); | ||
104 | 59 | + return r; | ||
105 | 60 | +} | ||
106 | 61 | + | ||
107 | 62 | int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const char *value); | ||
108 | 63 | int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value); | ||
109 | 64 | void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new); | ||
110 | 65 | --- systemd-251.4.orig/src/coredump/coredump.c | ||
111 | 66 | +++ systemd-251.4/src/coredump/coredump.c | ||
112 | 67 | @@ -4,6 +4,7 @@ | ||
113 | 68 | #include <stdio.h> | ||
114 | 69 | #include <sys/prctl.h> | ||
115 | 70 | #include <sys/statvfs.h> | ||
116 | 71 | +#include <sys/auxv.h> | ||
117 | 72 | #include <sys/xattr.h> | ||
118 | 73 | #include <unistd.h> | ||
119 | 74 | |||
120 | 75 | @@ -105,6 +106,7 @@ enum { | ||
121 | 76 | |||
122 | 77 | META_EXE = _META_MANDATORY_MAX, | ||
123 | 78 | META_UNIT, | ||
124 | 79 | + META_PROC_AUXV, | ||
125 | 80 | _META_MAX | ||
126 | 81 | }; | ||
127 | 82 | |||
128 | 83 | @@ -119,10 +121,12 @@ static const char * const meta_field_nam | ||
129 | 84 | [META_COMM] = "COREDUMP_COMM=", | ||
130 | 85 | [META_EXE] = "COREDUMP_EXE=", | ||
131 | 86 | [META_UNIT] = "COREDUMP_UNIT=", | ||
132 | 87 | + [META_PROC_AUXV] = "COREDUMP_PROC_AUXV=", | ||
133 | 88 | }; | ||
134 | 89 | |||
135 | 90 | typedef struct Context { | ||
136 | 91 | const char *meta[_META_MAX]; | ||
137 | 92 | + size_t meta_size[_META_MAX]; | ||
138 | 93 | pid_t pid; | ||
139 | 94 | bool is_pid1; | ||
140 | 95 | bool is_journald; | ||
141 | 96 | @@ -184,13 +188,16 @@ static uint64_t storage_size_max(void) { | ||
142 | 97 | return 0; | ||
143 | 98 | } | ||
144 | 99 | |||
145 | 100 | -static int fix_acl(int fd, uid_t uid) { | ||
146 | 101 | +static int fix_acl(int fd, uid_t uid, bool allow_user) { | ||
147 | 102 | + assert(fd >= 0); | ||
148 | 103 | + assert(uid_is_valid(uid)); | ||
149 | 104 | |||
150 | 105 | #if HAVE_ACL | ||
151 | 106 | int r; | ||
152 | 107 | |||
153 | 108 | - assert(fd >= 0); | ||
154 | 109 | - assert(uid_is_valid(uid)); | ||
155 | 110 | + /* We don't allow users to read coredumps if the uid or capabilities were changed. */ | ||
156 | 111 | + if (!allow_user) | ||
157 | 112 | + return 0; | ||
158 | 113 | |||
159 | 114 | if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY) | ||
160 | 115 | return 0; | ||
161 | 116 | @@ -250,7 +257,8 @@ static int fix_permissions( | ||
162 | 117 | const char *filename, | ||
163 | 118 | const char *target, | ||
164 | 119 | const Context *context, | ||
165 | 120 | - uid_t uid) { | ||
166 | 121 | + uid_t uid, | ||
167 | 122 | + bool allow_user) { | ||
168 | 123 | |||
169 | 124 | int r; | ||
170 | 125 | |||
171 | 126 | @@ -260,7 +268,7 @@ static int fix_permissions( | ||
172 | 127 | |||
173 | 128 | /* Ignore errors on these */ | ||
174 | 129 | (void) fchmod(fd, 0640); | ||
175 | 130 | - (void) fix_acl(fd, uid); | ||
176 | 131 | + (void) fix_acl(fd, uid, allow_user); | ||
177 | 132 | (void) fix_xattr(fd, context); | ||
178 | 133 | |||
179 | 134 | r = fsync_full(fd); | ||
180 | 135 | @@ -330,6 +338,153 @@ static int make_filename(const Context * | ||
181 | 136 | return 0; | ||
182 | 137 | } | ||
183 | 138 | |||
184 | 139 | +static int parse_auxv64( | ||
185 | 140 | + const uint64_t *auxv, | ||
186 | 141 | + size_t size_bytes, | ||
187 | 142 | + int *at_secure, | ||
188 | 143 | + uid_t *uid, | ||
189 | 144 | + uid_t *euid, | ||
190 | 145 | + gid_t *gid, | ||
191 | 146 | + gid_t *egid) { | ||
192 | 147 | + | ||
193 | 148 | + assert(auxv || size_bytes == 0); | ||
194 | 149 | + | ||
195 | 150 | + if (size_bytes % (2 * sizeof(uint64_t)) != 0) | ||
196 | 151 | + return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes); | ||
197 | 152 | + | ||
198 | 153 | + size_t words = size_bytes / sizeof(uint64_t); | ||
199 | 154 | + | ||
200 | 155 | + /* Note that we set output variables even on error. */ | ||
201 | 156 | + | ||
202 | 157 | + for (size_t i = 0; i + 1 < words; i += 2) | ||
203 | 158 | + switch (auxv[i]) { | ||
204 | 159 | + case AT_SECURE: | ||
205 | 160 | + *at_secure = auxv[i + 1] != 0; | ||
206 | 161 | + break; | ||
207 | 162 | + case AT_UID: | ||
208 | 163 | + *uid = auxv[i + 1]; | ||
209 | 164 | + break; | ||
210 | 165 | + case AT_EUID: | ||
211 | 166 | + *euid = auxv[i + 1]; | ||
212 | 167 | + break; | ||
213 | 168 | + case AT_GID: | ||
214 | 169 | + *gid = auxv[i + 1]; | ||
215 | 170 | + break; | ||
216 | 171 | + case AT_EGID: | ||
217 | 172 | + *egid = auxv[i + 1]; | ||
218 | 173 | + break; | ||
219 | 174 | + case AT_NULL: | ||
220 | 175 | + if (auxv[i + 1] != 0) | ||
221 | 176 | + goto error; | ||
222 | 177 | + return 0; | ||
223 | 178 | + } | ||
224 | 179 | + error: | ||
225 | 180 | + return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), | ||
226 | 181 | + "AT_NULL terminator not found, cannot parse auxv structure."); | ||
227 | 182 | +} | ||
228 | 183 | + | ||
229 | 184 | +static int parse_auxv32( | ||
230 | 185 | + const uint32_t *auxv, | ||
231 | 186 | + size_t size_bytes, | ||
232 | 187 | + int *at_secure, | ||
233 | 188 | + uid_t *uid, | ||
234 | 189 | + uid_t *euid, | ||
235 | 190 | + gid_t *gid, | ||
236 | 191 | + gid_t *egid) { | ||
237 | 192 | + | ||
238 | 193 | + assert(auxv || size_bytes == 0); | ||
239 | 194 | + | ||
240 | 195 | + size_t words = size_bytes / sizeof(uint32_t); | ||
241 | 196 | + | ||
242 | 197 | + if (size_bytes % (2 * sizeof(uint32_t)) != 0) | ||
243 | 198 | + return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Incomplete auxv structure (%zu bytes).", size_bytes); | ||
244 | 199 | + | ||
245 | 200 | + /* Note that we set output variables even on error. */ | ||
246 | 201 | + | ||
247 | 202 | + for (size_t i = 0; i + 1 < words; i += 2) | ||
248 | 203 | + switch (auxv[i]) { | ||
249 | 204 | + case AT_SECURE: | ||
250 | 205 | + *at_secure = auxv[i + 1] != 0; | ||
251 | 206 | + break; | ||
252 | 207 | + case AT_UID: | ||
253 | 208 | + *uid = auxv[i + 1]; | ||
254 | 209 | + break; | ||
255 | 210 | + case AT_EUID: | ||
256 | 211 | + *euid = auxv[i + 1]; | ||
257 | 212 | + break; | ||
258 | 213 | + case AT_GID: | ||
259 | 214 | + *gid = auxv[i + 1]; | ||
260 | 215 | + break; | ||
261 | 216 | + case AT_EGID: | ||
262 | 217 | + *egid = auxv[i + 1]; | ||
263 | 218 | + break; | ||
264 | 219 | + case AT_NULL: | ||
265 | 220 | + if (auxv[i + 1] != 0) | ||
266 | 221 | + goto error; | ||
267 | 222 | + return 0; | ||
268 | 223 | + } | ||
269 | 224 | + error: | ||
270 | 225 | + return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), | ||
271 | 226 | + "AT_NULL terminator not found, cannot parse auxv structure."); | ||
272 | 227 | +} | ||
273 | 228 | + | ||
274 | 229 | +static int grant_user_access(int core_fd, const Context *context) { | ||
275 | 230 | + int at_secure = -1; | ||
276 | 231 | + uid_t uid = UID_INVALID, euid = UID_INVALID; | ||
277 | 232 | + uid_t gid = GID_INVALID, egid = GID_INVALID; | ||
278 | 233 | + int r; | ||
279 | 234 | + | ||
280 | 235 | + assert(core_fd >= 0); | ||
281 | 236 | + assert(context); | ||
282 | 237 | + | ||
283 | 238 | + if (!context->meta[META_PROC_AUXV]) | ||
284 | 239 | + return log_warning_errno(SYNTHETIC_ERRNO(ENODATA), "No auxv data, not adjusting permissions."); | ||
285 | 240 | + | ||
286 | 241 | + uint8_t elf[EI_NIDENT]; | ||
287 | 242 | + errno = 0; | ||
288 | 243 | + if (pread(core_fd, &elf, sizeof(elf), 0) != sizeof(elf)) | ||
289 | 244 | + return log_warning_errno(errno_or_else(EIO), | ||
290 | 245 | + "Failed to pread from coredump fd: %s", errno != 0 ? strerror_safe(errno) : "Unexpected EOF"); | ||
291 | 246 | + | ||
292 | 247 | + if (elf[EI_MAG0] != ELFMAG0 || | ||
293 | 248 | + elf[EI_MAG1] != ELFMAG1 || | ||
294 | 249 | + elf[EI_MAG2] != ELFMAG2 || | ||
295 | 250 | + elf[EI_MAG3] != ELFMAG3 || | ||
296 | 251 | + elf[EI_VERSION] != EV_CURRENT) | ||
297 | 252 | + return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), | ||
298 | 253 | + "Core file does not have ELF header, not adjusting permissions."); | ||
299 | 254 | + if (!IN_SET(elf[EI_CLASS], ELFCLASS32, ELFCLASS64) || | ||
300 | 255 | + !IN_SET(elf[EI_DATA], ELFDATA2LSB, ELFDATA2MSB)) | ||
301 | 256 | + return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), | ||
302 | 257 | + "Core file has strange ELF class, not adjusting permissions."); | ||
303 | 258 | + | ||
304 | 259 | + if ((elf[EI_DATA] == ELFDATA2LSB) != (__BYTE_ORDER == __LITTLE_ENDIAN)) | ||
305 | 260 | + return log_info_errno(SYNTHETIC_ERRNO(EUCLEAN), | ||
306 | 261 | + "Core file has non-native endianness, not adjusting permissions."); | ||
307 | 262 | + | ||
308 | 263 | + if (elf[EI_CLASS] == ELFCLASS64) | ||
309 | 264 | + r = parse_auxv64((const uint64_t*) context->meta[META_PROC_AUXV], | ||
310 | 265 | + context->meta_size[META_PROC_AUXV], | ||
311 | 266 | + &at_secure, &uid, &euid, &gid, &egid); | ||
312 | 267 | + else | ||
313 | 268 | + r = parse_auxv32((const uint32_t*) context->meta[META_PROC_AUXV], | ||
314 | 269 | + context->meta_size[META_PROC_AUXV], | ||
315 | 270 | + &at_secure, &uid, &euid, &gid, &egid); | ||
316 | 271 | + if (r < 0) | ||
317 | 272 | + return r; | ||
318 | 273 | + | ||
319 | 274 | + /* We allow access if we got all the data and at_secure is not set and | ||
320 | 275 | + * the uid/gid matches euid/egid. */ | ||
321 | 276 | + bool ret = | ||
322 | 277 | + at_secure == 0 && | ||
323 | 278 | + uid != UID_INVALID && euid != UID_INVALID && uid == euid && | ||
324 | 279 | + gid != GID_INVALID && egid != GID_INVALID && gid == egid; | ||
325 | 280 | + log_debug("Will %s access (uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)", | ||
326 | 281 | + ret ? "permit" : "restrict", | ||
327 | 282 | + uid, euid, gid, egid, yes_no(at_secure)); | ||
328 | 283 | + return ret; | ||
329 | 284 | +} | ||
330 | 285 | + | ||
331 | 286 | static int save_external_coredump( | ||
332 | 287 | const Context *context, | ||
333 | 288 | int input_fd, | ||
334 | 289 | @@ -452,6 +607,8 @@ static int save_external_coredump( | ||
335 | 290 | context->meta[META_ARGV_PID], context->meta[META_COMM]); | ||
336 | 291 | truncated = r == 1; | ||
337 | 292 | |||
338 | 293 | + bool allow_user = grant_user_access(fd, context) > 0; | ||
339 | 294 | + | ||
340 | 295 | #if HAVE_COMPRESSION | ||
341 | 296 | if (arg_compress) { | ||
342 | 297 | _cleanup_(unlink_and_freep) char *tmp_compressed = NULL; | ||
343 | 298 | @@ -489,7 +646,7 @@ static int save_external_coredump( | ||
344 | 299 | uncompressed_size += partial_uncompressed_size; | ||
345 | 300 | } | ||
346 | 301 | |||
347 | 302 | - r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid); | ||
348 | 303 | + r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid, allow_user); | ||
349 | 304 | if (r < 0) | ||
350 | 305 | return r; | ||
351 | 306 | |||
352 | 307 | @@ -516,7 +673,7 @@ static int save_external_coredump( | ||
353 | 308 | "SIZE_LIMIT=%"PRIu64, max_size, | ||
354 | 309 | "MESSAGE_ID=" SD_MESSAGE_TRUNCATED_CORE_STR); | ||
355 | 310 | |||
356 | 311 | - r = fix_permissions(fd, tmp, fn, context, uid); | ||
357 | 312 | + r = fix_permissions(fd, tmp, fn, context, uid, allow_user); | ||
358 | 313 | if (r < 0) | ||
359 | 314 | return log_error_errno(r, "Failed to fix permissions and finalize coredump %s into %s: %m", coredump_tmpfile_name(tmp), fn); | ||
360 | 315 | |||
361 | 316 | @@ -764,7 +921,7 @@ static int change_uid_gid(const Context | ||
362 | 317 | } | ||
363 | 318 | |||
364 | 319 | static int submit_coredump( | ||
365 | 320 | - Context *context, | ||
366 | 321 | + const Context *context, | ||
367 | 322 | struct iovec_wrapper *iovw, | ||
368 | 323 | int input_fd) { | ||
369 | 324 | |||
370 | 325 | @@ -925,16 +1082,15 @@ static int save_context(Context *context | ||
371 | 326 | struct iovec *iovec = iovw->iovec + n; | ||
372 | 327 | |||
373 | 328 | for (size_t i = 0; i < ELEMENTSOF(meta_field_names); i++) { | ||
374 | 329 | - char *p; | ||
375 | 330 | - | ||
376 | 331 | /* Note that these strings are NUL terminated, because we made sure that a | ||
377 | 332 | * trailing NUL byte is in the buffer, though not included in the iov_len | ||
378 | 333 | * count (see process_socket() and gather_pid_metadata_*()) */ | ||
379 | 334 | assert(((char*) iovec->iov_base)[iovec->iov_len] == 0); | ||
380 | 335 | |||
381 | 336 | - p = startswith(iovec->iov_base, meta_field_names[i]); | ||
382 | 337 | + const char *p = startswith(iovec->iov_base, meta_field_names[i]); | ||
383 | 338 | if (p) { | ||
384 | 339 | context->meta[i] = p; | ||
385 | 340 | + context->meta_size[i] = iovec->iov_len - strlen(meta_field_names[i]); | ||
386 | 341 | count++; | ||
387 | 342 | break; | ||
388 | 343 | } | ||
389 | 344 | @@ -1176,6 +1332,7 @@ static int gather_pid_metadata(struct io | ||
390 | 345 | uid_t owner_uid; | ||
391 | 346 | pid_t pid; | ||
392 | 347 | char *t; | ||
393 | 348 | + size_t size; | ||
394 | 349 | const char *p; | ||
395 | 350 | int r; | ||
396 | 351 | |||
397 | 352 | @@ -1240,13 +1397,26 @@ static int gather_pid_metadata(struct io | ||
398 | 353 | (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t); | ||
399 | 354 | |||
400 | 355 | p = procfs_file_alloca(pid, "cgroup"); | ||
401 | 356 | - if (read_full_virtual_file(p, &t, NULL) >=0) | ||
402 | 357 | + if (read_full_virtual_file(p, &t, NULL) >= 0) | ||
403 | 358 | (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t); | ||
404 | 359 | |||
405 | 360 | p = procfs_file_alloca(pid, "mountinfo"); | ||
406 | 361 | - if (read_full_virtual_file(p, &t, NULL) >=0) | ||
407 | 362 | + if (read_full_virtual_file(p, &t, NULL) >= 0) | ||
408 | 363 | (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t); | ||
409 | 364 | |||
410 | 365 | + /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */ | ||
411 | 366 | + p = procfs_file_alloca(pid, "auxv"); | ||
412 | 367 | + if (read_full_virtual_file(p, &t, &size) >= 0) { | ||
413 | 368 | + char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1); | ||
414 | 369 | + if (buf) { | ||
415 | 370 | + /* Add a dummy terminator to make save_context() happy. */ | ||
416 | 371 | + *((uint8_t*) mempcpy(stpcpy(buf, "COREDUMP_PROC_AUXV="), t, size)) = '\0'; | ||
417 | 372 | + (void) iovw_consume(iovw, buf, size + strlen("COREDUMP_PROC_AUXV=")); | ||
418 | 373 | + } | ||
419 | 374 | + | ||
420 | 375 | + free(t); | ||
421 | 376 | + } | ||
422 | 377 | + | ||
423 | 378 | if (get_process_cwd(pid, &t) >= 0) | ||
424 | 379 | (void) iovw_put_string_field_free(iovw, "COREDUMP_CWD=", t); | ||
425 | 380 | |||
426 | diff --git a/debian/patches/CVE-2022-45873.patch b/debian/patches/CVE-2022-45873.patch | |||
427 | 0 | new file mode 100644 | 381 | new file mode 100644 |
428 | index 0000000..2badbcb | |||
429 | --- /dev/null | |||
430 | +++ b/debian/patches/CVE-2022-45873.patch | |||
431 | @@ -0,0 +1,115 @@ | |||
432 | 1 | From 076b807be472630692c5348c60d0c2b7b28ad437 Mon Sep 17 00:00:00 2001 | ||
433 | 2 | From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> | ||
434 | 3 | Date: Tue, 18 Oct 2022 18:23:53 +0200 | ||
435 | 4 | Subject: [PATCH] coredump: avoid deadlock when passing processed backtrace | ||
436 | 5 | data | ||
437 | 6 | |||
438 | 7 | We would deadlock when passing the data back from the forked-off process that | ||
439 | 8 | was doing backtrace generation back to the coredump parent. This is because we | ||
440 | 9 | fork the child and wait for it to exit. The child tries to write too much data | ||
441 | 10 | to the output pipe, and and after the first 64k blocks on the parent because | ||
442 | 11 | the pipe is full. The bug surfaced in Fedora because of a combination of four | ||
443 | 12 | factors: | ||
444 | 13 | - 87707784c70dc9894ec613df0a6e75e732a362a3 was backported to v251.5, which | ||
445 | 14 | allowed coredump processing to be successful. | ||
446 | 15 | - 1a0281a3ebf4f8c16d40aa9e63103f16cd23bb2a was NOT backported, so the output | ||
447 | 16 | was very verbose. | ||
448 | 17 | - Fedora has the ELF package metadata available, so a lot of output can be | ||
449 | 18 | generated. Most other distros just don't have the information. | ||
450 | 19 | - gnome-calendar crashes and has a bazillion modules and 69596 bytes of output | ||
451 | 20 | are generated for it. | ||
452 | 21 | |||
453 | 22 | Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2135778. | ||
454 | 23 | |||
455 | 24 | The code is changed to try to write data opportunistically. If we get partial | ||
456 | 25 | information, that is still logged. In is generally better to log partial | ||
457 | 26 | backtrace information than nothing at all. | ||
458 | 27 | --- | ||
459 | 28 | src/shared/elf-util.c | 37 +++++++++++++++++++++++++++++++------ | ||
460 | 29 | 1 file changed, 31 insertions(+), 6 deletions(-) | ||
461 | 30 | |||
462 | 31 | --- systemd-251.4.orig/src/shared/elf-util.c | ||
463 | 32 | +++ systemd-251.4/src/shared/elf-util.c | ||
464 | 33 | @@ -30,6 +30,9 @@ | ||
465 | 34 | #define THREADS_MAX 64 | ||
466 | 35 | #define ELF_PACKAGE_METADATA_ID 0xcafe1a7e | ||
467 | 36 | |||
468 | 37 | +/* The amount of data we're willing to write to each of the output pipes. */ | ||
469 | 38 | +#define COREDUMP_PIPE_MAX (1024*1024U) | ||
470 | 39 | + | ||
471 | 40 | static void *dw_dl = NULL; | ||
472 | 41 | static void *elf_dl = NULL; | ||
473 | 42 | |||
474 | 43 | @@ -704,13 +707,13 @@ int parse_elf_object(int fd, const char | ||
475 | 44 | return r; | ||
476 | 45 | |||
477 | 46 | if (ret) { | ||
478 | 47 | - r = RET_NERRNO(pipe2(return_pipe, O_CLOEXEC)); | ||
479 | 48 | + r = RET_NERRNO(pipe2(return_pipe, O_CLOEXEC|O_NONBLOCK)); | ||
480 | 49 | if (r < 0) | ||
481 | 50 | return r; | ||
482 | 51 | } | ||
483 | 52 | |||
484 | 53 | if (ret_package_metadata) { | ||
485 | 54 | - r = RET_NERRNO(pipe2(json_pipe, O_CLOEXEC)); | ||
486 | 55 | + r = RET_NERRNO(pipe2(json_pipe, O_CLOEXEC|O_NONBLOCK)); | ||
487 | 56 | if (r < 0) | ||
488 | 57 | return r; | ||
489 | 58 | } | ||
490 | 59 | @@ -754,8 +757,24 @@ int parse_elf_object(int fd, const char | ||
491 | 60 | goto child_fail; | ||
492 | 61 | |||
493 | 62 | if (buf) { | ||
494 | 63 | - r = loop_write(return_pipe[1], buf, strlen(buf), false); | ||
495 | 64 | - if (r < 0) | ||
496 | 65 | + size_t len = strlen(buf); | ||
497 | 66 | + | ||
498 | 67 | + if (len > COREDUMP_PIPE_MAX) { | ||
499 | 68 | + /* This is iffy. A backtrace can be a few hundred kilobytes, but too much is | ||
500 | 69 | + * too much. Let's log a warning and ignore the rest. */ | ||
501 | 70 | + log_warning("Generated backtrace is %zu bytes (more than the limit of %u bytes), backtrace will be truncated.", | ||
502 | 71 | + len, COREDUMP_PIPE_MAX); | ||
503 | 72 | + len = COREDUMP_PIPE_MAX; | ||
504 | 73 | + } | ||
505 | 74 | + | ||
506 | 75 | + /* Bump the space for the returned string. | ||
507 | 76 | + * Failure is ignored, because partial output is still useful. */ | ||
508 | 77 | + (void) fcntl(return_pipe[1], F_SETPIPE_SZ, len); | ||
509 | 78 | + | ||
510 | 79 | + r = loop_write(return_pipe[1], buf, len, false); | ||
511 | 80 | + if (r == -EAGAIN) | ||
512 | 81 | + log_warning("Write failed, backtrace will be truncated."); | ||
513 | 82 | + else if (r < 0) | ||
514 | 83 | goto child_fail; | ||
515 | 84 | |||
516 | 85 | return_pipe[1] = safe_close(return_pipe[1]); | ||
517 | 86 | @@ -764,13 +783,19 @@ int parse_elf_object(int fd, const char | ||
518 | 87 | if (package_metadata) { | ||
519 | 88 | _cleanup_fclose_ FILE *json_out = NULL; | ||
520 | 89 | |||
521 | 90 | + /* Bump the space for the returned string. We don't know how much space we'll need in | ||
522 | 91 | + * advance, so we'll just try to write as much as possible and maybe fail later. */ | ||
523 | 92 | + (void) fcntl(json_pipe[1], F_SETPIPE_SZ, COREDUMP_PIPE_MAX); | ||
524 | 93 | + | ||
525 | 94 | json_out = take_fdopen(&json_pipe[1], "w"); | ||
526 | 95 | if (!json_out) { | ||
527 | 96 | r = -errno; | ||
528 | 97 | goto child_fail; | ||
529 | 98 | } | ||
530 | 99 | |||
531 | 100 | - json_variant_dump(package_metadata, JSON_FORMAT_FLUSH, json_out, NULL); | ||
532 | 101 | + r = json_variant_dump(package_metadata, JSON_FORMAT_FLUSH, json_out, NULL); | ||
533 | 102 | + if (r < 0) | ||
534 | 103 | + log_warning_errno(r, "Failed to write JSON package metadata, ignoring: %m"); | ||
535 | 104 | } | ||
536 | 105 | |||
537 | 106 | _exit(EXIT_SUCCESS); | ||
538 | 107 | @@ -805,7 +830,7 @@ int parse_elf_object(int fd, const char | ||
539 | 108 | |||
540 | 109 | r = json_parse_file(json_in, NULL, 0, &package_metadata, NULL, NULL); | ||
541 | 110 | if (r < 0 && r != -EINVAL) /* EINVAL: json was empty, so we got nothing, but that's ok */ | ||
542 | 111 | - return r; | ||
543 | 112 | + log_warning_errno(r, "Failed to read or parse json metadata, ignoring: %m"); | ||
544 | 113 | } | ||
545 | 114 | |||
546 | 115 | if (ret) | ||
547 | diff --git a/debian/patches/backport-for-CVE-2022-45873.patch b/debian/patches/backport-for-CVE-2022-45873.patch | |||
548 | 0 | new file mode 100644 | 116 | new file mode 100644 |
549 | index 0000000..857fd1d | |||
550 | --- /dev/null | |||
551 | +++ b/debian/patches/backport-for-CVE-2022-45873.patch | |||
552 | @@ -0,0 +1,45 @@ | |||
553 | 1 | From 7922ead507e0d83e4ec72a8cbd2b67194766e58c Mon Sep 17 00:00:00 2001 | ||
554 | 2 | From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl> | ||
555 | 3 | Date: Tue, 18 Oct 2022 18:09:06 +0200 | ||
556 | 4 | Subject: [PATCH] shared/json: allow json_variant_dump() to return an error | ||
557 | 5 | |||
558 | 6 | --- | ||
559 | 7 | src/shared/json.c | 7 ++++--- | ||
560 | 8 | src/shared/json.h | 2 +- | ||
561 | 9 | 2 files changed, 5 insertions(+), 4 deletions(-) | ||
562 | 10 | |||
563 | 11 | --- systemd-251.4.orig/src/shared/json.c | ||
564 | 12 | +++ systemd-251.4/src/shared/json.c | ||
565 | 13 | @@ -1770,9 +1770,9 @@ int json_variant_format(JsonVariant *v, | ||
566 | 14 | return (int) sz - 1; | ||
567 | 15 | } | ||
568 | 16 | |||
569 | 17 | -void json_variant_dump(JsonVariant *v, JsonFormatFlags flags, FILE *f, const char *prefix) { | ||
570 | 18 | +int json_variant_dump(JsonVariant *v, JsonFormatFlags flags, FILE *f, const char *prefix) { | ||
571 | 19 | if (!v) | ||
572 | 20 | - return; | ||
573 | 21 | + return 0; | ||
574 | 22 | |||
575 | 23 | if (!f) | ||
576 | 24 | f = stdout; | ||
577 | 25 | @@ -1798,7 +1798,8 @@ void json_variant_dump(JsonVariant *v, J | ||
578 | 26 | fputc('\n', f); /* In case of SSE add a second newline */ | ||
579 | 27 | |||
580 | 28 | if (flags & JSON_FORMAT_FLUSH) | ||
581 | 29 | - fflush(f); | ||
582 | 30 | + return fflush_and_check(f); | ||
583 | 31 | + return 0; | ||
584 | 32 | } | ||
585 | 33 | |||
586 | 34 | int json_variant_filter(JsonVariant **v, char **to_remove) { | ||
587 | 35 | --- systemd-251.4.orig/src/shared/json.h | ||
588 | 36 | +++ systemd-251.4/src/shared/json.h | ||
589 | 37 | @@ -195,7 +195,7 @@ typedef enum JsonFormatFlags { | ||
590 | 38 | } JsonFormatFlags; | ||
591 | 39 | |||
592 | 40 | int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret); | ||
593 | 41 | -void json_variant_dump(JsonVariant *v, JsonFormatFlags flags, FILE *f, const char *prefix); | ||
594 | 42 | +int json_variant_dump(JsonVariant *v, JsonFormatFlags flags, FILE *f, const char *prefix); | ||
595 | 43 | |||
596 | 44 | int json_variant_filter(JsonVariant **v, char **to_remove); | ||
597 | 45 | |||
598 | diff --git a/debian/patches/lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch b/debian/patches/lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch | |||
599 | 0 | new file mode 100644 | 46 | new file mode 100644 |
600 | index 0000000..382f6ea | |||
601 | --- /dev/null | |||
602 | +++ b/debian/patches/lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch | |||
603 | @@ -0,0 +1,66 @@ | |||
604 | 1 | From: Nick Rosbrook <nick.rosbrook@canonical.com> | ||
605 | 2 | Date: Tue, 22 Nov 2022 17:01:47 -0500 | ||
606 | 3 | Subject: sd-netlink: add a test for rtnl_set_link_name() | ||
607 | 4 | |||
608 | 5 | Origin: upstream, https://github.com/systemd/systemd/commit/b338a8bb40 | ||
609 | 6 | Bug-Ubuntu: https://launchpad.net/bugs/2002445 | ||
610 | 7 | |||
611 | 8 | Add a test that verifies a deleted alternative name is restored on error | ||
612 | 9 | in rtnl_set_link_name(). | ||
613 | 10 | --- | ||
614 | 11 | src/libsystemd/sd-netlink/test-netlink.c | 28 ++++++++++++++++++++++++++++ | ||
615 | 12 | 1 file changed, 28 insertions(+) | ||
616 | 13 | |||
617 | 14 | diff --git a/src/libsystemd/sd-netlink/test-netlink.c b/src/libsystemd/sd-netlink/test-netlink.c | ||
618 | 15 | index fbc3ef0..440e9ce 100644 | ||
619 | 16 | --- a/src/libsystemd/sd-netlink/test-netlink.c | ||
620 | 17 | +++ b/src/libsystemd/sd-netlink/test-netlink.c | ||
621 | 18 | @@ -8,6 +8,7 @@ | ||
622 | 19 | #include <linux/if_macsec.h> | ||
623 | 20 | #include <linux/l2tp.h> | ||
624 | 21 | #include <linux/nl80211.h> | ||
625 | 22 | +#include <unistd.h> | ||
626 | 23 | |||
627 | 24 | #include "sd-netlink.h" | ||
628 | 25 | |||
629 | 26 | @@ -666,6 +667,32 @@ static void test_genl(void) { | ||
630 | 27 | } | ||
631 | 28 | } | ||
632 | 29 | |||
633 | 30 | +static void test_rtnl_set_link_name(sd_netlink *rtnl, int ifindex) { | ||
634 | 31 | + _cleanup_strv_free_ char **alternative_names = NULL; | ||
635 | 32 | + int r; | ||
636 | 33 | + | ||
637 | 34 | + log_debug("/* %s */", __func__); | ||
638 | 35 | + | ||
639 | 36 | + if (geteuid() != 0) | ||
640 | 37 | + return (void) log_tests_skipped("not root"); | ||
641 | 38 | + | ||
642 | 39 | + /* Test that the new name (which is currently an alternative name) is | ||
643 | 40 | + * restored as an alternative name on error. Create an error by using | ||
644 | 41 | + * an invalid device name, namely one that exceeds IFNAMSIZ | ||
645 | 42 | + * (alternative names can exceed IFNAMSIZ, but not regular names). */ | ||
646 | 43 | + r = rtnl_set_link_alternative_names(&rtnl, ifindex, STRV_MAKE("testlongalternativename")); | ||
647 | 44 | + if (r == -EPERM) | ||
648 | 45 | + return (void) log_tests_skipped("missing required capabilities"); | ||
649 | 46 | + if (r == -EOPNOTSUPP) | ||
650 | 47 | + return (void) log_tests_skipped("alternative name is not supported"); | ||
651 | 48 | + | ||
652 | 49 | + assert_se(r >= 0); | ||
653 | 50 | + assert_se(rtnl_set_link_name(&rtnl, ifindex, "testlongalternativename") == -EINVAL); | ||
654 | 51 | + assert_se(rtnl_get_link_alternative_names(&rtnl, ifindex, &alternative_names) >= 0); | ||
655 | 52 | + assert_se(strv_contains(alternative_names, "testlongalternativename")); | ||
656 | 53 | + assert_se(rtnl_delete_link_alternative_names(&rtnl, ifindex, STRV_MAKE("testlongalternativename")) >= 0); | ||
657 | 54 | +} | ||
658 | 55 | + | ||
659 | 56 | int main(void) { | ||
660 | 57 | sd_netlink *rtnl; | ||
661 | 58 | sd_netlink_message *m; | ||
662 | 59 | @@ -697,6 +724,7 @@ int main(void) { | ||
663 | 60 | test_pipe(if_loopback); | ||
664 | 61 | test_event_loop(if_loopback); | ||
665 | 62 | test_link_configure(rtnl, if_loopback); | ||
666 | 63 | + test_rtnl_set_link_name(rtnl, if_loopback); | ||
667 | 64 | |||
668 | 65 | test_get_addresses(rtnl); | ||
669 | 66 | test_message_link_bridge(rtnl); | ||
670 | diff --git a/debian/patches/lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch b/debian/patches/lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch | |||
671 | 0 | new file mode 100644 | 67 | new file mode 100644 |
672 | index 0000000..2cf4997 | |||
673 | --- /dev/null | |||
674 | +++ b/debian/patches/lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch | |||
675 | @@ -0,0 +1,54 @@ | |||
676 | 1 | From: Nick Rosbrook <nick.rosbrook@canonical.com> | ||
677 | 2 | Date: Fri, 2 Dec 2022 15:26:18 -0500 | ||
678 | 3 | Subject: sd-netlink: do not swap old name and alternative name | ||
679 | 4 | |||
680 | 5 | Origin: upstream, https://github.com/systemd/systemd/commit/080afbb57c | ||
681 | 6 | Bug-Ubuntu: https://launchpad.net/bugs/2002445 | ||
682 | 7 | |||
683 | 8 | Commit 434a348380 ("netlink: do not fail when new interface name is | ||
684 | 9 | already used as an alternative name") added logic to set the old | ||
685 | 10 | interface name as an alternative name, but only when the new name is | ||
686 | 11 | currently an alternative name. This is not the desired outcome in most | ||
687 | 12 | cases, and the important part of this commit was to delete the new name | ||
688 | 13 | from the list of alternative names if necessary. | ||
689 | 14 | --- | ||
690 | 15 | src/libsystemd/sd-netlink/netlink-util.c | 12 ------------ | ||
691 | 16 | 1 file changed, 12 deletions(-) | ||
692 | 17 | |||
693 | 18 | diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c | ||
694 | 19 | index ce2c4f3..f88169e 100644 | ||
695 | 20 | --- a/src/libsystemd/sd-netlink/netlink-util.c | ||
696 | 21 | +++ b/src/libsystemd/sd-netlink/netlink-util.c | ||
697 | 22 | @@ -12,7 +12,6 @@ | ||
698 | 23 | int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { | ||
699 | 24 | _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL; | ||
700 | 25 | _cleanup_strv_free_ char **alternative_names = NULL; | ||
701 | 26 | - char old_name[IF_NAMESIZE] = {}; | ||
702 | 27 | int r; | ||
703 | 28 | |||
704 | 29 | assert(rtnl); | ||
705 | 30 | @@ -32,10 +31,6 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { | ||
706 | 31 | if (r < 0) | ||
707 | 32 | return log_debug_errno(r, "Failed to remove '%s' from alternative names on network interface %i: %m", | ||
708 | 33 | name, ifindex); | ||
709 | 34 | - | ||
710 | 35 | - r = format_ifname(ifindex, old_name); | ||
711 | 36 | - if (r < 0) | ||
712 | 37 | - return log_debug_errno(r, "Failed to get current name of network interface %i: %m", ifindex); | ||
713 | 38 | } | ||
714 | 39 | |||
715 | 40 | r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); | ||
716 | 41 | @@ -50,13 +45,6 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { | ||
717 | 42 | if (r < 0) | ||
718 | 43 | return r; | ||
719 | 44 | |||
720 | 45 | - if (!isempty(old_name)) { | ||
721 | 46 | - r = rtnl_set_link_alternative_names(rtnl, ifindex, STRV_MAKE(old_name)); | ||
722 | 47 | - if (r < 0) | ||
723 | 48 | - log_debug_errno(r, "Failed to set '%s' as an alternative name on network interface %i, ignoring: %m", | ||
724 | 49 | - old_name, ifindex); | ||
725 | 50 | - } | ||
726 | 51 | - | ||
727 | 52 | return 0; | ||
728 | 53 | } | ||
729 | 54 | |||
730 | diff --git a/debian/patches/lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch b/debian/patches/lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch | |||
731 | 0 | new file mode 100644 | 55 | new file mode 100644 |
732 | index 0000000..a5e2e24 | |||
733 | --- /dev/null | |||
734 | +++ b/debian/patches/lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch | |||
735 | @@ -0,0 +1,64 @@ | |||
736 | 1 | From: Nick Rosbrook <nick.rosbrook@canonical.com> | ||
737 | 2 | Date: Wed, 2 Nov 2022 05:36:14 -0400 | ||
738 | 3 | Subject: sd-netlink: restore altname on error in rtnl_set_link_name | ||
739 | 4 | |||
740 | 5 | Origin: upstream, https://github.com/systemd/systemd/commit/4d600667f8 | ||
741 | 6 | Bug-Ubuntu: https://launchpad.net/bugs/2002445 | ||
742 | 7 | |||
743 | 8 | If a current alternative name is to be used to rename a network | ||
744 | 9 | interface, the alternative name must be removed first. If interface | ||
745 | 10 | renaming fails, restore the alternative name that was deleted if | ||
746 | 11 | necessary. | ||
747 | 12 | --- | ||
748 | 13 | src/libsystemd/sd-netlink/netlink-util.c | 19 ++++++++++++++++--- | ||
749 | 14 | 1 file changed, 16 insertions(+), 3 deletions(-) | ||
750 | 15 | |||
751 | 16 | diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c | ||
752 | 17 | index 7f2596d..c4ead31 100644 | ||
753 | 18 | --- a/src/libsystemd/sd-netlink/netlink-util.c | ||
754 | 19 | +++ b/src/libsystemd/sd-netlink/netlink-util.c | ||
755 | 20 | @@ -11,6 +11,7 @@ | ||
756 | 21 | int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { | ||
757 | 22 | _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL; | ||
758 | 23 | _cleanup_strv_free_ char **alternative_names = NULL; | ||
759 | 24 | + bool altname_deleted = false; | ||
760 | 25 | int r; | ||
761 | 26 | |||
762 | 27 | assert(rtnl); | ||
763 | 28 | @@ -30,21 +31,33 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { | ||
764 | 29 | if (r < 0) | ||
765 | 30 | return log_debug_errno(r, "Failed to remove '%s' from alternative names on network interface %i: %m", | ||
766 | 31 | name, ifindex); | ||
767 | 32 | + | ||
768 | 33 | + altname_deleted = true; | ||
769 | 34 | } | ||
770 | 35 | |||
771 | 36 | r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); | ||
772 | 37 | if (r < 0) | ||
773 | 38 | - return r; | ||
774 | 39 | + goto fail; | ||
775 | 40 | |||
776 | 41 | r = sd_netlink_message_append_string(message, IFLA_IFNAME, name); | ||
777 | 42 | if (r < 0) | ||
778 | 43 | - return r; | ||
779 | 44 | + goto fail; | ||
780 | 45 | |||
781 | 46 | r = sd_netlink_call(*rtnl, message, 0, NULL); | ||
782 | 47 | if (r < 0) | ||
783 | 48 | - return r; | ||
784 | 49 | + goto fail; | ||
785 | 50 | |||
786 | 51 | return 0; | ||
787 | 52 | + | ||
788 | 53 | +fail: | ||
789 | 54 | + if (altname_deleted) { | ||
790 | 55 | + int q = rtnl_set_link_alternative_names(rtnl, ifindex, STRV_MAKE(name)); | ||
791 | 56 | + if (q < 0) | ||
792 | 57 | + log_debug_errno(q, "Failed to restore '%s' as an alternative name on network interface %i, ignoring: %m", | ||
793 | 58 | + name, ifindex); | ||
794 | 59 | + } | ||
795 | 60 | + | ||
796 | 61 | + return r; | ||
797 | 62 | } | ||
798 | 63 | |||
799 | 64 | int rtnl_set_link_properties( | ||
800 | diff --git a/debian/patches/lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch b/debian/patches/lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch | |||
801 | 0 | new file mode 100644 | 65 | new file mode 100644 |
802 | index 0000000..5ffc818 | |||
803 | --- /dev/null | |||
804 | +++ b/debian/patches/lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch | |||
805 | @@ -0,0 +1,61 @@ | |||
806 | 1 | From: Nick Rosbrook <nick.rosbrook@canonical.com> | ||
807 | 2 | Date: Fri, 2 Dec 2022 15:35:25 -0500 | ||
808 | 3 | Subject: udev: attempt device rename even if interface is up | ||
809 | 4 | |||
810 | 5 | Origin: upstream, https://github.com/systemd/systemd/commit/53584e7b61 | ||
811 | 6 | Bug-Ubuntu: https://launchpad.net/bugs/2002445 | ||
812 | 7 | |||
813 | 8 | Currently rename_netif() will not attempt to rename a device if it is | ||
814 | 9 | already up, because the kernel will return -EBUSY unless live renaming | ||
815 | 10 | is allowed on the device. This restriction will be removed in a future | ||
816 | 11 | kernel version [1]. | ||
817 | 12 | |||
818 | 13 | To cover both cases, always attempt to rename the interface and return 0 | ||
819 | 14 | if we get -EBUSY. | ||
820 | 15 | |||
821 | 16 | [1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=bd039b5ea2a9 | ||
822 | 17 | --- | ||
823 | 18 | src/udev/udev-event.c | 16 +++++----------- | ||
824 | 19 | 1 file changed, 5 insertions(+), 11 deletions(-) | ||
825 | 20 | |||
826 | 21 | diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c | ||
827 | 22 | index 9b53105..681aa87 100644 | ||
828 | 23 | --- a/src/udev/udev-event.c | ||
829 | 24 | +++ b/src/udev/udev-event.c | ||
830 | 25 | @@ -871,7 +871,6 @@ int udev_event_spawn( | ||
831 | 26 | static int rename_netif(UdevEvent *event) { | ||
832 | 27 | const char *oldname; | ||
833 | 28 | sd_device *dev; | ||
834 | 29 | - unsigned flags; | ||
835 | 30 | int ifindex, r; | ||
836 | 31 | |||
837 | 32 | assert(event); | ||
838 | 33 | @@ -905,16 +904,6 @@ static int rename_netif(UdevEvent *event) { | ||
839 | 34 | return 0; | ||
840 | 35 | } | ||
841 | 36 | |||
842 | 37 | - r = rtnl_get_link_info(&event->rtnl, ifindex, NULL, &flags, NULL, NULL, NULL); | ||
843 | 38 | - if (r < 0) | ||
844 | 39 | - return log_device_warning_errno(dev, r, "Failed to get link flags: %m"); | ||
845 | 40 | - | ||
846 | 41 | - if (FLAGS_SET(flags, IFF_UP)) { | ||
847 | 42 | - log_device_info(dev, "Network interface '%s' is already up, refusing to rename to '%s'.", | ||
848 | 43 | - oldname, event->name); | ||
849 | 44 | - return 0; | ||
850 | 45 | - } | ||
851 | 46 | - | ||
852 | 47 | /* Set ID_RENAMING boolean property here, and drop it in the corresponding move uevent later. */ | ||
853 | 48 | r = device_add_property(dev, "ID_RENAMING", "1"); | ||
854 | 49 | if (r < 0) | ||
855 | 50 | @@ -936,6 +925,11 @@ static int rename_netif(UdevEvent *event) { | ||
856 | 51 | return log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m"); | ||
857 | 52 | |||
858 | 53 | r = rtnl_set_link_name(&event->rtnl, ifindex, event->name); | ||
859 | 54 | + if (r == -EBUSY) { | ||
860 | 55 | + log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.", | ||
861 | 56 | + oldname, event->name); | ||
862 | 57 | + return 0; | ||
863 | 58 | + } | ||
864 | 59 | if (r < 0) | ||
865 | 60 | return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m", | ||
866 | 61 | ifindex, oldname, event->name); | ||
867 | diff --git a/debian/patches/lp2002445/udev-net-allow-new-link-name-as-an-altname-before-renamin.patch b/debian/patches/lp2002445/udev-net-allow-new-link-name-as-an-altname-before-renamin.patch | |||
868 | 0 | new file mode 100644 | 62 | new file mode 100644 |
869 | index 0000000..0d651a2 | |||
870 | --- /dev/null | |||
871 | +++ b/debian/patches/lp2002445/udev-net-allow-new-link-name-as-an-altname-before-renamin.patch | |||
872 | @@ -0,0 +1,34 @@ | |||
873 | 1 | From: Nick Rosbrook <nick.rosbrook@canonical.com> | ||
874 | 2 | Date: Wed, 2 Nov 2022 11:05:01 -0400 | ||
875 | 3 | Subject: udev/net: allow new link name as an altname before renaming happens | ||
876 | 4 | |||
877 | 5 | Origin: upstream, https://github.com/systemd/systemd/commit/d0b31efc1a | ||
878 | 6 | Bug-Ubuntu: https://launchpad.net/bugs/2002445 | ||
879 | 7 | |||
880 | 8 | When configuring a link's alternative names, the link's new name to-be | ||
881 | 9 | is not allowed to be included because interface renaming will fail if | ||
882 | 10 | the new name is already present as an alternative name. However, | ||
883 | 11 | rtnl_set_link_name will delete the conflicting alternative name before | ||
884 | 12 | renaming the device, if necessary. | ||
885 | 13 | |||
886 | 14 | Allow the new link name to be set as an alternative name before the | ||
887 | 15 | device is renamed. This means that if the rename is later skipped (i.e. | ||
888 | 16 | because the link is already up), then the name can at least still be | ||
889 | 17 | present as an alternative name. | ||
890 | 18 | --- | ||
891 | 19 | src/udev/net/link-config.c | 2 -- | ||
892 | 20 | 1 file changed, 2 deletions(-) | ||
893 | 21 | |||
894 | 22 | diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c | ||
895 | 23 | index 28c0e63..cdad078 100644 | ||
896 | 24 | --- a/src/udev/net/link-config.c | ||
897 | 25 | +++ b/src/udev/net/link-config.c | ||
898 | 26 | @@ -844,8 +844,6 @@ static int link_apply_alternative_names(Link *link, sd_netlink **rtnl) { | ||
899 | 27 | } | ||
900 | 28 | } | ||
901 | 29 | |||
902 | 30 | - if (link->new_name) | ||
903 | 31 | - strv_remove(altnames, link->new_name); | ||
904 | 32 | strv_remove(altnames, link->ifname); | ||
905 | 33 | |||
906 | 34 | r = rtnl_get_link_alternative_names(rtnl, link->ifindex, ¤t_altnames); | ||
907 | diff --git a/debian/patches/lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch b/debian/patches/lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch | |||
908 | 0 | new file mode 100644 | 35 | new file mode 100644 |
909 | index 0000000..6bc3b3c | |||
910 | --- /dev/null | |||
911 | +++ b/debian/patches/lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch | |||
912 | @@ -0,0 +1,54 @@ | |||
913 | 1 | From: Tuetuopay <tuetuopay@me.com> | ||
914 | 2 | Date: Fri, 27 Jan 2023 15:10:49 +0100 | ||
915 | 3 | Subject: network/dhcp4: accept local subnet routes from DHCP | ||
916 | 4 | |||
917 | 5 | Origin: upstream, https://github.com/systemd/systemd/commit/1d84a3c7792a8910b05904937c703307ca19740f | ||
918 | 6 | Bug-Ubuntu: https://launchpad.net/bugs/2004478 | ||
919 | 7 | |||
920 | 8 | RFC3442 specifies option 121 (Classless Static Routes) that allow a DHCP | ||
921 | 9 | server to push arbitrary routes to a client. It has a Local Subnet | ||
922 | 10 | Routes section expliciting the behavior of routes with a null (0.0.0.0) | ||
923 | 11 | gateway. | ||
924 | 12 | |||
925 | 13 | Such routes are to be installed on the interface with a Link scope, to | ||
926 | 14 | mark them as directly available on the link without any gateway. | ||
927 | 15 | |||
928 | 16 | Networkd currently drops those routes, which is against the RFC, as | ||
929 | 17 | Linux has proper support for such routes. | ||
930 | 18 | |||
931 | 19 | Fixes: 7f20627 ("network: dhcp4: ignore gateway in static routes if destination is link-local or in the same network") | ||
932 | 20 | --- | ||
933 | 21 | src/network/networkd-dhcp4.c | 19 +++++++++++-------- | ||
934 | 22 | 1 file changed, 11 insertions(+), 8 deletions(-) | ||
935 | 23 | |||
936 | 24 | diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c | ||
937 | 25 | index 0941ad0..7b1133e 100644 | ||
938 | 26 | --- a/src/network/networkd-dhcp4.c | ||
939 | 27 | +++ b/src/network/networkd-dhcp4.c | ||
940 | 28 | @@ -376,15 +376,18 @@ static int dhcp4_request_route_auto( | ||
941 | 29 | route->gw = IN_ADDR_NULL; | ||
942 | 30 | route->prefsrc.in = address; | ||
943 | 31 | |||
944 | 32 | - } else { | ||
945 | 33 | - if (in4_addr_is_null(gw)) { | ||
946 | 34 | - log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is not in the assigned network " | ||
947 | 35 | - IPV4_ADDRESS_FMT_STR"/%u, but no gateway is specified, ignoring.", | ||
948 | 36 | - IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, | ||
949 | 37 | - IPV4_ADDRESS_FMT_VAL(prefix), prefixlen); | ||
950 | 38 | - return 0; | ||
951 | 39 | - } | ||
952 | 40 | + } else if (in4_addr_is_null(gw)) { | ||
953 | 41 | + log_link_debug(link, "DHCP: requested route destination "IPV4_ADDRESS_FMT_STR"/%u is not in the assigned network " | ||
954 | 42 | + IPV4_ADDRESS_FMT_STR"/%u, but no gateway is specified, using 'link' scope.", | ||
955 | 43 | + IPV4_ADDRESS_FMT_VAL(route->dst.in), route->dst_prefixlen, | ||
956 | 44 | + IPV4_ADDRESS_FMT_VAL(prefix), prefixlen); | ||
957 | 45 | |||
958 | 46 | + route->scope = RT_SCOPE_LINK; | ||
959 | 47 | + route->gw_family = AF_UNSPEC; | ||
960 | 48 | + route->gw = IN_ADDR_NULL; | ||
961 | 49 | + route->prefsrc.in = address; | ||
962 | 50 | + | ||
963 | 51 | + } else { | ||
964 | 52 | r = dhcp4_request_route_to_gateway(link, gw); | ||
965 | 53 | if (r < 0) | ||
966 | 54 | return r; | ||
967 | diff --git a/debian/patches/series b/debian/patches/series | |||
968 | index 5b33e52..24359ad 100644 | |||
969 | --- a/debian/patches/series | |||
970 | +++ b/debian/patches/series | |||
971 | @@ -52,3 +52,12 @@ lp1981042-core-firstboot-workaround-timezone-issues-caused-by-Ubunt.patch | |||
972 | 52 | test-denylist-TEST-29-PORTABLE-again.patch | 52 | test-denylist-TEST-29-PORTABLE-again.patch |
973 | 53 | lp1989969-test-deny-list-TEST-36-NUMAPOLICY-on-ppc64el.patch | 53 | lp1989969-test-deny-list-TEST-36-NUMAPOLICY-on-ppc64el.patch |
974 | 54 | lp1991829-add-CAP_LINUX_IMMUTABLE-to-systemd-machined-so-it-ca.patch | 54 | lp1991829-add-CAP_LINUX_IMMUTABLE-to-systemd-machined-so-it-ca.patch |
975 | 55 | CVE-2022-4415.patch | ||
976 | 56 | backport-for-CVE-2022-45873.patch | ||
977 | 57 | CVE-2022-45873.patch | ||
978 | 58 | lp2004478-network-dhcp4-accept-local-subnet-routes-from-DHCP.patch | ||
979 | 59 | lp2002445/udev-net-allow-new-link-name-as-an-altname-before-renamin.patch | ||
980 | 60 | lp2002445/sd-netlink-do-not-swap-old-name-and-alternative-name.patch | ||
981 | 61 | lp2002445/sd-netlink-restore-altname-on-error-in-rtnl_set_link_name.patch | ||
982 | 62 | lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch | ||
983 | 63 | lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch |
Thanks for the explanation about the test failures. Apparently networkd-test.py regressed in release, which is bad but seems unrelated to the systemd package:
"There is an autopkgtest failure currently in kinetic (not considered a regression) that seems to be caused by a kernel change. I am not exactly sure what change broke the test, but if I run the test in a fresh kinetic install (nothing from -updates installed yet), the test passes. But if I install just the new kernel from -updates, the test fails." -- enr0n
Do we already have a tracking bug report about that ^ issue? Could you please reference it here and escalate it to the corresponding (Kernel?) team? Having systemd autopkgtests failing in a stable series is not a good position to be in and we should try to get it fixed soon (by the corresponding team).
Your changes/patches to systemd packaging itself LGTM!