storagectl's mount.storage helper bundles "open StorageProvider socket
+ Acquire() + dispatch reply + take fd" inline. Future consumers
(systemd-vmspawn boot-time --bind-volume, machinectl bind-volume) need
the same dance.
Factor it into a single libshared helper that takes the Acquire()
parameters by value and returns the fd plus the actual type/read-only
flags. Library code, so no logging — varlink errors are surfaced via
sd_varlink_error_to_errno() and the StorageProvider error_id is
returned to the caller via reterr_error_id (caller decides how to
format messages).
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Let's cap the number of states/patterns per query to something
reasonable, i.e. max 256 states and 4K patterns per query.
---
Also, use the env assignments limit in a couple more places.
Turns out we can utilize this limit at a couple more places, so let's
move the previously defined limit constant to env-util.h and use it to
guard a couple more D-Bus methods. Also, bump it a bit, given it's meant
to be a safety cap that can't be hit in valid scenarios.
Follow-up for 7671b43cb8.
I am really not a fan of full code lines passed to macros as parameters.
Let's get rid of the 3rd parameter of FOREACH_OPTION() hence:
1. Let's return errors just as a regular value (though a negative one),
that can be handled via a OPTION_ERROR case statement for the switch.
This normalizes handling of the error, just like any other event
returned by the option parser.
2. In order to avoid exploding the amount of boilerplate in each use
(that just propagates the error on OPTION_ERROR), let's then
introduce an explicit FOREACH_OPTION_OR_RETURN(), that returns from
the calling function on its own (and makes that clear in the name).
Together this cleans up, normalizes the logic and shortens the code.
DEFINE_POINTER_ARRAY_CLEAR_FUNC() generates a helper of the form
helper_array_clear(T *array, size_t n) that drops each element but does
not free the array itself, parallel to DEFINE_POINTER_ARRAY_FREE_FUNC()
for cases where the array has automatic storage duration.
CLEANUP_ELEMENTS() pairs with these helpers to provide a _cleanup_-like
attribute for fixed-size arrays: the bound is taken from ELEMENTSOF(),
and the helper is invoked across the elements at scope exit. Compared to
CLEANUP_ARRAY(), the storage is neither freed nor zeroed.
Migrate various logic across the tree over to the new macros.
sd-device: use DEFINE_POINTER_ARRAY_CLEAR_FUNC() for sd_device_unref_array_clear()
Replace the local device_unref_many() helper with the macro-generated
equivalent.
format-table: switch help-table arrays to CLEANUP_ELEMENTS()
Generate table_unref_array_clear() via DEFINE_POINTER_ARRAY_CLEAR_FUNC()
and convert the help-table arrays in bootctl, cryptenroll, nspawn,
repart and vmspawn to CLEANUP_ELEMENTS(). The arrays no longer need a
trailing NULL slot, so the size matches ELEMENTSOF() of the groups
array.
firewall-util: switch netlink message arrays to CLEANUP_ELEMENTS()
Generate sd_netlink_message_unref_array_clear() via
DEFINE_POINTER_ARRAY_CLEAR_FUNC() in place of the NULL-terminated
sd_netlink_message_unref_many(), and convert the two stack arrays of
sd_netlink_message pointers to CLEANUP_ELEMENTS().
BindPath= in unit files, and --bind= in nspawn/vmspawn doesn't really
cut it to connect arbitrary storage infra to it. Let's do something
about it, and implement a simple, light-weight API for acquiring an fd
to a storage volume. Benefits:
1. the interface can be implemented by anyone, connecting anything to
vmspawn/nspawn/service management
2. very lose coupling: just bind a socket into a well-known dir, done
3. mounting can happen on-demand
Generic Varlink API for services that hand out file descriptors to
storage volumes. Three methods: Acquire() returns an fd for a named
volume (optionally creating it from a template), ListVolumes()
enumerates available volumes, ListTemplates() enumerates supported
creation templates. Volume types follow kernel inode-type naming:
blk (block device), reg (regular file), dir (directory).
Intent is that multiple providers can sit behind AF_UNIX sockets in a
well-known directory and be consumed uniformly by nspawn, vmspawn,
the service manager (BindVolume=) and similar tools.
Merge the two blocks adding tests, since there seems to be
no obvious reason to have two separate blocks, as they both
contain tests from the same libraries.
The intent was good, but we now print two or three of those messages
for each report metrics received on the wire. If the json object is
extensible, then it's all good and we don't need to inundate the user
with this trivial information. (And the message also sounds like
something is wrong or unexpected, when it totally isn't.)
...
(string):1:73: Unrecognized object field 'object', assuming extension.
(string):1:89: Unrecognized object field 'value', assuming extension.
json-stream: Received message: {"parameters":{"name":"io.systemd.Network.CarrierState","object":"virbr0","value":"degraded-carrier"},"continues":true}
(string):1:66: Unrecognized object field 'object', assuming extension.
(string):1:83: Unrecognized object field 'value', assuming extension.
json-stream: Received message: {"parameters":{"name":"io.systemd.Network.CarrierState","object":"lo","value":"carrier"},"continues":true}
(string):1:66: Unrecognized object field 'object', assuming extension.
(string):1:79: Unrecognized object field 'value', assuming extension.
json-stream: Received message: {"parameters":{"name":"io.systemd.Network.CarrierState","object":"wlp0s20f3","value":"carrier"},"continues":true}
(string):1:66: Unrecognized object field 'object', assuming extension.
(string):1:86: Unrecognized object field 'value', assuming extension.
...
As is often the case, in this case because of alignment, we are actually
not saving any space. With the bitfield we are using one bit of the 8 bytes
allocated, and without the bitfield we are using 8 bits of that.
But we're paying a price in generated code, at every access site to the
field:
$ diff <(objdump -S build/libsystemd.so.old) <(objdump -S build/libsystemd.so.new)
...
v->protocol_upgrade = false;
- fa2d2: 48 8b 45 a8 mov -0x58(%rbp),%rax
- fa2d6: 0f b6 90 90 01 00 00 movzbl 0x190(%rax),%edx
- fa2dd: 83 e2 fe and $0xfffffffe,%edx
- fa2e0: 88 90 90 01 00 00 mov %dl,0x190(%rax)
+ fa2a9: 48 8b 45 a8 mov -0x58(%rbp),%rax
+ fa2ad: c6 80 90 01 00 00 00 movb $0x0,0x190(%rax)
It was requested to make the 'c', 'opt', and 'arg' params the same, i.e.
defined through the FOREACH_OPTION macro. But we can't do that easily,
because 'c' was defined in the for loop definition, and we can only
define variables of the same type in that way. Also, in some cases we
need only 'c', in other cases with need 'c' and 'arg, in some cases 'c'
and 'opt', and in other cases all three. We'd need to either
conditionalize or mark those variables with _unused_ to deal with
compiler warnings. But a different approach works quite nicely: add
state.opt and state.arg to show the current option and it's argument.
(The short names are picked on purpose to reduce verbosity since those
are used a lot.)
json_stream_format_queue() drains queued output items into the output buffer and
stages their fds in n_output_fds, relying on the downstream sendmsg() to deliver
bytes-and-ancillary atomically as one SCM_RIGHTS message. If the output buffer
already holds bytes (from a prior fast-path enqueue that hasn't been sent yet or
from a partial write), concatenating a new fd-bearing item's JSON into it means
the next sendmsg() ships the combined bytes with those fds attached — violating
the per-message fd boundary on transports where that boundary is load-bearing.
Bail out of the drain loop when we would cross that boundary, so the next
write() first sends the buffered bytes with no ancillary, then pulls the
fd-bearing item into a clean buffer and ships it on its own sendmsg.
This only produces an observable difference for SOCK_SEQPACKET / SOCK_DGRAM
consumers: on those transports each sendmsg() is its own datagram with its own
SCM_RIGHTS cmsg, so whether we concatenate matters. On AF_UNIX SOCK_STREAM
(today's sole consumer shape, used by sd-varlink and the QMP client) the kernel
absorbs a preceding non-scm skb forward into the next scm-bearing skb's recv,
so per-sendmsg separation is invisible to the receiver anyway — the guard is
cheap defensive sender hygiene there, not a behaviour change. It becomes load-
bearing the moment a SEQPACKET/DGRAM consumer wires JsonStream up.
Add --forward-journal=FILE|DIR to forward the container's journal
entries to the host via systemd-journal-remote. When specified,
nspawn starts systemd-journal-remote listening on a Unix socket,
bind-mounts it into the container at /run/host/journal/socket, and
passes a journal.forward_to_socket credential pointing to it.
Add --forward-journal-max-use=, --forward-journal-keep-free=,
--forward-journal-max-file-size=, and --forward-journal-max-files=
to configure disk usage limits for the forwarded journal.
Consolidate nspawn's per-machine on-disk state under a single runtime
directory at /run/systemd/nspawn/<machine>/. The container rootdir
mount point moves from /tmp/nspawn-root-XXXXXX to <runtime_dir>/root,
the unix-export directory moves from
/run/systemd/nspawn/unix-export/<machine> to <runtime_dir>/unix-export,
and the journal-remote socket lives at
<runtime_dir>/journal-remote-socket. Update ssh-generator and
ssh-proxy to follow the new unix-export path layout.
Extract fork_journal_remote() into fork-notify.{c,h} as a shared
helper used by both nspawn and vmspawn, replacing vmspawn's
start_systemd_journal_remote().
Extract runtime_directory_make() into path-lookup.{c,h} as a shared
helper used by both nspawn and vmspawn, replacing vmspawn's inline
runtime directory creation logic.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Every dlopen_xxx() helper now takes an int log_level argument. It is
passed through to dlopen_many_sym_or_warn() (which in turn propagates it
to dlopen_verbose() for the library-not-installed case), and is used by
the fallback stub when support for the library is not compiled in to
emit a "<lib> support is not compiled in." message at the caller's level.
Callers pass LOG_DEBUG when gracefully degrading, or a higher level when
the failure should surface, and no longer need to log redundantly at the
call site.
As part of this, dlopen_bpf_full() (which already took a log_level) is
merged into dlopen_bpf() rather than keeping both.
The static inline fallbacks used to live in the headers, which required
pulling log.h in from every header that declared a dlopen_xxx(). Move
them into the .c files instead: the declaration is always outside the
#if HAVE_XXX block, the impl sits outside the outer #if HAVE_XXX wrap
with its own internal #if HAVE_XXX/#else/#endif, and apparmor-util.c,
idn-util.c, libmount-util.c and pam-util.c are now always compiled so
they can host their stubs.
The help string is adjusted/reworded. In particular, [a, b) is used
as notation to show a closed-open range, instead of the unusual <a; b).
--help is shown in --help.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Split the single directory fd that chaseat() used to take into two separate
fds: a root_fd that sets the chroot boundary (symlinks may not escape it,
absolute symlinks resolve relative to it), and a dir_fd that path resolution
starts from. This makes the chroot semantics of chaseat() explicit at every
call site instead of encoding them in the CHASE_AT_RESOLVE_IN_ROOT flag,
which is removed. It also decouples the starting directory from the root
boundary, so callers can descend from any inode inside the tree without
having to reopen the root separately.
XAT_FDROOT passed as root_fd means "no containment" (host root); as dir_fd
it means "start at root_fd". For a smoother transition, AT_FDCWD is also
accepted as root_fd and treated as XAT_FDROOT. When root_fd points to a
directory that is actually the host root, it is normalized to XAT_FDROOT
up front so the existing shortcut path can kick in.
Absolute paths returned by chaseat() are now relative to root_fd, and
relative paths are relative to dir_fd. The result is absolute only when
there is no chroot boundary (root_fd is XAT_FDROOT), or when an absolute
symlink made resolution jump out of the dir_fd subtree; otherwise callers
get a relative path they can feed straight back into an openat()-style
call against dir_fd. Specifically, when dir_fd == root_fd and we're not
operating on the host's root directory, we return a relative path even if
we received an absolute path or resolved an absolute symlink to allow
passing the path directly to openat() style functions. We do this to not
have to go modify every caller of chaseat() to make sure they deal properly
with any absolute paths they might receive. Only when root_fd != dir_fd do
we have to return an absolute path to indicate that the path is relative to
root_fd and not dir_fd.
The shortcut that skips the per-component walk is reworked around a new
chase_xopenat() helper that funnels CHASE_NOFOLLOW, CHASE_MUST_BE_* and
CHASE_TRIGGER_AUTOFS through xopenat_full()'s O_NOFOLLOW, O_DIRECTORY,
XO_REGULAR, XO_SOCKET and XO_TRIGGER_AUTOMOUNT flags. As a result these
flags no longer force us off the shortcut and can be dropped from
CHASE_NO_SHORTCUT_MASK, and the old openat_opath_with_automount() helper
goes away. A CHASE_MUST_BE_ANY alias is introduced for shortcut callers
(stat/access paths) that don't go through xopenat_full() and still need
to bail on those flags locally.
All *_and_* helpers built on top of chaseat() (chase_and_openat,
chase_and_opendirat, chase_and_statat, chase_and_accessat,
chase_and_fopenat_unlocked, chase_and_unlinkat, chase_and_open_parent_at)
gain the same root_fd parameter, and every call site in the tree is
ported to the new signature.
chase_and_open() is also fixed to correctly handle CHASE_EXTRACT_FILENAME
without CHASE_PARENT.
The json-stream API previously exposed JsonStreamQueueItem and several
functions operating on it (json_stream_make_queue_item(),
json_stream_enqueue_item(), json_stream_queue_item_free(),
json_stream_queue_item_get_data()). These existed solely to support
sd-varlink's "defer-and-modify" pattern for streaming replies, where a
reply is held back so its "continues" field can be set before
transmission. This is a varlink protocol concern that should not leak
into the generic transport layer.
Similarly, the fd pushing API (json_stream_push_fd(),
json_stream_reset_pushed_fds()) and the pushed_fds state lived inside
JsonStream, even though fd-to-message association is a protocol-level
concern managed entirely by sd-varlink.
Rework the API so that:
- JsonStreamQueueItem and all its functions become static to
json-stream.c. The only output API is now json_stream_enqueue_full()
(accepting explicit fds) and the inline json_stream_enqueue() wrapper
for the common no-fds case.
- The pushed_fds state moves from JsonStream into sd_varlink, where
sd_varlink_push_fd() and sd_varlink_reset_fds() manage it directly.
- The deferred reply in sd-varlink changes from a JsonStreamQueueItem*
to a plain sd_json_variant* plus a separate previous_fds/n_previous_fds
pair, keeping the protocol-specific bookkeeping in sd-varlink where it
belongs.
- A new varlink_enqueue() helper wraps json_stream_enqueue_full() with
the varlink connection's pushed fds, transferring fd ownership to the
queue item on success.
qmp-client.c is fixed to use the new API as well.
sd_varlink_error() deliberately returns a negative errno mapped from
the error id on success so callbacks can `return sd_varlink_error(...);`
to enqueue the reply and propagate a matching errno at once. When
varlink_dispatch_method() dispatches a configured error sentinel itself,
it doesn't need that mapping — but it was treating any negative return
as a dispatch failure and logging "Failed to process sentinel" even
though the error reply had been successfully enqueued.
Detect success via the state transition to VARLINK_PROCESSED_METHOD
instead, so only genuine enqueue failures are logged.
Move json_stream_description(), json_stream_log(), and
json_stream_log_errno() from json-stream.c into json-stream.h so that
consumers like the QMP client can use the same description-prefixed
logging that json-stream itself uses internally.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
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.
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).
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 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.
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.
Christian Brauner
Diego Viola
Lennart Poettering
Frantisek Sumsal
Daan De Meyer
Zbigniew Jędrzejewski-Szmek
Christian Brauner
Yu Watanabe
Michael Vogt