In ccecae0efd ("vmspawn: use machine name in runtime directory path")
support for RUNTIME_DIRECTORY was dropped which makes it difficult to
run systemd-vmspawn in a service unit which doesn't have write access to
the regular /run but should use its own managed RUNTIME_DIRECTORY. What
worked before was --keep-unit --system but we can't use XDG_RUNTIME_DIR
and --user because then --keep-unit breaks which we need because it
can't create a scope as there is no session. Switch back to
runtime_directory which handles RUNTIME_DIRECTORY and tells us whether
we should use it as is without later cleanup or if we need to use the
regular path where we create and delete the directory ourselves.
- Use persist-credentials: false for actions/checkout, so we don't
leak the github token credentials to subsequent jobs.
- Remove one / from the Edit/Write permissions. Currently, with the
absolute path from github.workspace, we expand to three slashes while
we only need two.
In ccecae0efd ("vmspawn: use machine name in runtime directory path")
support for RUNTIME_DIRECTORY was dropped which makes it difficult to
run systemd-vmspawn in a service unit which doesn't have write access
to the regular /run but should use its own managed RUNTIME_DIRECTORY.
What worked before was --keep-unit --system but we can't use
XDG_RUNTIME_DIR and --user because then --keep-unit breaks which
we need because it can't create a scope as there is no session.
Switch back to runtime_directory which handles RUNTIME_DIRECTORY and
tells us whether we should use it as is without later cleanup or if we
need to use the regular path where we create and delete the directory
ourselves.
I promised in https://github.com/systemd/systemd/pull/41426 that its the
final update for coccinelle pointer deref checks. However it turned out
there is this coccinelle/parsing_hacks.h that I wasn't aware of. The
file missed some important things like _cleanup_(x) that prevented
coccinelle to check a bunch of functions.
This PR adds some missing defines to the parsing_hacks.h and fixes the
missing asserts(). I apologize that its a bit long (and frankly boring)
and that I missed this earlier.
The last commit contains one small behavior change (ret in
sd_varlink_idl_parse() is now really optional) but the big one is very
mechanical.
This is useful when moving from `--pty` or `--pipe` to using
`--verbose`: you can use `--verbose-output=cat` to get similar output on
stdout while still having all of the advantages of `--verbose` over the
other options.
Check S_ISDIR() before S_ISLINK() for all stat_verify_xyz() helpers
first, where we check them. Just to ensure we systematically return the
same errors.
The loop used the byte offset end (struct_off + struct_size) as the
iteration limit, but cursor[i] indexes uint32_t words. This reads
past the struct block when end > size_words.
Use size_words (struct_size / sizeof(uint32_t)) which is the correct
number of words to iterate over.
Also fix a pre-existing OOB in the FDT_BEGIN_NODE handler: the guard
i >= size_words is always false inside the loop (since the loop
condition already ensures i < size_words), so cursor[++i] at the
boundary reads one word past the struct block. Use i + 1 >= size_words
to check before incrementing.
Fixes: https://github.com/systemd/systemd/issues/41590
Bound image dimensions before computing row_size to prevent overflow
in the depth * x multiplication on 32-bit. Without this, crafted
dimensions like depth=32 x=0x10000001 wrap to a small row_size that
passes all subsequent checks.
Reject channel masks where all bits are set (popcount == 32), since
1U << 32 is undefined behavior and causes division by zero on
architectures where it evaluates to zero. Move the validation before
computing derived values for clarity. Use unsigned 1U in shifts to
avoid signed integer overflow UB for popcount == 31.
Also reject zero-width and zero-height images.
Fixes: https://github.com/systemd/systemd/issues/41589
We already have checks in place during compression that limit the data
we compress, so they shouldn't decompress to anything larger than
DATA_SIZE_MAX unless they've been tampered with. Let's make this
explicit and limit all our decompress_blob() calls in journal-handling
code to that limit.
One possible scenario this should prevent is when one tries to open and
verify a journal file that contains a compression bomb in its payload:
```
$ ls -lh test.journal
-rw-rw-r--+ 1 fsumsal fsumsal 1.2M Apr 12 15:07 test.journal
$ systemd-run --user --wait --pipe -- build-local/journalctl --verify --file=$PWD/test.journal
Running as unit: run-p682422-i4875779.service
000110: Invalid hash (00000000 vs. 11e4948d73bdafdd)
000110: Invalid object contents: Bad message
File corruption detected at /home/fsumsal/repos/@systemd/systemd/test.journal:272 (of 1249896 bytes, 0%).
FAIL: /home/fsumsal/repos/@systemd/systemd/test.journal (Bad message)
Finished with result: exit-code
Main processes terminated with: code=exited, status=1/FAILURE
Service runtime: 48.051s
CPU time consumed: 47.941s
Memory peak: 8G (swap: 0B)
```
Same could be, in theory, possible with just `journalctl --file=`, but
the reproducer would be a bit more complicated (haven't tried it, yet).
Lastly, the change in journal-remote is mostly hardening, as the maximum
input size to decompress_blob() there is mandated by MHD's connection
memory limit (set to JOURNAL_SERVER_MEMORY_MAX which is 128 KiB at the
time of writing), so the possible output size there is already quite
limited (e.g. ~800 - 900 MiB for xz-compressed data).
Add a new --restrict-address-families= command line option and
corresponding RestrictAddressFamilies= setting for .nspawn files to
restrict which socket address families may be used inside a container.
Many address families such as AF_VSOCK and AF_NETLINK are not
network-namespaced, so restricting access to them in containers
improves isolation. The option supports allowlist and denylist modes
(via ~ prefix), as well as "none" to block all families, matching the
semantics of RestrictAddressFamilies= in unit files.
The address family parsing logic is extracted into a shared
parse_address_families() helper in parse-helpers.c, which is now also
used by config_parse_address_families() in load-fragment.c.
This is currently opt-in. In a future version, the default will be
changed to restrict address families to AF_INET, AF_INET6 and AF_UNIX.
Replace the fork+exec of /usr/bin/kexec in load_kexec_kernel() with a
direct kexec_file_load() syscall, removing the runtime dependency on
kexec-tools for systemctl kexec.
The kexec_file_load() syscall (available since Linux 3.17) accepts
kernel and initrd file descriptors directly, letting the kernel handle
image parsing, segment setup, and purgatory internally. This is much
simpler than the older kexec_load() syscall which requires complex
userspace setup of memory segments and boot protocol structures — that
complexity is the raison d'être of kexec-tools.
The implementation follows the established libc wrapper pattern: a
missing_kexec_file_load() fallback in src/libc/kexec.c calls the
syscall directly when glibc doesn't provide a wrapper (which is
currently always the case). The syscall is not available on all
architectures — alpha, i386, ia64, m68k, mips, sh, and sparc lack
__NR_kexec_file_load — so the wrapper and caller are guarded with
HAVE_KEXEC_FILE_LOAD_SYSCALL to compile cleanly everywhere.
When kexec_file_load() rejects the kernel image with ENOEXEC (e.g. the
image is compressed or wrapped in a PE container that the kernel's kexec
handler doesn't understand natively), we attempt to unwrap/decompress
and retry. This is effectively the same decompression and extraction
logic that already lives in src/ukify/ukify.py (maybe_decompress() and
get_zboot_kernel()), now implemented in C so that systemctl can handle
it natively without shelling out to external tools:
- Compressed kernels (Image.gz, Image.zst, Image.xz, Image.lz4): the
format is detected by magic bytes (per RFC 1952, RFC 8878,
tukaani.org xz spec, and lz4 frame format spec) and decompressed to
a memfd using the existing decompress_stream_*() infrastructure plus
the new gzip support from the previous commit. This is primarily
needed on arm64 where kexec_file_load() only accepts raw Image files.
On x86_64, bzImage is already the native format and works directly.
- EFI ZBOOT PE images (vmlinuz.efi): detected by "MZ" + "zimg" magic
at the start of the file. The compressed payload offset, size, and
compression type are read from the ZBOOT header defined in
linux/drivers/firmware/efi/libstub/zboot-header.S.
- Unified Kernel Images (UKI): detected as PE files with a .linux
section via the existing pe_is_uki() infrastructure. The .linux
section (kernel) and optionally .initrd section are extracted to
memfds. When a UKI provides an embedded initrd and the boot entry
doesn't specify one separately, the embedded initrd is used.
The try-first-then-decompress approach means we never decompress
unnecessarily: on x86_64 the first kexec_file_load() call succeeds
immediately with the raw bzImage, and on architectures where the
kernel's kexec handler natively understands PE (like LoongArch with
kexec_efi_ops), ZBOOT/UKI images work without decompression too.
If kexec_file_load() is unavailable (architectures without the syscall)
or all attempts fail, we fall back to forking+execing the kexec binary.
This preserves compatibility on architectures like i386 and mips where
only the older kexec_load() syscall exists and kexec-tools is needed to
handle the complex userspace setup.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Replace the duplicated magic byte signatures in decompressor_detect()
with a call to the new compression_detect_from_magic() helper and use a
switch statement to initialize the appropriate decompression context.
We generally assume that valid times returned by clock_gettime() are > 0
and < USEC_INFINITY. If this wouldn't hold all kinds of things would
break, because we couldn't distuingish our niche values from regular
values anymore.
Let's hence encode our assumptions in C, already to help static
analyzers and LLMs.
Inspired by: https://github.com/systemd/systemd/pull/41601#pullrequestreview-4094645891
nss_count_strv() counts trailing NULL pointers in n. The pointer area
then used (n + 1), reserving one slot more than the size check
accounted for.
Drop the + 1 since n already includes the trailing NULLs, unlike the
non-shadow nss_pack_group_record() where n does not.
Fixes: https://github.com/systemd/systemd/issues/41591
One more round, this time with the help of the claudebot, especially for
spelunking in git blame to find the original commit and writing commit
messages from the list of warnings exported from coverity
Co-developed-by: Claude
[claude@anthropic.com](mailto:claude@anthropic.com)
Convert string fields to varlink enums in io.systemd.Unit
Following
https://github.com/systemd/systemd/pull/39391#discussion_r2489599449,
convert all configuration setting fields in the io.systemd.Unit varlink
interface from bare SD_VARLINK_STRING to proper enum types, adding type
safety to the IDL.
This converts ~30 fields across ExecContext, CGroupContext, and
UnitContext, adding 25 new varlink enum types.
Weak compatibility breakage (per
https://github.com/systemd/systemd/pull/40972#issuecomment-4222294318):
Varlink enum identifiers cannot contain - or +, so affected values are
underscorified on the wire. For example, "tty-force" becomes tty_force,
"kmsg+console" becomes kmsg_console.
The full list of affected values:
```
- ExecInputType: tty-force, tty-fail
- ExecOutputType: kmsg+console, journal+console
- ProtectHome: read-only
- CGroupController: bpf-firewall, bpf-devices, bpf-foreign, bpf-socket-bind, bpf-restrict-network-interfaces, bpf-bind-network-interface
- CollectMode: inactive-or-failed
- EmergencyAction: exit-force, reboot-force, reboot-immediate, poweroff-force, poweroff-immediate, soft-reboot, soft-reboot-force, kexec-force, halt-force, halt-immediate
- JobMode: replace-irreversibly, ignore-dependencies, ignore-requirements, restart-dependencies
```
We already have checks in place during compression that limit the data
we compress, so they shouldn't decompress to anything larger than
DATA_SIZE_MAX unless they've been tampered with. Let's make this
explicit and limit all our decompress_blob() calls in journal-handling
code to that limit.
One possible scenario this should prevent is when one tries to open and
verify a journal file that contains a compression bomb in its payload:
$ ls -lh test.journal
-rw-rw-r--+ 1 fsumsal fsumsal 1.2M Apr 12 15:07 test.journal
$ systemd-run --user --wait --pipe -- build-local/journalctl --verify --file=$PWD/test.journal
Running as unit: run-p682422-i4875779.service
000110: Invalid hash (00000000 vs. 11e4948d73bdafdd)
000110: Invalid object contents: Bad message
File corruption detected at /home/fsumsal/repos/@systemd/systemd/test.journal:272 (of 1249896 bytes, 0%).
FAIL: /home/fsumsal/repos/@systemd/systemd/test.journal (Bad message)
Finished with result: exit-code
Main processes terminated with: code=exited, status=1/FAILURE
Service runtime: 48.051s
CPU time consumed: 47.941s
Memory peak: 8G (swap: 0B)
Same could be, in theory, possible with just `journalctl --file=`, but
the reproducer would be a bit more complicated (haven't tried it, yet).
Lastly, the change in journal-remote is mostly hardening, as the maximum
input size to decompress_blob() there is mandated by MHD's connection
memory limit (set to JOURNAL_SERVER_MEMORY_MAX which is 128 KiB at the
time of writing), so the possible output size there is already quite
limited (e.g. ~800 - 900 MiB for xz-compressed data).
We have code like `size_t max_size = sizeof(*private)` in three
places. This is evaluated at compile time so its safe to use. However
the new pointer-deref checker in coccinelle is not smart enough to know
this and will flag those as errors. To avoid these false positives
we have some options:
1. Reorder so that we do:
```C
size_t max_size = 0;
assert(private);
max_size = sizeof(*private);
```
2. Use something like `size_t max_size = sizeof(*ASSERT_PTR(private));`
3. Place the assert before the declaration
4. Workaround coccinelle via SIZEOF(*private) that we can then hide
via parsing_hacks.h
5. Fix coccinelle (OCaml, hard)
6. ... somehting I missed?
None of these is very appealing. I went for (4) but happy about
suggestions.
We have a test failure where the testsuite is calling
sd_varlink_idl_parse() with *ret being NULL. This is now an
assert error. So we could either fix the test or fix the code
Given that it seems genuinely useful to run sd_varlink_idl_parse()
without *ret to e.g. just check if the idl is valid I opted to
fix the code.
The updated parsing_hacks.h file uncovered a bunch of extra
things that the check-pointer-deref coccinelle script flags.
This commit fixes them to make the tree check-pointer-deref clean.
The existing iszero_safe guards at index 9 and 10 were added to
silence Coverity, but the same division-by-float-zero warning also
applies to the divisions at index 0 (DBL_MIN) and 1 (DBL_MAX).
CID#1587762
Follow-up for 7f133c996c
The BreakpointType enum includes _BREAKPOINT_TYPE_INVALID (-EINVAL),
so Coverity flags the bit shift as potentially using a negative shift
amount. Add an assert to verify the type is in valid range, since the
static table only contains valid entries.
CID#1568482
Follow-up for 1929226e7e
Use overflow-safe INC_SAFE() instead of raw addition for idx
accumulation, so that Coverity can see the addition is checked.
CID#1548028
Follow-up for a05483a921
Coverity flags range->n_entries - j as a potential underflow
in the memmove size calculation. Add assert(range->n_entries > 0)
before decrementing n_entries, which holds since the loop condition
guarantees j < n_entries.
CID#1548015
Follow-up for 8dcc66cefc
1024 connections per UID is unnecessarily generous, so let's scale this
down a bit. D-Bus defaults to 256 connections per UID, but let's be even
more conservative and go with 128.
This commit runs the check-coccinelle checker scripts with the
parsing_hacks.h. Because this was missing before there were some
issues that did not get flagged.
While at it it also adds some missing cleanup attributes and
iterators to get better results. Its a bit sad that there is no
(easy/obvious) way to detect when new things are needed for
parsing_hacks.h
Coverity was complaining that we we're doing a integer division and then
casting that to double. This was OK, but it was also a bit pointless.
An operation on a double and unsigned promoted the unsigned to a double,
so it's enough if we have a double somewhere as an argument early enough.
Drop noop casts and parens to make the formulas easier to read.
CID#1466459
Sometimes we want need to diff two unsigned numbers, which is awkward
because we need to cast them to something with a sign first, if we want
to use abs(). Let's add a helper that avoids the function call
altogether.
Also drop unnecessary parens arounds args which are delimited by commas.
Christian Brauner
rusty-snake
Daan De Meyer
Kai Lüke
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
Arnout Engelen
Milan Kyselica
Daan De Meyer
Luca Boccassi
Frantisek Sumsal
Michael Vogt
joo es