This time, various commands with non-trivial requirements are handled.
So the series is shorter, but has a pattern of feature additions and
then those new features being used. The way that --help output is
formatted is changed a bit in various places. Those changes are
described in individual commits.
22 'static int help()' functions left after this.
We already have io.systemd.service.SetLogLevel. Add a getter so that
CLI tools with `log-level` subcommands can be more easily implemented
with varlink.
Bind this method to all services that already bind io.systemd.service.SetLogLevel.
Almost all programs are simple and have only a single command. When
printing options, let's just use the first COMMAND block found by default.
This means we can drop the explicit program-name argument and reduce
copy&paste boilerplate.
Track which machine the cached address list belongs to and refresh it
when ResolveRecord() advances to another machine. This keeps A/AAAA
reuse for the same machine while avoiding mixed name/address records.
Reproducer: register two machines with private addresses 10.88.1.1
and 10.88.2.1, then call io.systemd.Resolve.Hook.ResolveRecord
with both A questions in one request.
Before: the answer for the second machine used its own name but
reused the first machine's address, e.g. resolve-bug-m2 returned
10.88.1.1 instead of 10.88.2.1.
Follow-up: 3cd929f837
Previously "machinectl shell --uid=1000 <container>" resulted in a
sucessful drop into a shell in the respective target machine. After
commit a9e9288288 this is no longer the
case.
This fixes the above breaking change brought in commit a9e9288288
Reflinking is a safe optimization whenever the source and destination are
regular, seekable files on a filesystem that supports cloning. Requiring each
caller to opt in through COPY_REFLINK adds flag plumbing without changing the
necessary fallback behavior.
Drop COPY_REFLINK, renumber the remaining flags, and have copy_bytes_full()
unconditionally try FICLONE or FICLONERANGE before entering its normal copy
loop. Isolate the clone attempt and its file-offset bookkeeping in a helper so
copy_bytes_full() only has to handle its cloned, unavailable, and error results.
A successful clone therefore completes in one operation, while an unsupported
or rejected clone falls back to progress-limited copy_file_range(), sendfile(),
or buffered copying. Keep the public reflink helpers for operations that
specifically require clone semantics.
Existing test-copy coverage exercises both bounded and unbounded regular-file
copies through copy_bytes_full().
Signed-off-by: Daan De Meyer <daan@amutable.com>
Previously, even though sd-journal does not support bzip2 and gzip
compression, the dlopen ELF notes for those libraries were still being
attached to libsystemd.so and various journal-handling executables.
This occurred because the unified compression/decompression interfaces
handled all formats unconditionally, causing the compiler and linker to
pull in all associated dlopen notes across the board.
To resolve this, split these functions into generic and journal-specific
variants (e.g., introducing compress_blob_journal() and decompress_blob_journal()).
The journal-specific variants only handle formats actually supported by
the journal (LZ4, XZ, and ZSTD).
By updating sd-journal, journald, journalctl, and related utilities to
use these new `_journal` interfaces and switching them to the narrower
COMPRESS_JOURNAL_NOTE macro, we ensure that unnecessary dlopen notes
(for bzip2 and gzip) are no longer embedded into these binaries.
Previously, when a library was dynamically loaded via a helper function
inside libsystemd-shared.so, the resulting dlopen ELF note was not
propagated to the invoking executable's ELF metadata.
This commit explicitly annotates each executable with the relevant
dlopen ELF notes for any optional dependencies it might potentially
load. This ensures that package managers and build systems can properly
discover runtime dependencies that are triggered indirectly.
We have two concepts: a list of files that should be extracted from an
intermediate build artifact to be used in a later build artifact, and for a
given artifact, a list "donors". The first list was called "extract", because
it is passed to the .extract_objects() meson function, and the second was
called "objects", because that is the meson parameter to specify pre-built
objects files (6350d2dbd9). But this naming is
confusing: we don't care about the 'extract' step, this is something internal
to the build machinery. And 'objects' is a very generic term.
Let's use 'export' for the stuff that is "exported" for other binaries to use,
and 'import' to say where to import from. Those terms are symmetrical and the
association between them should be intuitive. (If you think the terms are
actually assymetrical, there are precedents for confusing the import with the
import sources. E.g. in modern English, turkeys are called so because they were
imported from the Americas and guineafowls were imported from Africa via
Türkiye and all that foreign stuff is alike.)
The new polkit will return a new detail regarding a successful
authentication: the actual result type, which we can use to
see whether the user authenticated as admin. This can be used
to grant additional privileges.
Drop the -fundamental suffix from src/fundamental/ headers in favor of names
that match their src/basic/ or src/shared/ counterparts (e.g.
macro-fundamental.h -> macro.h, assert-fundamental.h -> assert-util.h,
cleanup-fundamental.h -> cleanup-util.h). Rename src/basic/{btrfs,label}.{c,h}
to use the -util suffix to match the existing shared/btrfs-util and
shared/label-util siblings. Rename src/shared/mkdir-label.{c,h} to mkdir.{c,h}
and src/shared/tmpfile-util-label.{c,h} to tmpfile-util.{c,h} to match the
corresponding src/basic names.
This saves us from having to come up with separate names for files that do
the same thing across tiers, and it makes it easier to move stuff between
src/fundamental, src/basic and src/shared: consumers just #include "foo.h"
and pick up whichever tier their -I path resolves to first, so call sites
don't need to be updated when an API moves between layers.
Where a higher-tier wrapper exists (e.g. src/basic/macro.h wrapping
src/fundamental/macro.h), the wrapper uses an explicit "../fundamental/foo.h"
or "../basic/foo.h" relative include for the lower-tier header. We can't use
GCC's #include_next directive for this — when the wrapper is reachable both
via same-dir-as-source lookup and via -I (e.g. -Isrc/shared) for the
directory it lives in, #include_next advances by exactly one slot in libcpp's
internal directory chain and lands on the same physical directory it was
already in, never reaching the lower-tier sibling (see make_cpp_dir() in
gcc/libcpp/files.cc:1986).
To make sure the right headers are always picked up, the include directories
are reordered so that e.g. src/shared always takes priority over src/basic and
similar for the other directories.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
Our baseline glibc is 2.34, which merged libdl, libpthread (the
dependency('threads') target), and librt into libc. Empty .so/.a stubs
remain for backward compatibility with old binaries, but new builds
resolve dl_*, pthread_*, mq_*, timer_*, etc. directly from libc.
On musl the same libraries are likewise empty stubs.
Drop the libdl, threads, and librt entries from every meson.build, and
remove the now-stale 'Libs.private: -lrt -pthread' from libudev.pc.in
since both flags resolve to empty link-time stubs on glibc 2.34+ and
musl.
Verified with readelf -d that libsystemd.so, libudev.so, and systemd no
longer carry DT_NEEDED entries for libdl/libpthread/librt.
For the shell verb we want switches specified after the program name to
be passed to the program to execute, not processed by us. Mirror the
approach in 'userdbctl ssh-authorized-keys': start with
OPTION_PARSER_RETURN_POSITIONAL_ARGS, then lates switch to
STOP_AT_FIRST_NONOPTION for "shell" or NORMAL otherwise.
VERB declarations are placed directly above each function; functions
that dispatch multiple verb names get stacked VERB() declarations.
chainload_importctl() now takes the args strv instead of relying on the
global optind.
--help output is mostly the same.
--no-pager/--no-legend/--no-ask-password/-q/--quiet are now at the end.
bind-volume/unbind-volume are documented.
Fixes c9f461a806.
Co-developed-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The net diff is negative because some spurious whitespace and forward
declarations were dropped. One new forward declaration was added. (For
verb_poweroff_machine. The func could be moved, but I think it's better
to keep it adjacent to verb_reboot_machine which is very similar.)
machinectl bind-volume MACHINE PROVIDER:VOLUME[:CONFIG][:K=V,...]
machinectl unbind-volume MACHINE PROVIDER:VOLUME
For bind-volume, machinectl parses the SPEC with the shared
bind_volume_parse(), Acquires the storage volume from the named
provider on the machinectl side, locates the target machine's
io.systemd.MachineInstance control socket via
machine_get_control_address(), pushes the fd across, and calls
io.systemd.MachineInstance.AddStorage with name='<provider>:<volume>'
and the user-supplied config string.
For unbind-volume, machinectl just forwards the name string to
io.systemd.MachineInstance.RemoveStorage.
Volumes attached at machine startup (e.g. via systemd-vmspawn's
--bind-volume=) are rejected with StorageImmutable when the user
attempts to unbind them at runtime.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
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.
ASAN showed a use-after-free error for systemd-vmspawn's
machine_register call because the reply got accessed and freed again
through _cleanup. The same problem exists in two
verb_machine_control_one/unregister_machine.
Fix these call sites to not set up _cleanup.
Ensure only privileged users can call the system scope machined's
APIs that get data out of a machine
Follow-up for 1bd979dddb
Follow-up for 9153b02bb5
Add new verbs for controlling vmspawn VMs:
machinectl poweroff <machine>
machinectl pause <machine>
machinectl resume <machine>
Each verb discovers the machine's varlinkAddress via machined's
Machine.List, connects directly to vmspawn's varlink socket, and
calls the corresponding io.systemd.MachineInstance method.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Follow the existing sshAddress pattern to add a controlAddress field
that allows machine registrants (like vmspawn) to advertise a varlink
socket address for direct VM control. machined stores and exposes
the address but never connects to it itself.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Follow-up to #40972. Convert remaining plain string fields to proper
varlink enum types across all interfaces, per the policy that
user-controlled/API fields should be declared as proper enums in the
IDL.
Interfaces converted:
- io.systemd.Manager — LogTarget, DefaultStandardOutput/Error,
DefaultMemory/CPU/IOPressureWatch, DefaultOOMPolicy,
CtrlAltDelBurstAction (8 fields)
- io.systemd.Unit — CPUSchedulingPolicy, IOSchedulingClass, NUMAPolicy,
MountFlags (4 fields)
- io.systemd.Machine — class, whom (2 fields, input dispatch converted
to JSON_DISPATCH_ENUM_DEFINE for automatic dashify fallback)
- io.systemd.oom / io.systemd.ManagedOOM — mode in ControlGroup (1
field)
Shared types moved to varlink-idl-common: ExecOutputType,
CGroupPressureWatch, EmergencyAction, ManagedOOMMode — these are reused
across multiple interfaces.
Each interface change includes a corresponding enum sync test to catch
future drift between C string tables and varlink IDL definitions.
Convert the class field (Register input, List output) from plain string
to MachineClass enum type, and the whom field (Kill input) to KillWhom
enum type.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Instead of interpolating the user name directly into the sh -c script
body via asprintf %s, pass it as a positional parameter ($1) in a
separate argv entry. This avoids the user string being parsed as part
of the shell script syntax.
Also validate the user name in bus_machine_method_open_shell() with
valid_user_group_name(), matching the validation already done on the
Varlink path via json_dispatch_const_user_group_name().
Follow-up for 49af9e1368
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.
When registering a machine, machined verifies that the leader process
is owned by the calling user via process_is_owned_by_uid(). This
check fails for user scope machined when the leader is inside a user
namespace: after the leader calls setns(CLONE_NEWUSER), it becomes
non-dumpable, and the subsequent ptrace_may_access() check in the
kernel denies access to the process's user namespace, since the
calling user lacks CAP_SYS_PTRACE in the mm's user namespace (the
host namespace), even though the user owns the child user namespace.
Skip this check when running in user scope. For system scope, the
check is important because multiple users share the same machined
instance, so one user must not be able to claim another user's process
as a machine leader. For user scope this is unnecessary: the varlink
socket lives under $XDG_RUNTIME_DIR (mode 0700), so only the owning
user can connect, and the user machined instance can only perform
operations bounded by that user's own privileges. Registering a
foreign PID does not escalate capabilities.
Almost all callers of table_print() specify stdout or NULL (equivalent
to stdout) as the output stream. Simplify things by not requiring the
the stream to be specified.
In almost all cases, the printing of the table is surrounded by normal
printfs() that don't do explicit flushing and for which we don't check
the output stream status. Let's simplify most callers and skip this
step. The reason is not so much to avoid the extra step itself, but
instead to avoid the _handling_ of the potential failure. We generally
only want to print an error message for ENOMEM and other "internal"
errors, so strictly speaking we should filter out the errors from the
stream. By skipping the flush&check step we implicitly do this.
I tried to implement a varlink service using sd-varlink, and
not being able to use the approach with sentinel is exteremely
painful. This is useful internally and likewise externally.
The fix in 8f1751a111 made me wonder if we could automatically detect
when pointers are accessed but when this might not be safe. Systemd
is already using a lot of `assert(dst)` and this change now forces
us to use them.
So this commit (ab)uses coccinelle to flag any pointer parameter
dereference not preceded by assert(param), ASSERT_PTR(param), or an
explicit NULL check. It adds integration into meson as a new "coccinelle"
test suite (just like clang-tidy) and is run in CI. The check is not
perfect but seems a reasonable heuristic.
For this RFC commit it is scoped to a subset, it excludes 25 dirs right
now and includes around 100. About 300 warnings left. Busywork that I am
happy to do if there is agreement that it is worth it.
With this in place we would have caught the bug from 8f1751a111 in CI:
```
FAIL: check-pointer-deref.cocci found issues in systemd/src/boot:
diff -u -p systemd/src/boot/measure.c /tmp/nothing/measure.c
--- systemd/src/boot/measure.c
+++ /tmp/nothing/measure.c
@@ -312,7 +312,6 @@ EFI_STATUS tpm_log_tagged_event(
if (err != EFI_SUCCESS)
return err;
- *ret_measured = true;
return EFI_SUCCESS;
}
```
This also adds a new POINTER_MAY_BE_NULL() for the cases when the
called function will do the NULL check (like `iovec_is_set()`).
We often have a pattern where the same verb function is used for
multiple actions. This leads to an antipattern where we figure out what
action needs to be taken from argv[0] multiple times: often once in
parse_argv() to figure out what options are allowed, then once again
implicitly in dispatch_verb(), and then again in the action verb itself.
Let's allow passing a parameter into the verb to simplify this.
This matches a pattern we have in conf-parser.h, where we have both
void *userdata (more global) and void *data (per-config item). Here,
I opted for uintptr_t userdata. It seems that most of the time we'll
want to just pass an enum value. This works OK with no casts. I also
tried a void* and union. In both cases, much more boilerplate is needed:
either a cast or a macro to help avoid compiler warnings. uintptr_t
seems generic enough to cover foreseeable usecases with no fuss.
This is a noop refactoring. See next commit for an example.
Zbigniew Jędrzejewski-Szmek
Nick Rosbrook
dongshengyuan
cidkidnix
Daan De Meyer
Yu Watanabe
Lennart Poettering
Luca Boccassi
Chitoku
Frantisek Sumsal
Christian Brauner
Kai Lüke
Ivan Kruglov
Michael Vogt
Mike Yuan