If a unit has FileDescriptorStorePreserve=yes we'll keep its FDs
around in case it starts again. But if there are no reverse
dependencies referencing it, we'll also GC it and lose all the FDs,
which defeats the point of the setting (which is opt-in).
Do not GC units that have FDs stored to avoid this.
Follow-up for b9c1883a9c
This commit tweaks the TEST-74-AUX-UTILS.varlinkctl.sh code
to use `systemd-notify --fork $UPGRADE_SERVER` instead of
the (ugly) timeout.
This also fixes a stale comment in around
`Test --upgrade with stdin redirected from a regular file`.
Thanks to Daan for suggesting this!
Instead of exiting in exec_with_listen_fds() just return an error
and do the actual _exit() in the caller. Much nicer this way.
Thanks for Lennart for suggesting this.
This commit fixes an issue with the fd handling in
sd_varlink_call_and_upgrade() when one direction of the output
FDs is unset.
Thanks to Lennart for spotting this and suggesting the fix.
Pass -no-user-config while we're at it to avoid loading qemu config
from /etc which is more likely to cause hard to debug issues rather
than do something useful.
Reduce virtiofsd log noise by setting --log-level=error, and drop
the unnecessary mknod capability with --modcaps=-mknod, matching
mkosi's virtiofsd invocation.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Fix two issues in WRITING_VM_AND_CONTAINER_MANAGERS.md:
1. The Host OS Integration section implied that -M switch and
machinectl shell/login work for VMs, but they currently only
work for containers. Add a note clarifying this limitation.
2. The Guest OS Integration section said "there's only one" VM
integration API (SMBIOS Product UUID), but VM_INTERFACE.md
documents five. Replace the outdated single-API description
with a reference to VM_INTERFACE.md listing all five.
Fixes#40935
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
This enables deriving the minimum size of the `Verity=hash` partition
using the `Verity=` logic when the size of the `Verity=data` partition
is bigger than the `CopyBlocks=` target.
This enables using `Minimize=true` for an "installer image" and later
using sd-repart to install to a system with reserve space for future
updates by specifying `Size{Min,Max}Bytes=` only in the `Verity=data`
partition, without needing to hardcode the corresponding size for the
`Verity=hash` partition.
While not strictly necessary for `Verity=signature` partitions (since
they have a fixed size) there isn't too much reason to not support it,
since then you can still specify `VerityMatchKey=` to indicate that
the partition is logically still part of that group of partitions.
We ensure that if one of the hash uses `CopyBlocks=` that the data
partition does so as well. Similarly if the signature partition does it
checks that the hash and data partition do so as well. This is to
minimize the chance of accidental misconfiguration of mixing
`CopyBlocks=auto` and `CopyBlocks=<path>` and of manually populating
partitions while hash/sig partitions are copied from existing sources.
Follow-up for af5126568a ("nspawn: keep backing files for boot_id and
kmsg bind mounts alive").
The backing files for the boot_id and kmsg bind mounts were previously
created in the inner child. However, /run/host/ is remounted read-only
by mount_all() in the inner child (via the MOUNT_IN_USERNS mount table
entry) before setup_boot_id() and setup_kmsg() run, so creating files
there would fail with EROFS.
Fix this by splitting the file creation into separate functions
(setup_boot_id_file() and setup_kmsg_fifo()) that run in the outer
child, where /run/host/ is still writable. The bind mounts onto /proc
remain in the inner child, since procfs is only mounted there.
Also move the backing files from /run/ to /run/host/ and drop the dot
prefix, since /run/host/ is the container-manager-owned namespace and
there is no need to hide these files there. Additionally, apply
userns_lchown() to the created files, matching the convention used by
all other outer child functions that create files in the container.
The collection of facts is entirely unprivileged and has very little to
do with PID1. PID1 is privileged and single-threaded and a point of
contention, so we shouldn't put things in PID1 that don't need to be
there. A separate service can be enabled/disabled/started/stopped at
will, is easy to sandbox, etc. If it turns out to be necessary to
collect some facts through PID1 in the future, we can always add a
smaller facts endpoint to PID1 again.
The main agent doesn't need to read pr-context.json until all
reviews have finished. This should prevent it from passing unnecessary
data from pr-context.json in the prompt to its subagents, which can just
read that file themselves when needed.
We now have three kinds of endpoints under /run/systemd/report/:
those that implement facts, those that implement metrics, and those that
implement both. Let's downgrade the message to avoid pointless warnings.
The Linux kernel does not reliably auto-detect serial consoles on
headless systems. While the docs claim serial is used as a fallback
when no VGA card is found, in practice CONFIG_VT's dummy console
(dummycon) registers early and satisfies the kernel's console
requirement, preventing the serial fallback from ever triggering. The
ACPI SPCR table can help on ARM/RISC-V where QEMU generates it, but
x86 QEMU does not produce SPCR, and SPCR cannot describe virtio
consoles at all. This means UKIs booted via sd-stub in headless VMs
produce no visible console output unless console= is explicitly
passed on the kernel command line.
Fix this by having sd-stub auto-detect the console type and append an
appropriate console= argument when one isn't already present.
Detection priority:
1. VirtIO console PCI device (vendor 0x1AF4, device 0x1003): if
exactly one is found, append console=hvc0. This takes highest
priority since a VirtIO console is explicitly configured by the
VMM (e.g. systemd-vmspawn's virtconsole device). If multiple
VirtIO console devices exist, we cannot determine which hvc index
is correct, so we skip this path entirely.
2. EFI Graphics Output Protocol (GOP): if present, don't add any
console= argument. The kernel will use the framebuffer console by
default, and adding a serial console= would redirect the primary
console away from the display.
3. Serial console: first, we count the total number of serial devices
via EFI_SERIAL_IO_PROTOCOL. If there are zero or more than one,
we bail out — with multiple UARTs, the kernel assigns ttyS indices
based on its own enumeration order and we cannot determine which
index the console UART will receive. Only when exactly one serial
device exists (guaranteeing it will be ttyS0) do we proceed to
verify it's actually used as a console by checking for UART device
path nodes (MESSAGING_DEVICE_PATH + MSG_UART_DP). The firmware's
ConOut handle is checked first; if it has no device path (common
with OVMF's ConSplitter virtual handle when using -nographic
-nodefaults), we fall back to enumerating all
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL handles and checking each one's
device path. The architecture-specific console argument is then
appended:
- x86: console=ttyS0
- ARM: console=ttyAMA0
- Others: console=ttyS0 (RISC-V, LoongArch, MIPS all use ttyS0)
Note on OVMF's VirtioSerialDxe: it exposes virtio serial ports with
the same UART device path nodes as real serial ports (ACPI PNP 0x0501
+ MSG_UART_DP), making them indistinguishable from real UARTs via
device path inspection alone. This is why we check for the VirtIO
console PCI device via EFI_PCI_IO_PROTOCOL before falling back to
device path analysis.
Also add a minimal EFI_PCI_IO_PROTOCOL definition (proto/pci-io.h)
with just enough to call Pci.Read for vendor/device ID enumeration,
and add the MSG_UART_DP subtype to the device path header.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
hmac_sha256() leaves four stack buffers containing key-derived material
(inner_padding, outer_padding, replacement_key, hash state) on the stack
after returning. The inner_padding and outer_padding arrays contain
key XOR 0x36 and key XOR 0x5c respectively, which are trivially
reversible to recover the original HMAC key.
This function is called with security-sensitive keys including the LUKS
volume key (cryptsetup-util.c), TPM2 PIN (tpm2-util.c), and boot secret
(tpm2-swtpm.c). The key material persists on the stack until overwritten
by later unrelated function calls.
Add CLEANUP_ERASE() to all four local buffers, following the same
pattern applied to tpm2-util.c in commit 6c80ce6 (PR #41394).
The kernel loop driver has a race condition in LOOP_CONFIGURE when
LO_FLAGS_PARTSCAN is set: it sends a KOBJ_CHANGE uevent (with
GD_NEED_PART_SCAN set) before calling loop_reread_partitions(). If
udev opens the device in response to the uevent before
loop_reread_partitions() runs, the kernel's blkdev_get_whole() sees
GD_NEED_PART_SCAN and triggers a first partition scan. Then
loop_reread_partitions() runs a second scan that drops all partitions
from the first scan (via blk_drop_partitions()) before re-adding them.
This causes partition devices to briefly disappear (plugged -> dead ->
plugged), which breaks systemd units with BindsTo= on the partition
device: systemd observes the dead transition, fails the dependent
units with 'dependency', and does not retry when the device reappears.
Work around this in loop_device_make_internal() by splitting the loop
device setup into two steps: first LOOP_CONFIGURE without
LO_FLAGS_PARTSCAN, then LOOP_SET_STATUS64 to enable partscan. This
avoids the race because:
1. LOOP_CONFIGURE without partscan: disk_force_media_change() sets
GD_NEED_PART_SCAN, but GD_SUPPRESS_PART_SCAN remains set. If udev
opens the device, blkdev_get_whole() calls bdev_disk_changed()
which clears GD_NEED_PART_SCAN, but blk_add_partitions() returns
early because disk_has_partscan() is false — no partitions appear,
the flag is drained harmlessly.
2. Between the two ioctls, we open and close the device to ensure
GD_NEED_PART_SCAN is drained regardless of whether udev processed
the uevent yet.
3. LOOP_SET_STATUS64 with LO_FLAGS_PARTSCAN: clears
GD_SUPPRESS_PART_SCAN and calls loop_reread_partitions() for a
single clean scan. Crucially, loop_set_status() does not call
disk_force_media_change(), so GD_NEED_PART_SCAN is never set again.
A proper kernel fix has been submitted:
https://lore.kernel.org/linux-block/20260330081819.652890-1-daan@amutable.com/T/#u
This workaround should be dropped once the fix is widely available.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Both setup_boot_id() and setup_kmsg() previously created temporary files
in /run, bind mounted them over their respective /proc targets, and then
immediately unlinked the backing files. While the bind mount keeps the
inode alive, the kernel marks the dentry as deleted.
This is a problem because bind mounts backed by unlinked files cannot be
replicated: both the old mount API (mount(MS_BIND)) and the new mount
API (open_tree(OPEN_TREE_CLONE) + move_mount()) fail with ENOENT when
the source mount references a deleted dentry. This affects
mount_private_apivfs() in namespace.c, which needs to replicate these
submounts when setting up a fresh /proc instance for services with
ProtectProc= or similar sandboxing options — with an unlinked backing
file, the boot_id submount simply gets lost.
Fix this by using fixed paths (/run/proc-sys-kernel-random-boot-id and
/run/proc-kmsg) instead of randomized tempfiles, and not unlinking them
after the bind mount. The files live in /run which is cleaned up on
shutdown anyway.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace claude-code-action with a direct claude CLI invocation. This
gives us explicit control over settings, permissions, and output
handling.
Other changes:
- Prepare per-commit git worktrees with pre-generated commit.patch and
commit-message.txt files, replacing the pr-review branch approach.
- Use structured JSON output (--output-format stream-json --json-schema)
instead of having Claude write review-result.json directly.
- Use jq instead of python3 for JSON prettification.
- Add timeout-minutes: 60 to the review job.
- List tool permissions explicitly instead of using a wildcard.
- Fix sandbox filesystem paths to use regular paths instead of the "//"
prefix.
When callers do not explicitly request read-only mode, pass open_flags
as -1 (auto-detect) instead of hardcoding O_RDWR. This enables the
existing O_RDWR-to-O_RDONLY retry logic in loop_device_make_by_path_at()
which falls back to O_RDONLY when opening the backing device with O_RDWR
fails with EROFS or similar errors.
Previously, callers passed O_RDWR explicitly when read-only mode was not
requested, which bypassed the retry logic entirely. This meant that
inherently read-only block devices (such as CD-ROMs) would fail to open
instead of gracefully falling back to read-only mode.
Also propagate the unresolved open_flags through
loop_device_make_by_path_at() into loop_device_make_internal() instead
of resolving it to O_RDWR early. For loop_device_make_by_path_memory(),
resolve to O_RDWR immediately since memfds are always writable.
In mstack, switch from loop_device_make() to
loop_device_make_by_path_at() with a NULL path, which reopens the
O_PATH file descriptor with the appropriate access mode. This is
necessary because the backing file descriptor is opened with O_PATH,
which prevents loop_device_make_internal() from auto-detecting the
access mode via fcntl(F_GETFL).
The shared/ dns_packet_read_uint*() functions used to be a bit
inconsistent when it comes to the handling of NULL in *ret. The
helpers dns_packet_read_{uint16,name} accepted NULL but the
dns_packet_read_uint{8,32} did not.
This commit makes them all symmetric in accepting NULL.
Some functions are in the hot path of a sort. While it is more
correct to also use assert() here for the passed pointers it is
also a runtime cost that should never be needed when e.g. qsort()
is used as it will always pass non-NULL pointers.
So for these critical functions use POINTER_MAY_BE_NULL() which
is compiled out and add a comment to them. Not all _compare()
functions are changed, it was a bit of a judgement call because
some (like ether_addr_compare) are not called with that much data.
But happy to get advise here and tweak accordingly.
Add the needed assert changes to make the code clean
for the new check-pointer-deref script.
Note that for `_cmp()` functions its a bit of a question
if we want `assert()` or `POINTER_MAY_BE_NULL()` here. The
former is more correct but it adds a (small) runtime hit. If
we use POINTER_MAY_BE_NULL() it is compiled out but strictly
speaking it is wrong (so at least we would need a comment).
There are some unnecessary() if() checks for variables
that are already guarded by assert()s. Drop those.
Thanks to keszybz for clarifying the direction of this commit.
The previous code used strv_join() when it generated the log
message for `varlinkctl --exec`. However this can lead to
inaccurate logging so use `quote_command_line()` instead.
Thanks to Lennart for suggesting this.
Having support for `--exec` when using `--upgrade` is nice so this
commit adds it. It does it by extracting a shared helper called
`exec_with_listen_fds()` and then use that in the `verb_call()`
and `varlink_call_and_upgrade()` calls.
The varlink spec supports protocol upgrades and they are very
useful to e.g. transfer binary data directly via varlink. So
far varlinkctl/sd-varlink was not supporting this. This commit
adds support for it in varlinkctl by using the new code in
sd-varlink and the generalized socket-forward code.
The SimplexForwader was using the naming of the SocketForwarder
for bi-directional sockets. This was to keep the diff small and
to make it easier to follow what changed and what was reused.
However the name "client/server" for the SimplexForwader does
no longer make much sense. The SimplexForwader is no longer
about client/server but really just read/write. So this commit
adjusts the naming.
Now that the socket forward code is extracted we can
extend it to not just support bidirectional sockets
but also input/output fd-pairs. This will be needed
for e.g. the varlinkctl protocol upgrade support where
one side of the connection is a fd-pair (stdin/stdout).
This is done by creating two half-duplex forwarders
that operate independantly. This also allows to simplify
some state tracking, e.g. because each fd serves only one
direction we don't need to dynamically create the event mask
with EPOLLIN etc, its enough to set it once. It also handles
non-pollable FDs transparently.
Thanks to Lennart for his excellent suggestions here.
Luca Boccassi
Michael Ferrari
Michael Vogt
Daan De Meyer
Kit Dallege
Christian Hesse
Zbigniew Jędrzejewski-Szmek
Yaping Li
dependabot[bot]
Michal Rybecky
Tobias Heider