--introspect-cli follows the same deferred pattern as --help: parse_argv()
only records the action, and it is executed in the same place where
ACTION_HELP and ACTION_VERSION are handled.
The help is looked up by explicit name: in the multicall configuration
the systemd and systemd-executor commands share one binary, so the
"first COMMAND" default would depend on link order.
The "This program takes no positional arguments." line is dropped from
the help output — the default "[OPTION…]" synopsis already says so.
(And also, this is not quite true. It does take positional args when
called as init.)
In review it was raised that the file is often synthesized and that this
synthetization might not include all the fields, let's allow the more
recently added fields to be missing to avoid a regression in
compatibility.
Extend the io.systemd.Basic metric families with host-level runtime
metrics: cumulative per-state CPU seconds, memory in use (MemTotal minus
MemAvailable), swap in use, the kernel's 10s pressure stall averages
(cpu/memory/io x some/full). Partitions and stacked devices are excluded
from the disk counters, since their traffic is already accounted for in
the parent device.
Also extend the TEST-74 report subtest with checks for the new metrics.
Network metrics are not added, because the service runs with
PrivateNetwork=yes, and it doesn't make sense to report stats for 'lo'.
After some back-and-forth, I settled on "CPUUsage". We use this scheme of
capitalization in other metric family names: "IPv4Public", "IPv6Public",
IPv4AddressState", "IPv6AddressState", "OSRelease" "SMBIOS" "BootID",
"CPUsOnline", "MachineID", "MachineInfo", "OSRelease",
"SMBIOS.SystemManufacturer", "TPM2.Manufacturer", "TPM2.VendorString",
"IOReadBytes", "IOReadOperations", "CGroup". The exception is
"io.systemd.cgroup.CpuUsage". I think we should treat that one as a
mistake and consider changing it to conform.
This drops a good chunk of 21ab2fe6ec
again, which I think is plainly wrong or at the very least too
overengineered.
1. it's wrong to assume that overlayfs layers are mounted elsewhere in
the fs. it's quite likely they aren't. and overlayfs correctly passes
through fsync() to the top layer, hence we really should keep syncing
it.
2. network file systems deserve syncing. This code here runs in the
initrd, where a very restrictive set of file systems is mounted and
which runs for a very short time only, i.e. the circumstances haven't
really changed. Hence I am very sure it's better to sync file systems
here than not to, we do this for safety reasons after all.
Reliability always trumps speed here, we should not give up
correctness over presumed speed improvements.
3. There's zero reason to believe FUSE doesn't need syncing. We live in
a world where increasingly complex kernel file systems are moved to
userspace, and hence FUSE can be used to implement *anything*. Hence
we should sync it. Yes, FUSE means some backing process needs to run,
but that's a job for the initrd to get right: make sure the process
runs properly over the switch root if the mount is left in tact over
the switch root too.
Hence, dumb this all down and drop the weird fine tuning here.
The kernel reports guest usage in guest/guest-nice and also includes it in
user/nice. By adding the guest and guest-nice values, we were counting
those ticks twice.
C.f. https://gitlab.com/procps-ng/procps/-/blob/master/library/stat.c?ref_type=heads#L612-615
> note: we exclude guest tics from xtot since ...
> 'user' already includes 'guest'
> 'nice' already includes 'gnice'
> (see linux sources: ./kernel/sched/cputime.c, kcpustat_cpu_fetch)
Rewrite procfs_cpu_get_usage() to use procfs_cpu_get_ticks() and fix the
accounting error.
Since kernel v6.19, ioctl(PIDFD_GET_INFO) can return the set of
supported flags when PIDFD_INFO_SUPPORTED_MASK is specified.
This allows checking whether, e.g., PIDFD_INFO_COREDUMP_SIGNAL is
supported by the running kernel.
PID, UID, and GID are converted by sending struct ucred, however, TID
was not converted. Let's convert TID if corresponding pidfd exists, and
discards TID otherwise.
Fixes 4c7af7d9d1.
Now that fstab_filter_options() no longer uses strv_join_full() with
escape_separator=true (it splits with EXTRACT_UNESCAPE_RELAX and joins the kept
options verbatim), the escape_separator flag has no users. Drop it from
strv_join_full() and its wrapper strv_join(), and update the remaining callers in
kernel-install and the tests accordingly.
extract_first_word() handled a backslash followed by another character in the
escape-processing branch only when EXTRACT_CUNESCAPE or EXTRACT_UNESCAPE_SEPARATORS
was set. With EXTRACT_UNESCAPE_RELAX alone, the backslash was silently dropped and
only the following character kept, which is inconsistent with the documented "keep
invalid escapes verbatim" intent of RELAX. Let RELAX also enter that branch, so an
invalid escape like "\x" is preserved verbatim as "\x" rather than collapsing to
"x".
EXTRACT_UNESCAPE_RELAX is not used on its own anywhere outside the test cases —
the non-test callers always combine it with EXTRACT_CUNESCAPE, which already enters
this branch — so only the test cases that use RELAX by itself are affected, and
their expectations are updated accordingly.
A newline embedded in an input entry only reset the width counter and the
leading-whitespace state, but the text collected before it was never flushed.
If nothing but whitespace followed the newline, the end-of-line path then saw
that we were still in the leading whitespace and emitted an empty line,
dropping everything before the newline: "foo\n" turned into "".
Newlines also didn't start a new output entry, so returned entries could
contain newlines and be wider than the requested width, even though callers
expect one output entry per output line.
Flush the collected line at the newline instead, reusing the same logic as at
the end of the string, and treat a trailing newline as terminating the last
line rather than adding an empty one.
Alibaba Cloud ECS instances expose "Alibaba Cloud ECS" in DMI
product_name/bios_vendor and "Alibaba Cloud" in sys_vendor. None of
these match dmi_vendor_table[], so systemd-detect-virt --vm reports
"none" on these machines, even though they run on a KVM-based
hypervisor. This is especially visible on ARM64 instances, where the
CPUID fallback does not exist (x86 only) and /proc/device-tree is
not populated.
Add "Alibaba Cloud ECS" to dmi_vendor_table[], mapping to
VIRTUALIZATION_KVM. The narrower prefix avoids matching the bare
"Alibaba Cloud" sys_vendor value, reducing the false-positive
surface for Alibaba's bare-metal (X-Dragon) instances compared to
the original broader prefix. bios_vendor also carries the same
string on ECS VMs though, so the protection is not complete —
fully resolving the VM-vs-bare-metal ambiguity would need the
SMBIOS BIOS-Characteristics bit check that the Amazon EC2 entry
uses.
The existing "back to DMI" fallback path in detect_vm() reports
"kvm" on both x86 and ARM64.
We currently blindly send the configured reload signal (e.g. SIGHUP or
SIGUSR1) to the main service PID for Type=notify-reload units, even if
the service hasn't installed a userspace handler. This can lead to
invoking unintended default behavior (typically process termination) in
daemons which later deprecate and remove their reload handler.
This is a real problem we have seen in production on multiple occasions.
In one particularly egregious instance, a production distributed storage
service had a large percentage of its nodes all terminate at once when
sent a reload signal. In this case a signal handler had been removed,
but another place still sending the signal was missed.
To mitigate this, introduce two new checks for Type=notify-reload
services:
1. On READY=1: When the service first sends READY=1 during initial
startup, we check for the handler. If it's missing, the service startup
is failed with SERVICE_FAILURE_PROTOCOL. This enforces the contract that
services *must* install their handler before signalling readiness.
2. On reload: Immediately before sending the reload signal, we check
again. If the handler is now missing, we log a warning but still send
the signal. This preserves the requested operator action while making
the missing handler visible.
The startup check catches definite static misconfiguration before the
service is accepted as ready. The reload check is advisory because the
service has already started successfully, but warns if its handler later
disappears. Together these provide a best-effort safety net while
preserving the agreed reload semantics.
Add new pidref_has_sigcgt() and pidref_has_sigblk() helpers. These read
the SigCgt and SigBlk bitmasks from /proc/<pid>/status to detect either
traditional signal handlers installed via sigaction(2), or blocked signals
that are typically handled via signalfd(2), such as through
sd_event_add_signal().
Both mechanisms represent valid ways to handle a signal without invoking
the default action. Share the parsing and pidref verification between the
two public helpers.
Explicitly call label_ops_post() where necessary with specific
arguments. Previously, regardless if fopen_tmpfile_linkable_at()
success, label_ops_post() was called but its argument was heavily
conditionalized. Let's call it both on failure and success cases
with specific arguments. This should be easy to read.
This also
- moves variable declarations where used,
- drops unnecessary boolean flag call_label_ops_post.
No functional change. Just reafactoring.
Hopefully silence CID#1664328, though it is false-positive.
This is useful when writing coredump pattern. But the logic itself is
quite generic. Let's add the helper in our basic library.
Preparation for later change.
This PR changes some SELinux bits related to working with alternate
roots (specifically when using `--root` or `--image` on a bunch of
executables).
It addresses bug #42643 and it's hopefully the more whole approach than
the naive approach I PR'ed in #42644.
Before this PR the 'wrong' labels get applied because the path lookups
in the SELinux label database are prefixed with whatever the location of
the alternate root is (explained in more detail below).
Initially I had taken a very naive approach that did fix the issue by
stripping the alternate root from the path; however this still looks up
that path in the hosts' label database, which might differ from the one
contained in the alternate root.
So this expanded approach actually reads the label database from the
alternate root, strips the prefix *if* an alternate root is used
directly in `selinux-util.c` and then uses that to assign labels
instead.
See under the line for the behavior pre/post.
I've tried builds of this PR on both enforcing/non-enforcing/non-enabled
hosts *and* on enabled/non-enabled disk images and things seem to work
or at least fall back to ignoring MAC when required bits aren't present.
One thing is *if* an `/etc/selinux/config` is present that defines a
`SELINUXTYPE=` we *do* require the policy given to be present in the
image. This is the only new actual error in this code path that doesn't
get ignored.
We *could* verify that the path exists and also ignore it but I
personally don't think that's the right approach since the actual system
itself would likely also be broken anyhow. Let me know thoughts on that.
There's a tight coupling here still with the *hosts* SELinux policy in
that to set (potentially) unknown labels to the policy loaded in the
host kernel these executables would need to execute in a domain that
allows transitioning to `mac_admin`. I'd say that `install_t` is the
most likely candidate for that. See the first comment on this PR for
more explanation on it/request for input.
---
When mounting `a.raw` before running any tooling against it and showing
the `/etc/shadow` file labels we have:
```
€ sudo systemd-dissect --mount test/a.raw test/mnt/a
€ ls -Zlart test/mnt/a/etc/shadow
----------. 1 root root system_u:object_r:shadow_t:s0 520 Jun 27 07:55 test/mnt/a/etc/shadow
€ sudo systemd-dissect ---umount test/mnt/a
```
After running `systemd-firstboot` against the image, then remounting,
note the labels that have been changed to incorrect values:
```
€ sudo systemd-firstboot --image test/a.raw --root-password test
/home/user/src/github.com/teamsbc/artifacts/test/a.raw: /etc/passwd written.
/home/user/src/github.com/teamsbc/artifacts/test/a.raw: /etc/shadow written.
€ sudo systemd-dissect --mount test/a.raw test/mnt/a
€ ls -Zlart test/mnt/a/etc/shadow
----------. 1 root root system_u:object_r:init_var_run_t:s0 579 Jun 27 08:01 test/mnt/a/etc/shadow
```
The behavior before this PR looks up the labels in the label database of
the host, but the path that gets looked up is the path where the image
is temporarily mounted, or in the case of `--root` where the root is on
the host. Since that path doesn't define any labels we get the labels of
the location where the file was created on the host. In this case since
`--image` was used, which mounted things in a temporary location we end
up with `var_run_t`.
If this image is booted things that want to read `/etc/shadow` might not
be allowed to read files labeled this way; thus services fail to start,
and root can't login when SELinux is in enforcing mode.
After this PR is applied there are two main differences in how things
are handled. The first being that instead of reading the label database
from the host (which might have none, or have a different one from the
one contained inside an image or root) we read the label database from
inside the alternate root. This tries to make sure we get the correct
labels for given paths.
Second, and most importantly, if we did init SELinux with an alternate
root then any paths passed to the relevant label lookup functions strip
that alternate root from the path. While previously we'd look up a path
like `/run/dissect-XXXX/etc/shadow` we now look up a path like
`/etc/shadow` *and* this path gets looked up in the label database in
the alternate root.
Together these things give in my opinion better handling of SELinux in
alternate roots. To confirm things work here's the same operations on
the second copy of our image:
```
€ sudo systemd-dissect --mount test/b.raw test/mnt/b
artifacts € ls -Zlart test/mnt/b/etc/shadow
----------. 1 root root system_u:object_r:shadow_t:s0 520 Jun 27 07:55 test/mnt/b/etc/shadow
€ sudo ~/src/github.com/systemd/systemd/build/systemd-firstboot --image test/b.raw --root-password test
/home/user/src/github.com/teamsbc/artifacts/test/b.raw: /etc/passwd written.
/home/user/src/github.com/teamsbc/artifacts/test/b.raw: /etc/shadow written.
€ sudo systemd-dissect --mount test/b.raw test/mnt/b
€ ls -Zlart test/mnt/b/etc/shadow
----------. 1 root root system_u:object_r:shadow_t:s0 579 Jun 27 08:44 test/mnt/b/etc/shadow
```
Showing that we now have the correct labels applied.
The backwards scan started at offset length-3, so the last position at which a
sequence can begin, length-2, was never examined. CSI sequences are at least
three bytes long and were thus unaffected, but two byte Fe sequences (ESC
followed by 0x40…0x5F) terminating the examined slice were missed.
ellipsize_mem() calls this to figure out whether a sequence ends exactly at the
current position, so that it can be skipped over, which is precisely the case
that was broken: such a sequence was instead counted as two visible cells and
copied through as text, and the string was ellipsized more aggressively than
requested. For example ellipsize("🐱🐱\x1bM🐱🐱\x1bM", 5, 0) returned a three
cell wide string rather than the five cells asked for.
SELinux labeling with --root/--image needs per-call context to carry an
alternate policy database and root prefix. The current LabelOps interface
has no way to pass this, forcing any solution to rely on global state.
Add a LabelContext *label_context to the label_ops callbacks and all
intermediate layers. Generic filesystem functions that most callers use
(xopenat_full, write_string_file_full, etc.) get a _label variant
carrying the extra parameter, with inline wrappers preserving the
original signatures so the vast majority of call sites remain untouched.
Signed-off-by: Simon de Vlieger <cmdr@supakeen.com>
Turns out that clang's interprocedural analysis is quite smart and can
look through our expand_to_usable() trick, which then causes crashes
with _FORTIFY_SOURCE=3.
clang's interprocedural analysis can see that expand_to_usable() simply
returns its first argument, so it replaces all uses of the return value
with that argument (the original realloc() result). This effectively
bypasses expand_to_usable()'s alloc_size attribute, causing the fortify
check to use the (smaller) size from realloc() instead, which eventually
leads to a false-positive buffer overflow:
$ build/test-varlink-idl
/* test_parse_format */
...
*** buffer overflow detected ***: terminated
Aborted (core dumped) build/test-varlink-idl
This is not an issue with gcc (at least not yet), since gcc sees
expand_to_usable() as an opaque user-defined allocation-like function
and simply trusts the alloc_size attribute that comes with it.
To fix this, let's add a simple no-op barrier to malloc_sizeof_safe()
that clobbers the input pointer, which causes
__builtin_dynamic_object_size() to return (size_t)-1 - this is
interpreted as an "unknown" size by the following fortify check which is
then skipped instead of triggering the assertion.
Similarly, test-alloc-util now doesn't call malloc_usable_size()
directly but instead goes through malloc_sizeof_safe(), so it's also
guarded by the barrier.
This follows the already established pile of similar workaround for the
same class of issues we encountered with gcc, namely [0], which prompted
[1], that was later reverted in [2], and then followed by another couple
of fixes in [3] and [4].
Resolves: #43178
[0] https://github.com/systemd/systemd/issues/22801
[1] https://github.com/systemd/systemd/commit/0bd292567a543d124cd303f7dd61169a209cae64
[2] https://github.com/systemd/systemd/commit/2cfb790391958ada34284290af1f9ab863a515c7
[3] https://github.com/systemd/systemd/commit/7929e180aa47a2692ad4f053afac2857d7198758
[4] https://github.com/systemd/systemd/commit/4f79f545b3c46c358666c9f5f2b384fe50aac4b4
switch_root() calls a blanket sync() before detaching the old root
file system, in order to make sure it is in a good state before it
becomes unreachable via MNT_DETACH/pivot_root().
A global sync() however flushes out *every* mounted file system on
the system, not just the ones we are actually about to detach. On
real-world systems that commonly have several additional mounted file
systems (separate /home, /var, additional data partitions, network
shares, removable media, ...) this needlessly delays switch_root() with
completely unrelated I/O. This matters in particular for
initrd-switch-root.service, which runs this code on the critical path
of pretty much every single boot with an initrd, and for soft-reboot.
Replace the global sync() with a new sync_departing_file_systems()
helper that walks /proc/self/mountinfo and calls syncfs() on every
file system except:
- 'new_root' and anything mounted below it: these remain mounted
and reachable after the transition and keep being synced normally
as part of their regular life cycle, so they don't need to be
force-flushed here.
- API/pseudo file systems (proc, sysfs, cgroupfs, autofs, ...),
network file systems, and overlayfs (which has no backing store
of its own), as determined by the new fstype_is_worth_syncing()
predicate. There is nothing meaningful to flush on any of these,
and more importantly, opening an untriggered autofs mount point
would needlessly trigger it, and opening a stale network mount
could block for a long time - exactly what we are trying to avoid
on this code path.
- Any flavour of FUSE (plain 'fuse', 'fuseblk', or a
'fuse.<subtype>', e.g. sshfs, rclone, gvfs, ntfs-3g, exfat-fuse,
...), classified via the new fstype_is_fuse() predicate in
src/basic/mountpoint-util.c, plus a few other, non-FUSE guest/host
file sharing file systems with the same "backed by a companion
daemon/hypervisor that could be wedged" risk profile (virtiofs,
vboxsf, vmhgfs). All I/O against any of these, including the
syncfs() we'd otherwise issue, is routed through an arbitrary
userspace daemon (or, for virtiofs/vboxsf/vmhgfs, the host/
hypervisor side), which could hang indefinitely if wedged, dead,
or otherwise unresponsive - there's no timeout on this code path.
'fuseblk' might sound exempt given the name, and does wrap an
actual block device, but that doesn't bound its syncfs() latency
by the kernel block layer alone the way a native block device
file system's is: the request is still serviced by the same FUSE
daemon as any other FUSE variant, and can hang exactly the same
way, so it is excluded here too, trading its comparatively minor
data-safety benefit for avoiding that unbounded hang risk.
'9p' (which can be used with a writeback cache and hence carry
real dirty data, e.g. common in QEMU/KVM guests) and the
shared-storage cluster file systems 'gfs', 'gfs2' and 'ocfs2'
(which fstype_is_network() also happens to classify as "network"
file systems, since they additionally rely on a networked
distributed lock manager for coordination) are deliberately *not*
excluded: unlike FUSE/virtiofs/etc., these are serviced by a
mature, in-kernel client (talking directly to the hypervisor over
a bounded virtio transport, or to real - if shared - block
storage), not an arbitrary, potentially wedged userspace daemon,
so they carry the same bounded, local sync latency any other
block device backed file system already does here. Skipping them
would needlessly sacrifice the data-safety guarantee the original
blanket sync() gave them, without meaningfully improving safety.
- Mount table entries that we can positively confirm are currently
shadowed by another mount stacked on top of them at the same
path: since we can only reach a file system by (re-)opening its
target path, and that always resolves to whatever is currently on
top, syncing by path alone could end up flushing the wrong
superblock. Detect this via the new shared
libmount_fs_id_matches_path() helper (factored out of, and now
also used by, the pre-existing get_sub_mounts(), which needed the
exact same check for the same reason). This same check is also
applied to a mountinfo entry whose target is 'new_root' itself
(not just anything strictly below it): comparing its mount ID
against new_root's own, freshly determined mount ID tells apart
the file system that is actually still reachable there (which we
continue to skip) from a stale entry that merely shares the exact
same path (e.g. if new_root wasn't already its own mount point
and got bind-mounted onto itself earlier in switch_root()), which
is departing just the same and must not be skipped just because
of that coincidence.
Every failure mode that means we can no longer be sure we've covered
every departing file system correctly - libmount being unavailable,
/proc/self/mountinfo (or a specific entry in it) failing to parse,
being unable to tell whether a specific entry is currently shadowed,
or syncfs_path() itself failing for an otherwise-eligible entry - is
handled the exact same way: propagate the error up and let the sole
caller, switch_root(), fall back to one plain, global sync() to cover
everything, rather than deciding on and performing that fallback (or,
worse, silently skipping the affected file system without any
fallback at all) at each of these different spots individually. This
should be rare in practice, so it doesn't meaningfully undercut the
benefit of the targeted sync in the common case.
Everything else that's actually about to become unreachable (the old
root itself, but also any other, unrelated real file system that
happens to be mounted underneath it and gets detached along with it)
is still synced, so this keeps the same safety guarantee the original
blanket sync() gave for file systems that actually do go away here.
Uses the existing syncfs_path() helper for the actual open+syncfs.
sync_departing_file_systems() itself returns -EOPNOTSUPP if libmount
support isn't compiled in, handled the same way by switch_root() as
any of its other error returns.
Note we intentionally don't use O_PATH file descriptors here: syncfs()
requires a 'real' file descriptor and fails with EBADF on O_PATH ones.
Also note there remains an inherent, narrow TOCTOU race between the
mount-ID check described above and the open() syncfs_path() performs
right after it: if something else mounts something new on top of a
given 'path' in between, that open() could still end up triggering an
automount, or hanging on a stale mount, since there is no open()/
openat() equivalent of statx()'s AT_NO_AUTOMOUNT to prevent this for a
"real" (non-O_PATH) file descriptor. Unlike the other failure modes
handled here, a hanging open() can't be recovered from by falling back
to sync() afterwards, since control never returns to do so. Closing
this fully would require disproportionate effort (e.g. performing the
open() in a separate, killable/timeout-bounded process) for a window
that is already narrow, since this code only runs with most other
activity on the system already quiesced during the switch_root()
transition itself, so it is accepted as-is (see the comment at the
call site for details).
This mirrors the same reasoning already applied to the shutdown path
in src/shutdown/shutdown.c, which deliberately avoids a 'dumb' sync()
there for identical reasons.
Definitions enumerated under --root= are already rooted. Passing those
paths to the config parsers with the same root prefixes the root again,
so feature and transfer files are parsed from the wrong path.
Repro: create root/etc/sysupdate.d/rootfeat.feature and
01-root.transfer, then run:
build/systemd-sysupdate --root="$root" --verify=no --offline features rootfeat
Before: parsing failed at line 1 with a bogus Source Type= error.
Fixes#42783.
Follow-up for: e1384cfb09
The scan pointer was advanced with utf8_next_char(), which blindly skips
utf8_skip_data[lead byte] bytes, so a line ending in a truncated multibyte
sequence like "foo\xF0" stepped over the NUL and the loop read past the end
of the string. Decode each character with utf8_encoded_to_unichar() and
propagate the error instead of measuring broken characters.
Fixes#43052.
A `\u` escape in `cunescape_one()` was accepted for any 16-bit value
except NUL. That range `0x0000`-`0xFFFF` includes the UTF-16 surrogates
`0xD800`-`0xDFFF`, which are not valid Unicode scalar values and cannot
be encoded as valid UTF-8.
Nowadays, varlink is used to control systemd-udevd. Let's drop the
legacy socket.
Note, the existence of /run/udev/control socket is widely used in both
our code and external projects. Also, the dependency to
systemd-udevd-control.socket is widely used in many projects. Hence, we
need to create a symlink to the socket file and .socket unit file.
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
wyu
Armaan Sandhu
zhaoshuang
Chris Down
Simon de Vlieger
Frantisek Sumsal
Eric Curtin
Mike Yuan
dongshengyuan
Armaan Sandhu
Luca Boccassi