This test often fails because the VM reboots (eg: kernel panic) and the
previous state is left unclean. Do the cleanups at the beginning too,
to try and reduce flakiness.
After looking at the unit, I'm not sure if systemd-imds-import.service
is supposed to run in the host system or not. But if it is supposed to
only run in the initrd, then the static condition in the unit file gives
as the worst behaviour: the generator does not do any checks if we are
in the initrd or not, and if it enabled the unit, it'll influence the
transaction ordering (possibly causing loops or additional work) and
then the unit will be unconditionally skipped. So replace the static
condition by a check in the generator. If the user specifies
systemd.imds.import on the commandline, it'll be honoured also in the
host.
The imds services are (very-) early boot services, ordered before
sysinit.target. OTOH, network-online.target is something that is often
established very late [1]. In fact, we always recommended to _not_
depend on it for things that are ordered before the usual targets that
allow user logins (e.g. graphical.target). So we certainly cannot add
early-boot ordering for it. Currently systemd-imds-imports.service
is conditionalized to only run in the initrd. But if the unit is enabled
in the host system, it would affect the transaction there, so we should
drop this bogus ordering everywhere.
(I think this might have been added by mistake. After all,
systemd-imds-import.service itself shouldn't care about the network.)
This fixes boot ordering issues reported upstream and in Fedora [2, 3].
[1] https://systemd.io/NETWORK_ONLINE/#network-connectivity-has-been-established-network-onlinetarget
[2] https://github.com/systemd/systemd/pull/40980#discussion_r2949473028
[3] https://bugzilla.redhat.com/show_bug.cgi?id=2481304
This reverts commit a17efef137 ("test: try to detect SIGILL in stress-ng
and skip TEST-55-OOMD gracefully") and replaces it with a single
check to skip the test cases.
The previous check was not reliable as stress-ng can catch SIGILL
itself and exist with an error:
stress-ng[1068]: stress-ng: debug: [1068] caught SIGILL, address \
0x00005632f8330140 (ILL_ILLOPN)
stress-ng[1068]: stress-ng: debug: [1068] stress-ng: info: \
0x00005632f8330140:<62>71 fd 48 6f 2d 36 14 1c 00 c5 d1 ef ed 49 29
...
stress-ng[1053]: stress-ng: error: [1053] vm: [1061] terminated \
with an error, exit status=2 (stressor failed)
...
systemd[1]: TEST-55-OOMD-slowrule.service: Main process exited, \
code=exited, status=2/INVALIDARGUMENT
systemd[1]: TEST-55-OOMD-slowrule.service: Failed with result \
'exit-code'.
Try to detect at the beginning of the test and skip the test case if it
happens.
di is allocated while iterating over p.partitions. If the array is
empty,
the loop is skipped and di remains NULL, but the code still assigns
image
metadata through it.
Return EBADMSG before accessing di in this case.
Found by Linux Verification Center (linuxtesting.org) with Svace.
When adding new groups, putgrent_with_members() is called with the newly
constructed group n, but the error path logs gr->gr_name. gr belongs to
the
earlier loop over the existing group file and may be NULL after EOF.
Log n.gr_name instead.
Found by Linux Verification Center (linuxtesting.org) with Svace.
SUSE does not provide a vmlinux.h so the package is not built with
CO-RE support, hence the test fails. This was previously masked
by the fact that python3-packaging was never installed, so the
test always skipped everywhere as it could not detect the kernel
version.
Follow-up for c310106c15
Each <type>_id is picked from a Unit.List '{}' call and immediately
queried via a second Unit.List '{"name":"<id>"}' call. When the picked
unit is tied to a user session (run-user-N.mount, user@N.service,
user-runtime-dir@N.service, session-cN.scope) it may be torn down
between the two calls if the user manager happens to exit at that
moment, causing a spurious NoSuchUnit failure.
Filter those out of the candidate lists for mount/service/scope.
Excerpt from a failed CI run showing the race:
[ 2631.395826] systemd[1]: user-runtime-dir@4711.service: Trying to enqueue job user-runtime-dir@4711.service/stop/fail
[ 2632.101414] systemd[1]: user-runtime-dir@4711.service: Got final SIGCHLD for state stop.
[ 2632.182537] TEST-74-AUX-UTILS.sh[3051]: + mount_id=run-user-4711.mount
[ 2632.186760] TEST-74-AUX-UTILS.sh[3099]: + varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List '{"name":"run-user-4711.mount"}'
[ 2632.189752] systemd[1]: varlink-api-3-3: Sending message: {"error":"io.systemd.Unit.NoSuchUnit"}
[ 2632.190752] TEST-74-AUX-UTILS.sh[3099]: Method call io.systemd.Unit.List() failed: io.systemd.Unit.NoSuchUnit
[ 2632.191594] TEST-74-AUX-UTILS.sh[101]: Subtest /usr/lib/systemd/tests/testdata/units/TEST-74-AUX-UTILS.varlinkctl-unit.sh failed
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
The attempt to fix SIGILL in TEST-55-OOMD actually made the test more
flaky, as the chosen stress-ng method generates less pressure, so it
often fails.
Try a different approach: try to detect that SIGILL caused the unit
calling stress-ng to fail, and skip gracefully in that case.
The numbers of signals vary by arch. On the common arches the signals
listed here all use the same numbers, but people are likely to use
this on more fringe architectures too, so let's use symbolic names
instead.
Also the comment about gdb "hitting the same kill" didn't make sense.
The syntax is a bit baroque, but using a helper variable does not work.
Also shellcheck complains about $[ ] which would have made this more
legible.
The attempt to fix SIGILL in TEST-55-OOMD actually made the test more
flaky, as the chosen stress-ng method generates less pressure, so it
often fails.
Try a different approach: try to detect that SIGILL caused the unit
calling stress-ng to fail, and skip gracefully in that case.
This allows multi-call binaries to be easily invoked with a different
name. After installation, the name is set by creating a symlink. But in
build directories, we don't create the symlinks. (There are also other
ways to achieve the same thing, e.g. zsh supports $ARGV0, and exec -a
can be used, but those are either non-portable or are more complicated
to use.) The primary use-case for me is to test --help output for
multicall binaries.
Also reorder the help for env vars to group the more generic ones near
the top.
This was initially proposed in https://github.com/systemd/systemd/pull/24054,
but there were some comments about the implementation. I had a branch
with the patch, but I don't think I ever actually submitted it as a
pull request.
When building with Meson >=1.3.0, use the native `fs.relative_to()`
helper instead of shelling out to `realpath`.
This is more portable: BusyBox `realpath` does not support the
`--relative-to` option.
nspawn started logging at notice level when exiting a container:
root@sid-arm64:~# exit
logout
Container sid-arm64 exited successfully.
Failed to unregister machine in system context, ignoring: No such device or address
Looks like it's racing with machined which already noticed it was
gone and remove it. Downgrade to debug on ENXIO.
Follow-up for 1e084aad7d
On some architectures like m68k, alignof(void*) is 2, not sizeof(void*)
(which is 4). So the natural alignment of struct Option is 2 and
sizeof(Option) == 26.
However, each variable placed in the SYSTEMD_OPTIONS ELF section via
_OPTION() carries _alignptr_ (= __attribute__((aligned(sizeof(void*))))),
which forces each entry to start at a 4-byte boundary. The linker
therefore inserts 2 bytes of padding between adjacent entries, producing
an actual stride of 28 in the section.
option_parse() iterates over the section with pointer arithmetic
("opt++"), which advances by sizeof(Option) == 26 and lands inside the
padding. The fields read back as zero, and since commit cf88903637
("tree-wide: get rid of most uses of ALIGN_PTR") added the ordering
assertion below, the resulting "0 < 0" trips it when running --help:
Assertion 'opt->id < (opt + 1)->id' failed at src/shared/options.c:116, function option_parse(). Aborting.
#0 0xc0a7b248 in ?? () from /usr/lib/m68k-linux-gnu/libc.so.6
#1 0xc0a7b2ce in pthread_kill () from /usr/lib/m68k-linux-gnu/libc.so.6
#2 0xc0a2edc6 in raise () from /usr/lib/m68k-linux-gnu/libc.so.6
#3 0xc0a1c128 in abort () from /usr/lib/m68k-linux-gnu/libc.so.6
#4 0xc05c6f78 in option_parse (options=0x0, options_end=0x0, state=0xc09ca968) at ../src/shared/options.c:116
Fix this by applying _alignptr_ to the struct definition itself, so that
sizeof(Option) is padded up to a multiple of sizeof(void*) and matches
the actual on-disk stride. Add an assert_cc() so any future regression
is caught at compile time.
The same latent bug applies to Verb and TestFunc, which use the same
section-placement pattern. Their natural sizeof was already a multiple
of sizeof(void*) so no crash was observed, but apply the same fix
defensively.
Follow-up for cf88903637
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Not the case on the sparc architecture. Replace with two translation
functions. The poll to epoll variant isn't used yet but will be used
when we add io-uring support to sd-event so we already add it here so
that it is in place.
Follow-up for cf4c65afa8
This test can regress without changes on our end when
an older systemd is built against a newer glibc and
the newer glibc has an ABI break for a symbol we use
but don't dlsym(). To avoid spurious test failures for
downstream packagers, let's only run the test in developer
mode for now.
On architectures where glibc support was introduced after our baseline
(e.g. loongarch64, added in glibc 2.36), every imported symbol is
tagged with a version newer than the baseline, so the baseline check
fails spuriously. Detect this by taking the minimum version observed
across all binaries: if it exceeds the baseline, treat that minimum
as the effective baseline instead.
Follow up for d9600a2a
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Commit f795d54591 ("libc,shared: detect newer library symbols at
runtime") replaced the build-time HAVE_EPOLL_PWAIT2 gate with a runtime
shim that resolves the libc symbol via dlsym(RTLD_DEFAULT, "epoll_pwait2").
That breaks on 32-bit architectures built with _TIME_BITS=64 (e.g. Debian
armhf since the time64 transition).
When _TIME_BITS=64 is in effect on a 32-bit target, glibc's <sys/epoll.h>
asm-renames the C identifier `epoll_pwait2` to the matching 64-bit-time_t
ABI symbol `__epoll_pwait2_time64`, so a normal C call resolves to the
variant whose `struct timespec` layout (16 bytes: s64 tv_sec, long tv_nsec,
long pad) matches what the compiler is using:
/* /usr/include/sys/epoll.h */
#ifndef __USE_TIME64_REDIRECTS
extern int epoll_pwait2 (int __epfd, struct epoll_event *__events,
int __maxevents,
const struct timespec *__timeout,
const __sigset_t *__ss) ...;
#else
# ifdef __REDIRECT
extern int __REDIRECT (epoll_pwait2, (int __epfd, ...,
const struct timespec *__timeout,
const __sigset_t *__ss),
__epoll_pwait2_time64) ...;
# else
# define epoll_pwait2 __epoll_pwait2_time64
# endif
#endif
dlsym() doesn't see that header-level rename and returns the legacy
32-bit-time_t `epoll_pwait2` symbol, which interprets only the first 8
bytes of the 16-byte timespec we hand it. With a little-endian layout
that means the legacy callee reads tv_sec = (low 32 bits of real tv_sec)
and tv_nsec = (high 32 bits of real tv_sec, normally 0). Sub-second
timeouts therefore become {0,0} and return immediately, while multi-
second timeouts get truncated to whole seconds.
That matches the test failure reported on armv7:
/* test_simple_timeout */
src/libsystemd/sd-event/test-event.c:724: Assertion failed:
Expected "t >= usec_add(f, some_time)",
but 571153514285 < 571154403965
(the 889680 us shortfall is consistent with a sub-second `some_time` that
got rounded down to zero by the truncated wait).
Fix it by adding a DEFINE_LIBC_ERRNO_SHIM_NAMED() variant of the macro
that takes an explicit dlsym symbol name string. Both macros keep their
own copy of the body so every `func` reference stays behind `#` or `##`,
otherwise the override-header `#define epoll_pwait2 epoll_pwait2_shim`
would rewrite the token before the inner macro could paste it (yielding
`epoll_pwait2_shim_shim`). src/libc/epoll.c then picks
"__epoll_pwait2_time64" under __USE_TIME_BITS64 so the cached function
pointer matches the timespec ABI the rest of the code is built with.
Follow-up for f795d54591
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Four *_attach_event helpers swallow sd_event_default() failures and
return success while leaving obj->event NULL:
sd_dhcp_client_attach_event (sd-dhcp-client.c)
sd_dhcp6_client_attach_event (sd-dhcp6-client.c)
sd_ndisc_attach_event (sd-ndisc.c)
sd_radv_attach_event (sd-radv.c)
Each contains the same copy-pasted typo in the NULL-event branch:
r = sd_event_default(&obj->event);
if (r < 0)
return 0; /* swallows -ENOMEM / -ECHILD */
The caller is told the attach succeeded, but obj->event is still
NULL. A subsequent *_start() then trips assert_return(obj->event,
-EINVAL) and returns a spurious -EINVAL (or aborts on assert-abort
builds), and any consumer of *_get_event() dereferences NULL.
The sibling helper sd_dhcp_server_attach_event already uses the
correct pattern:
r = sd_event_default(&server->event);
if (r < 0)
return r;
Fix by returning r instead of 0 at each of the four sites so the
sd_event_default() errno is propagated to the caller and ownership
stays with the caller.
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
apply_syscall_filter() unconditionally inserts the write() syscall
into c->syscall_filter when exec_fd or handoff_timestamp_fd is in
use, so the parent can receive the exec status / handoff timestamp
from the child. When the unit configured a positive
SystemCallFilter= allow-list that deliberately omits write(), the
resulting widening of the operator's policy happens silently with
no trace in the journal.
Emit a log_debug() before the seccomp_filter_set_add_by_name() call
when syscall_allow_list is true, so the widening is at least
observable to operators inspecting the unit's debug log.
While here, document that mutating c->syscall_filter through a
'const ExecContext *c' is intentional: apply_syscall_filter() runs
only in the post-fork child, which owns a private copy of the
address space, so the hashmap change is never observed by the
manager.
No functional change for the allow-list itself; write() is still
added exactly as before.
Fixes: 84b79215cc ("core: do not filter out write() if required in the very late stage")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
apply_root_directory() calls chroot() but never pairs it with a
chdir("/"). Moving cwd into the new root is left to the subsequent
apply_working_directory() call. That delegation breaks when the unit
sets WorkingDirectory=-/path (working_directory_missing_ok):
apply_root_directory() apply_working_directory()
chroot(new_root) --> chdir(wd) fails (ENOENT)
return 0 return 0 /* missing_ok */
chroot(2) changes only the process root, not cwd. With cwd still
resolving to the pre-chroot host dentry, the process runs with the
kernel root pointing at the new root while relative-path accesses
and /proc/self/cwd reach paths outside RootDirectory=.
Only the chroot-only branch (exec_needs_mount_namespace() == false)
is affected. The mount-namespace branch uses pivot_root(), which
restructures the mount tree and makes the pre-chroot dentry
unreachable.
Fix by issuing chdir("/") immediately after the successful chroot()
in apply_root_directory() and propagating any failure with
EXIT_CHROOT, matching the standard chroot+chdir idiom and making
the chroot self-contained regardless of how
apply_working_directory() later handles a missing directory.
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
sd_dhcp6_client_add_option() and sd_dhcp6_client_add_vendor_option()
store the caller's sd_dhcp6_option in an ordered container whose value
destructor is sd_dhcp6_option_unref, registered via
DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR in dhcp6_option_hash_ops. The
container therefore owns exactly one ref per stored slot.
Both helpers guarded the ref bump only on r < 0:
r = ordered_{hashmap,set}_ensure_put(..., v);
if (r < 0)
return r;
sd_dhcp6_option_ref(v);
ordered_hashmap_ensure_put() (and the ordered_set wrapper) forward the
underlying hashmap_put_boldly tri-state verbatim: 1 on fresh insert, 0
when the identical key+value pair is already stored (no new slot
allocated), <0 on error. The r == 0 path fell through and bumped the
refcount without a corresponding slot.
The extra ref is permanently stranded once the container's destructor
runs. In a long-running networkd that reapplies .network files on
reload this leaks one sd_dhcp6_option allocation per duplicate-add.
Fix by tightening the guard to r <= 0 in both helpers, so
sd_dhcp6_option_ref(v) is reached only on a genuine new slot (r == 1).
Propagate the tri-valued result by returning r from both helpers
instead of a hard-coded constant.
sd_dhcp6_client_add_vendor_option() is a public libsystemd sd_ API;
its success return changes from a constant 1 to {0, 1}.
sd_dhcp6_client_add_option() now also propagates the tri-valued
result (previously a hard-coded 0 on success), giving the two parallel
helpers identical return contracts.
The sole non-fuzz in-tree callers are both in dhcp6_configure()
(src/network/networkd-dhcp6.c), which treat any r >= 0 as success and
do not distinguish 0 from 1, so the contract widening is caller-safe.
Fixes: e7d5fe17db ("DHCP client: make SendOption work for DHCPv6 too.")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Zbigniew Jędrzejewski-Szmek
Luca Boccassi
Yu Watanabe
Mikhail Nogin
Daan De Meyer
naly zzwd
Ali Ciloglu
Fco. Javier F. Serrador
Lennart Poettering
vad
Chris Mason