TEST-74-AUX-UTILS has a number of subtests.
test/units/TEST-74-AUX-UTILS.capsule.sh runs first and starts and stops
capsule@foobar.service. Looking at the test, the unit is cleanly stopped.
But later test/units/TEST-74-AUX-UTILS.machine-id-setup.sh tests for
failed units. capsule@foobar.service is listed as failed, causing the
second subtest to fail.
Add the same test in test/units/TEST-74-AUX-UTILS.capsule.sh to see
if the failed test really originates from there.
When systemd-cryptenroll --tpm2-device=auto is called on a system where
a tpm2-pcr-public-key.pem exists it automatically creates tokens with a
signed PCR policy. Unlocking such a token via --unlock-tpm2-device=auto
requires a tpm2-pcr-signature.json file, which is not present.
This creates a race with systemd-tpm2-setup.service at boot: if the
service completes before the test, the key exists and the subsequent
--unlock-tpm2-device=auto calls fail, which I believe is the cause of
the test flakiness.
This also seems to mesh with the fact that this only flakes on Debian
CI, since that's built with ukify which installs a public key.
Let's hopefully fix this by passing --tpm2-public-key= to all
--tpm2-device= enrollment calls that aren't explicitly intended to test
signed PCR policy behaviour.
When systemd-cryptenroll --tpm2-device=auto is called on a system where
a tpm2-pcr-public-key.pem exists it automatically creates tokens with a
signed PCR policy. Unlocking such a token via --unlock-tpm2-device=auto
requires a tpm2-pcr-signature.json file, which is not present.
This creates a race with systemd-tpm2-setup.service at boot: if the
service completes before the test, the key exists and the subsequent
--unlock-tpm2-device=auto calls fail, which I believe is the cause of
the test flakiness.
This also seems to mesh with the fact that this only flakes on Debian
CI, since that's built with ukify which installs a public key.
Let's hopefully fix this by passing --tpm2-public-key= to all
--tpm2-device= enrollment calls that aren't explicitly intended to test
signed PCR policy behaviour.
Verify that GPT images with an ISO9660 El Torito boot catalog are
dissected via the GPT partition table rather than being treated as a
single iso9660 filesystem.
Follow-up for e33eb053fb ("dissect-image: Drop
blkid_probe_filter_superblocks_usage() call from probe_blkid_filter()")
During daemon-reload (or daemon-reexec), when a unit becomes an alias to
another unit, deserialising the alias's stale serialised state can
corrupt the canonical unit's live runtime state.
Consider this scenario:
1. Before reload:
- a.service is running
- b.service was stopped earlier and is dead
- Both exist as independent units
2. User creates an alias to migrate from b -> a:
- `ln -s /run/systemd/system/a.service /etc/systemd/system/b.service`
3. daemon-reload triggers serialisation. State file contains both units:
- a.service -> state=running, cgroup=/system.slice/a.service, PID=1234,
...
- b.service -> state=dead, cgroup=(empty), no PIDs, ...
4. During deserialisation:
- Processes a.service: loads Unit A, deserialises -> state=RUNNING
- Processes b.service: manager_load_unit() detects symlink, returns Unit
A
- unit_deserialize_state(Unit A, ...) overwrites with b's dead state
5. The result is that:
- Unit A incorrectly shows state=dead despite PID 1234 still running
- If a.service has Upholds= dependents, catch-up logic sees a.service
should be running but is dead
- systemd starts a.service again -> PID 5678
- Two instances run: PID 1234 (left-over) and PID 5678 (new)
This bug is deterministic when serialisation orders a.service before
b.service.
The root cause is that manager_deserialize_one_unit() calls
manager_load_unit(name, &u) which resolves aliases via
unit_follow_merge(), returning the canonical Unit object. However, the
code doesn't distinguish between two cases when u->id differs from the
requested name from the state file. In the corruption case, we're
deserialising an alias entry and unit_deserialize_state() blindly
overwrites the canonical unit's fields with stale data from the old,
independent unit. The serialised b.service then overwrites Unit A's
correct live state.
This commit first scans the serialised unit names, then adds a check
after manager_load_unit():
if (!streq(u->id, name) && set_contains(serialized_units, u->id))
...
This detects when the loaded unit's canonical ID (u->id) differs from
the serialised name, indicating the name is now an alias for a different
unit and the canonical unit also has its own serialised state entry.
If the canonical unit does not have its own serialised state entry, we
keep the state entry. That handles cases where the old name is really
just a rename, and thus the old name is the only serialised state for
the unit. In that case there is no bug, because there is no separate
canonical state entry for the stale alias entry to overwrite.
Skipping is safe because:
1. The canonical unit's own state entry will be correctly deserialised
regardless of order. This fix only prevents other stale alias entries
from corrupting it.
2. unit_merge() has already transferred the necessary data. When
b.service became an alias during unit loading, unit_merge() already
migrated dependencies and references to the canonical unit.
3. After merging, the alias doesn't have its own runtime state. The
serialised data represents b.service when it was independent, which is
now obsolete once the canonical unit also has its own serialised entry.
4. All fields are stale. unit_deserialize_state() would overwrite state,
timestamps, cgroup paths, pids, etc. There's no scenario where we want
this data applied on top of the canonical unit's own serialised state.
This fix also correctly handles unit precedence. For example, imagine
this scenario:
1. `b.service` is a valid, running unit defined in `/run`.
2. The sysadmin creates `ln -s .../a.service /etc/.../b.service`.
3. On reload, the new symlink in `/etc` overrides the unit in `/run`.
The new perspective from the manager side is that `b.service` is now an
alias for `a.service`.
In this case, systemd correctly abandons the old b.service unit, because
that's the intended general semantics of unit file precedence. We also
do that in other cases, like when a unit file in /etc/systemd/system/
masks a vendor-supplied unit file in /lib/systemd/system/, or when an
admin uses systemctl mask to explicitly disable a unit.
In all these scenarios, the configuration with the highest precedence
(in /etc/) is treated as the new source of truth. The old unit's
definition is discarded, and its running processes are (correctly)
abandoned. In that respect we are not doing anything new here.
Some may ask why we shouldn't just ignore the symlink if we think this
case will come up. I think there are multiple very strong reasons not to
do so:
1. It violates unit precedence. The unit design is built on a strict
precedence list. When an admin puts any file in /etc, they are
intentionally overriding everything else. If manager_load_unit were to
"ignore" this file based on runtime state, it would break this
fundamental precedent.
2. It makes daemon-reload stateful. daemon-reload is supposed to be a
simple, stateless operation, basically to read the files on disk and
apply the new configuration. But doing this would make daemon-reload
stateful, because we'd have to read the files on disk, but
cross-reference the current runtime state, and... maybe ignore some
files. This is complex and unpredictable.
3. It also completely ignores the user intent. The admin clearly has
tried to replace the old service with an alias. Ignoring their
instruction is the opposite of what they want.
Fixes: https://github.com/systemd/systemd/issues/38817
Fixes: https://github.com/systemd/systemd/issues/37482
While we're at it, rename to test-loop-util.c so it matches its
source file.
We also drop the root check and solely check for CAP_SYS_ADMIN since
that's sufficient to run the tests.
This is a rebased version of #40936:
- the first few commits are #41003, which is greenlighted for merging
after the release
- then there's #40923
- and some changes on top
During daemon-reload (or daemon-reexec), when a unit becomes an alias to
another unit, deserialising the alias's stale serialised state can
corrupt the canonical unit's live runtime state.
Consider this scenario:
1. Before reload:
- a.service is running
- b.service was stopped earlier and is dead
- Both exist as independent units
2. User creates an alias to migrate from b -> a:
- `ln -s /run/systemd/system/a.service /etc/systemd/system/b.service`
3. daemon-reload triggers serialisation. State file contains both units:
- a.service -> state=running, cgroup=/system.slice/a.service,
PID=1234, ...
- b.service -> state=dead, cgroup=(empty), no PIDs, ...
4. During deserialisation:
- Processes a.service: loads Unit A, deserialises -> state=RUNNING
- Processes b.service: manager_load_unit() detects symlink, returns
Unit A
- unit_deserialize_state(Unit A, ...) overwrites with b's dead state
5. The result is that:
- Unit A incorrectly shows state=dead despite PID 1234 still running
- If a.service has Upholds= dependents, catch-up logic sees
a.service should be running but is dead
- systemd starts a.service again -> PID 5678
- Two instances run: PID 1234 (left-over) and PID 5678 (new)
This bug is deterministic when serialisation orders a.service before
b.service.
The root cause is that manager_deserialize_one_unit() calls
manager_load_unit(name, &u) which resolves aliases via
unit_follow_merge(), returning the canonical Unit object. However, the
code doesn't distinguish between two cases when u->id differs from the
requested name from the state file. In the corruption case, we're
deserialising an alias entry and unit_deserialize_state() blindly
overwrites the canonical unit's fields with stale data from the old,
independent unit. The serialised b.service then overwrites Unit A's
correct live state.
This commit first scans the serialised unit names, then adds a check
after manager_load_unit():
if (!streq(u->id, name) && set_contains(serialized_units, u->id))
...
This detects when the loaded unit's canonical ID (u->id) differs from
the serialised name, indicating the name is now an alias for a different
unit and the canonical unit also has its own serialised state entry.
If the canonical unit does not have its own serialised state entry, we
keep the state entry. That handles cases where the old name is really
just a rename, and thus the old name is the only serialised state for
the unit. In that case there is no bug, because there is no separate
canonical state entry for the stale alias entry to overwrite.
Skipping is safe because:
1. The canonical unit's own state entry will be correctly deserialised
regardless of order. This fix only prevents other stale alias entries
from corrupting it.
2. unit_merge() has already transferred the necessary data. When
b.service became an alias during unit loading, unit_merge() already
migrated dependencies and references to the canonical unit.
3. After merging, the alias doesn't have its own runtime state. The
serialised data represents b.service when it was independent, which
is now obsolete once the canonical unit also has its own serialised
entry.
4. All fields are stale. unit_deserialize_state() would overwrite state,
timestamps, cgroup paths, pids, etc. There's no scenario where we
want this data applied on top of the canonical unit's own serialised
state.
This fix also correctly handles unit precedence. For example, imagine
this scenario:
1. `b.service` is a valid, running unit defined in `/run`.
2. The sysadmin creates `ln -s .../a.service /etc/.../b.service`.
3. On reload, the new symlink in `/etc` overrides the unit in `/run`.
The new perspective from the manager side is that `b.service` is now an
alias for `a.service`.
In this case, systemd correctly abandons the old b.service unit, because
that's the intended general semantics of unit file precedence. We also
do that in other cases, like when a unit file in /etc/systemd/system/
masks a vendor-supplied unit file in /lib/systemd/system/, or when an
admin uses systemctl mask to explicitly disable a unit.
In all these scenarios, the configuration with the highest precedence
(in /etc/) is treated as the new source of truth. The old unit's
definition is discarded, and its running processes are (correctly)
abandoned. In that respect we are not doing anything new here.
Some may ask why we shouldn't just ignore the symlink if we think this
case will come up. I think there are multiple very strong reasons not to
do so:
1. It violates unit precedence. The unit design is built on a strict
precedence list. When an admin puts any file in /etc, they are
intentionally overriding everything else. If manager_load_unit were
to "ignore" this file based on runtime state, it would break this
fundamental precedent.
2. It makes daemon-reload stateful. daemon-reload is supposed to be a
simple, stateless operation, basically to read the files on disk and
apply the new configuration. But doing this would make daemon-reload
stateful, because we'd have to read the files on disk, but
cross-reference the current runtime state, and... maybe ignore some
files. This is complex and unpredictable.
3. It also completely ignores the user intent. The admin clearly has
tried to replace the old service with an alias. Ignoring their
instruction is the opposite of what they want.
Fixes: https://github.com/systemd/systemd/issues/38817
Fixes: https://github.com/systemd/systemd/issues/37482
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!
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.
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.
`updatectl enable --now` systematically fails because the update phase find
resources that are not pending. So instead we should ignore them.
Fixes#41254
AllowDiscards= will set allow-discards in the persistent flags which will make
activating the device automatically activate with that option. This is
useful for devices discovered through gpt-auto-generator without
needing to use some kernel command line to set the option.
Ubuntu is doing shenanigans with their coreutils so they are now
symlinks instead of binaries, so the grep fails. Check bash instead
to fix test failure on 26.04.
This has been requested previously for PCR 7 (#40567), but let's do that
for all firmware owned PCRs, since some firmwares forget to measure
their own separator. Let's hence measure our own guranteed one.
Fixes: #40567
Before this commit, the > 0 state of arg_newline tristate is
simply ignored.
Yes, this is a minor compat break, but I'd argue the previous
behavior was not useful as "yes" is treated the same as "auto".
An issue also reported that it was quite surprising.
Fixes#41348
Commit 0e1ede4b4b introduced a bug where
we'd always fetch the "original" (empty) list of groups when processing
a comma-separated list of groups from the --member-of= option, so only
the last group from the list would get applied. This bug was then later
(in 316e9887f2) refactored into a separate
function.
Follow-up for 0e1ede4b4b.
Resolves: #41286
This is an extension of the /etc/hosts concept, but can provide any kind
of RRs (well, actually, we only parse A/AAAA/PTR for now, but the
concept is open for more).
Fixes: #17791
With the planned extraction of the socket-forward code its useful
to have a basic way to validate the functionality. So add a basic
test that ensures at least base functionality is intact.
Add %I (TID in initial PID namespace) to the core_pattern, so the
kernel passes the crashing thread's TID to systemd-coredump. Use it
to read the thread's comm name from /proc/<tid>/comm and log both as
new journal fields:
COREDUMP_TID= — TID of the crashing thread
COREDUMP_THREAD_NAME= — comm name of the crashing thread
These fields are also stored as xattrs on external coredump files
(user.coredump.tid, user.coredump.thread_name) and displayed by
coredumpctl info alongside the PID line.
For single-threaded processes the TID equals the PID and thread_name
equals comm; for multi-threaded programs with named worker threads
(pthread_setname_np / PR_SET_NAME) this identifies which thread
crashed without needing to open the coredump file itself.
The new fields are optional in the socket forwarding path, so older
systemd-coredump senders are handled gracefully.
Co-developed-by: Claude <claude@anthropic.com>
--no-gpgchecks was introduced in 9204838724 but is only available in
dnf5. Use --nogpgcheck instead, which is supported by both dnf4 and
dnf5 (where it is an alias for --no-gpgchecks).
Fixes test failure on distros still using dnf4 (e.g. CentOS/RHEL 9).
Co-developed-by: Claude <claude@anthropic.com>
We have two places where those fields can be set: config and the dbus
interface. Let's clamp down on the number in both places. But in
principle, we could also be upgrading (through
serialization/deserialization) from an older systemd which didn't
enforce this limit, so also check on deserialization. A user could have
a unit with lots and lots of ExtraFields, but not enough to cause the
issue in #40916. To handle this gracefully, ignore the extra fields,
like we do in the parser. Where the field is used, assert that we are
within the expected bounds.
Fixes#40916.
Reproducer:
```console
$ python3 -c 'from pydbus import SystemBus; from gi.repository import GLib; SystemBus().get("org.freedesktop.systemd1", "/org/freedesktop/systemd1").StartTransientUnit("crash.service", "fail", [("ExecStart", GLib.Variant("a(sasb)", [("/bin/true", ["/bin/true"], False)])), ("LogExtraFields", GLib.Variant("aay", [b"F%d=x" % i for i in range(140000)]))], [])' Traceback (most recent call last):
File "<string>", line 1, in <module>
from pydbus import SystemBus; from gi.repository import GLib; SystemBus().get("org.freedesktop.systemd1", "/org/freedesktop/systemd1").StartTransientUnit("crash.service", "fail", [("ExecStart", GLib.Variant("a(sasb)", [("/bin/true", ["/bin/true"], False)])), ("LogExtraFields", GLib.Variant("aay", [b"F%d=x" % i for i in range(140000)]))], [])
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.14/site-packages/pydbus/proxy_method.py", line 102, in __call__
raise error
File "/usr/lib/python3.14/site-packages/pydbus/proxy_method.py", line 97, in __call__
result = instance._bus.con.call_sync(*call_args)
gi.repository.GLib.GError: g-dbus-error-quark: GDBus.Error:org.freedesktop.DBus.Error.InvalidArgs: Too many extra log fields. (16)
```
[] has higher precedence than pointer dereference, hence hilarity
ensues as soon as there are multuple images
Originally reported on yeswehack.com as:
YWH-PGM9780-122
Follow-up for dfdeb0b1cb
bus_cgroup_set_tasks_max_scale() used a hand-rolled percentage format
that produced values ~10x too small (e.g., "TasksMax=4.0%" instead of
"TasksMax=40.00%").
On daemon-reload, the incorrect value was re-read, silently reducing
the effective TasksMax by ~10x and causing fork rejections on systems
with high thread counts.
Fix by using the existing PERMYRIAD macros, consistent with memory
property handlers (MemoryMax, MemoryHigh, MemoryLow, etc.).
Fixes: #41009
1 and 10000 are valid cgroup weight values, but the condition was
incorrectly excluding them:
$ echo '{"userName":"crashhostarray","cpuWeight":1}' | userdbctl -F -
<stdin>:1:42: JSON field 'cpuWeight' is not in valid range 1…10000.
$ echo '{"userName":"crashhostarray","cpuWeight":10000}' | userdbctl -F -
<stdin>:1:42: JSON field 'cpuWeight' is not in valid range 1…10000.
Otherwise we might try to register the same scope again before the
previous instance gets a chance to be cleaned up:
[ 54.378392] systemd-nspawn[2554]: ░ Spawning container TEST-13-NSPAWN.defaultinaccessiblepaths.nxs on /var/lib/machines/TEST-13-NSPAWN.default_inaccessible_paths.nxs.
[ 54.382202] systemd-nspawn[2554]: Failed to allocate scope: Unit TEST-13-NSPAWN.defaultinaccessiblepaths.nxs.scope was already loaded or has a fragment file.
[ 54.411211] systemd[1]: TEST-13-NSPAWN.service: Main process exited, code=exited, status=1/FAILURE
[ 54.411413] systemd[1]: TEST-13-NSPAWN.service: Failed with result 'exit-code'.
[ 54.411885] systemd[1]: Failed to start TEST-13-NSPAWN.service - TEST-13-NSPAWN.
This is basically the same change as in
6a05abb9b4 but for the newly added tests.
Follow-up for 83b8daa032.
Resolves: #40945
The diff seems big, but it's just removing and if and deleting the
indentation. With `git show -w` you can see clearly that is just one
line. In the files tab, in github, you can click "hide whitespaces" and
it is clear too.
I don't know if there is some better way, but when using a portable
service with an extension image that just overlays files I need to
manually add the `ExtensionImages=`. And that is added automatically for
other extension images that provide a service unit.
So, this patch just makes sure we add the `ExtensionImages=` for any
extension.
Any thoughts?
Below the commit msg for more details:
---
Before this patch, when running:
portablectl attach --extension ext.raw ./base.raw
No drop-in is added for the "ExtensionImages" if there aren't units from
the extension loaded.
But the extension can just overlay files, as in my case. So before this
patch, I also need to manually add a drop-in with "ExtensionImages=" for
it to really be loaded.
Let's just always add the drop-in for extensions. This way, it works for
extensions that just overlay files too.
Please note this commit just removes the if (simpler to view the diff
with git show -w). Also, the if checked for m->image_path being not
NULL, but removing it shouldn't cause a NULL pointer dereference.
Because
m->image_path is not used inside the if (it was needed just for the if
itself) and image_path is asserted at the beginning of the function to
be non-NULL too.
This was like this since the beginning of time in 907952bbc9
("portabled: add --extension parameter for layered images support")
/run/systemd/inaccessible/ exists only on host - in the container we have
/run/host/inaccessible/, and since all the inaccessible mounts have
MOUNT_IN_USERNS we need to use the latter one, otherwise the masking
gets silently skipped:
~# SYSTEMD_LOG_LEVEL=debug systemd-nspawn -q --directory=foo ls -la /proc/kallsyms
...
Bind-mounting /run/systemd/inaccessible/reg on /proc/kallsyms (MS_BIND "")...
Failed to mount /run/systemd/inaccessible/reg (type n/a) on /proc/kallsyms (MS_BIND ""): No such file or directory
Changing mount flags /proc/kallsyms (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_REMOUNT|MS_BIND "")...
Failed to mount n/a (type n/a) on /proc/kallsyms (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_REMOUNT|MS_BIND ""): Invalid argument
Bind-mounting /run/systemd/inaccessible/reg on /proc/kcore (MS_BIND "")...
Failed to mount /run/systemd/inaccessible/reg (type n/a) on /proc/kcore (MS_BIND ""): No such file or directory
Changing mount flags /proc/kcore (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_REMOUNT|MS_BIND "")...
Failed to mount n/a (type n/a) on /proc/kcore (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_REMOUNT|MS_BIND ""): Invalid argument
...
Inner child finished, invoking payload.
-r--r--r--. 1 root root 0 Feb 25 13:19 /proc/kallsyms
Whenever delegating UID ranges to a user namespace, it can also be
useful to map the foreign UID range, so that the container running in
the user namespace with delegated UID ranges can download container
images and unpack them to the foreign UID range.
Let's add an option mapForeign to make this possible. Note that this option
gives unprivileged users full access to the any foreign UID range owned directory
that they can access. Hence it is recommended (and already was recommended) to
store foreign UID range owned directories in a 0700 directory owned by the
owner of the tree to avoid access and modifications by other users.
This is already the case for the main users of the foreign UID range,
namely /var/lib/machines, /var/lib/portables and /home/<user> which all
use 0700 as their mode.
Users will also be able to create foreign UID range owned inodes in any
directories their own user can write to (on most systems this means /tmp,
/var/tmp and /home/<user>).
Zbigniew Jędrzejewski-Szmek
Christian Brauner
Chris Down
Daan De Meyer
Lennart Poettering
Michael Vogt
Yaping Li
Valentin David
Luca Boccassi
Mike Yuan
Frantisek Sumsal
seaeunlee
noxiouz
Ivan Kruglov
Rodrigo Campos
Cyrus Xi
Daan De Meyer