I wanted to see if moving moving out constant string arguments our of
error messages results in smaller binary sizes. Turns out it does, but
the savings are not consistent. Sometimes we get a few kB in a single
binary, sometimes there is no size change.
Adrian Vovk [Wed, 21 May 2025 21:32:03 +0000 (17:32 -0400)]
Define uid range for greeter
In multi-seat scenarios, a display manager might need to start multiple
greeter sessions. But systemd allows at most one graphical session per
user. So, display managers now have a range of UIDs to dynamically
allocate users for their greeter sessions.
various: do not include file names directly in error messages
git grep -l 'Failed to open /'|xargs sed -r -i 's|"Failed to open (/[^ ]+): %m"|"Failed to open %s: %m", "\1"|g'
git grep -l $'Failed to open \'/'|xargs sed -r -i $'s|"Failed to open \'(/[^ ]+)\': %m"|"Failed to open %s: %m", "\\1"|g'
git grep -l "Failed to open /"|xargs sed -r -i $'s|"Failed to open (/[^ ]+), ignoring: %m"|"Failed to open %s, ignoring: %m", "\\1"|g'
+ some manual fixups.
repart: make CopyBlock=auto work for verity sig partitions
Note that this doesn't care which partition set (A or B in an A/B
scenario) is actually newer, it just picks the first suitable, but
that's something we should look into later. For now, let's just make
verity sig partitions work the same way as verity partitions.
Daan De Meyer [Sun, 1 Jun 2025 18:24:47 +0000 (20:24 +0200)]
repart: Apply verity-sig max size based on partition type
We already do this for partition_min_size(), let's do it for
partition_max_size() as well. This makes sure repart doesn't accidentally
try to grow verity sig partitions to larger sizes than the hardcoded
max size in systemd.
mount-util: avoid unnecessary mount_setattr() call in make_fsmount()
If .attr_set is zero (and .att_clr, .propagation too), then there's no
point in calling mount_setattr().
Fixes: #37062
Note that this optimization is not precisely load-bearing anymore, since 3cc23a2c2345eb188551565349c89ec1fa8f650f got merged which removes the
only caller of make_fsmount() that might trigger it. But it's worth
fixing generic code anyway, in case it gets used like this later again.
core/smack-setup: rework message to include full paths to files
We'd print the filename, but not the full path. Error messages without
the full path are annoying to users since they might not know where the file
is located, esp. if the name is fairly generic, and it is harder to search
for the error message too.
Use a trailing slash to indicate when we're trying to open a directory.
Drop quotes from around paths which are static and known to contain no
whitespace.
sd-device: do not include file name directly in error messages
$ diff -u <(strings -n 10 build/libsystemd.so.0.40.0.0 | sort) <(strings -n 10 build/libsystemd.so.0.40.0 | sort)
--- /proc/self/fd/11 2025-05-31 15:17:16.968761963 +0200
+++ /proc/self/fd/12 2025-05-31 15:17:16.970159823 +0200
@@ -3860,11 +3860,6 @@
Failed to fstat() journal file '%s', ignoring: %m
Failed to fstat %s: %m
Failed to get basic: %m
-Failed to get device "ACTION" property, ignoring: %m
-Failed to get device "DEVNUM" property, ignoring: %m
-Failed to get device "DISKSEQ" property, ignoring: %m
-Failed to get device "IFINDEX" property, ignoring: %m
-Failed to get device "SEQNUM" property, ignoring: %m
Failed to get device "%s" property, ignoring: %m
Failed to get inode number of pidfd for pid %i: %m
Failed to get peer's socket address, ignoring: %m
$ ls -l build/libsystemd.so.0.40.0{,.0}
-rwxr-xr-x 1 zbyszek zbyszek 7631640 May 31 15:16 build/libsystemd.so.0.40.0
-rwxr-xr-x 1 zbyszek zbyszek 7635736 May 31 15:16 build/libsystemd.so.0.40.0.0
$ size build/libsystemd.so.0.40.0{,.0}
text data bss dec hex filename 1241382 56185 3554 1301121 13da81 build/libsystemd.so.0.40.0 1241606 56185 3554 1301345 13db61 build/libsystemd.so.0.40.0.0
So the savings are ~4kB, which is more than I expected.
sleep: do not include file name directly in error messages
Here the .text section becomes smaller, but the file size doesn't change
(I guess some alignment issue). But is seems to be reasonable change to
anyway, we might get some savings in different compilations anyway.
random-seed: do not include file name directly in error messages
The path is fairly long and by embedding it in the message, we get a longer
data section in our binary. By using %s, we can make the template strings
shorter, deduplicating bytes in our binary. Those are error messages, i.e. by
definition they are only used very rarely, so it's completely fine if printf
does a bit more work when generating the message.
$ strings build/systemd-random-seed.0 | rg '/var/lib/systemd/random-seed'
/var/lib/systemd/random-seed
Failed to open /var/lib/systemd/random-seed for writing: %m
Failed to open /var/lib/systemd/random-seed for reading: %m
Failed to open /var/lib/systemd/random-seed: %m
Failed to stat() seed file /var/lib/systemd/random-seed: %m
Failed to read seed from /var/lib/systemd/random-seed: %m
Seed file /var/lib/systemd/random-seed not yet initialized, proceeding.
Yu Watanabe [Sat, 31 May 2025 01:24:17 +0000 (10:24 +0900)]
bootctl: do not print slash more than once
When bootctl is called by an unprivileged user, then previously we got
```
Failed to read "/boot/EFI/systemd": Permission denied
Failed to open '/boot//loader/loader.conf': Permission denied
```
Now, with this patch, we get
```
Failed to read "/boot/EFI/systemd": Permission denied
Failed to open '/boot/loader/loader.conf': Permission denied
```
Inspired by https://github.com/systemd/systemd/pull/37538, see a
detailed rationale in
https://github.com/systemd/systemd/pull/37538#discussion_r2110229075.
This is inspired by #37538, see the discussion in
https://github.com/systemd/systemd/pull/37538#discussion_r2110229075.
If the user already specifies $TERM (which is actually
quite common if you look at run0), we'd needlessly invoke
the "fallback" logic and
a) possibly issue a DCS query whose result we end up simply
discarding in strv_env_merge()
b) set $COLORTERM to "truecolor" unconditionally, whereas
the explicit $TERM value might intend to disable the color output
To address this, the logic of setting fallback $TERM and friends
has been split out of build_environment(), and we'd call into it
only after all envvars have been collected.
Mike Yuan [Thu, 29 May 2025 19:01:01 +0000 (21:01 +0200)]
Use DCS sequence to query terminal name and set $TERM automatically (#37538)
This code seems to work quickly and nicely for a bunch of modern
terminals. Setting $TERM automatically removes an common annoyance for
users. This code will not work for all terminal emulators, but by adding
it in systemd we'll entice maintainers of those terminals to add support
for the sequences. For the terminals that don't support the sequence, we
get a bit of a slowdown of `< 1 ms`, which seems hardly noticeable. The
user can always set TERM explicitly to avoid this if upgrading to a
newer terminal emulator is not possible.
query_term_for_tty() is used in two places: in fixup_environment(),
which affects PID1 itself, and in build_environment(), which affects
spawned services. There is obviously some cost to the extra call,
but I think it's worthwhile to do it. When $TERM is set incorrectly,
basic output works OK, but then there are various annoying corner
cases. In particular, we get the support for color (or lack of it)
wrong, and when output is garbled, users are annoyed. Things like
text editors are almost certain to behave incorrectly. Testing in
test-terminal-util indicates that the time required to make a successful
query is on the order of a dozen microseconds, and an unsuccessful
query costs as much as our timeout, i.e. currently 1/3 ms. I think
this is an acceptable tradeoff.
No caching is used, because fixup_environment() is only called once,
and the other place in build_environment(), only affects services
which are connected to a tty, which is only a handful of services,
and often only started in special circumstances.
As requested in https://github.com/systemd/systemd/issues/36994,
use DCS + q name ST. This works, but has limited terminal support:
xterm, foot, kitty.
Yu Watanabe [Thu, 29 May 2025 01:22:21 +0000 (10:22 +0900)]
login: add device monitor instance to receive events for devices with uaccess tag
With c960ca2be1cfd183675df581f049a0c022c1c802, logind triggers uevents
for devices with uaccess tag, and waits for the events being processed
by udevd.
However, logind received not all triggered events, and might lose some
events. That causes session and user state file not updated, and many
desktop environment application handled the session and user were inactive.
This introduces one more device monitor instance which monitor events
for devices with 'uaccess' tag. Hence, all triggered events will be
recieved by logind, and session and user state file will be updated.
coredump: introduce an enum to wrap dumpable constants
Two constants are described in the man page, but are not defined by a header.
The third constant is described in the kernel docs. Use explicit values to
show that those are values are defined externally.
A new core_pattern specifier was added, %F, to provide a PIDFD
to the usermode helper process referring to the crashed process.
This removes all possible race conditions, ensuring only the
crashed process gets inspected by systemd-coredump.
The check looks plausible, but when I started checking whether it needs
to be lowered for the recent changes, I realized that it doesn't make
much sense.
context_parse_iovw() is called from a few places, e.g.:
- process_socket(), where the other side controls the contents of the
message. We already do other checks on the correctness of the message
and this assert is not needed.
- gather_pid_metadata_from_argv(), which is called after
inserting MESSAGE_ID= and PRIORITY= into the array, so there is no
direct relation between _META_ARGV_MAX and the number of args in the
iovw.
- gather_pid_metadata_from_procfs(), where we insert a bazillion fields,
but without any relation to _META_ARGV_MAX.
Since we already separately check if the required stuff was set, drop this
misleading check.
The kernel provides %d which is documented as
"dump mode—same as value returned by prctl(2) PR_GET_DUMPABLE".
We already query /proc/pid/auxv for this information, but unfortunately this
check is subject to a race, because the crashed process may be replaced by an
attacker before we read this data, for example replacing a SUID process that
was killed by a signal with another process that is not SUID, tricking us into
making the coredump of the original process readable by the attacker.
With this patch, we effectively add one more check to the list of conditions
that need be satisfied if we are to make the coredump accessible to the user.
Reportedy-by: Qualys Security Advisory <qsa@qualys.com>
In principle, %d might return a value other than 0, 1, or 2 in the future.
Thus, we accept those, but emit a notice.
Yu Watanabe [Thu, 15 May 2025 03:34:35 +0000 (12:34 +0900)]
core: introduce Unit.dependency_generation counter and restart loop when dependency is updated in the loop
When starting unit A, a dependent unit B may be loaded if it is not
loaded yet, and the dependencies in unit A may be updated.
As Hashmap does not allow a new entry to be added in a loop, we need to
restart loop in such case.
Yu Watanabe [Tue, 20 May 2025 19:38:07 +0000 (04:38 +0900)]
core/transaction: do not override unit load state when unit_load() failed
When unit_load() failed for some reasons, previously we overrided the
load state with UNIT_NOT_FOUND, but we did not update the
Unit.fragment_not_found_timestamp_hash. So, the unit may be loaded
multiple times when the unit is in a dependency list of another unit,
as manager_unit_cache_should_retry_load() will be true again even on
next call.
Let's not override the unit state set by unit_load().
Note, after unit_load(), the unit state should not be UNIT_STUB.
Let's also add the assertion about that.
This change is important when combined with the next commit, as with the
next commit we will restart the FOREACH_UNIT_DEPENDENCY() loop if an unit
is reloaded, hence overriding load state with UNIT_NOT_FOUND may cause
infinit loop.
Yu Watanabe [Tue, 20 May 2025 19:32:09 +0000 (04:32 +0900)]
core/transaction: drop redundant call of bus_unit_validate_load_state()
The function manager_unit_cache_should_retry_load() reutrns true only
when the unit state is UNIT_NOT_FOUND. Hence, it is not necessary to
call bus_unit_validate_load_state() before checking
manager_unit_cache_should_retry_load().
Yu Watanabe [Tue, 27 May 2025 18:18:14 +0000 (03:18 +0900)]
udevadm: allow to specify device by device ID
We have already exposed device ID in the output of device ID in J
fields. Also sd_device_get_device_id() and sd_device_new_from_device_id()
are already public. Hence, making udevadm accept device IDs may be
useful.
With this change, as we save several data in /run/udev with device ID,
we can call udevadm something like the following:
```
udevadm info $(ls /run/udev/tags/uaccess)
```
Then, we can show all devices that has uaccess tag.
Jan Čermák [Wed, 28 May 2025 18:33:03 +0000 (20:33 +0200)]
journal-gatewayd: add /boots endpoint (#37574)
Add endpoint for listing boots. Output format mimics `journalctl
--list-boots -o json`, so it's a plain array containing index, boot ID
and timestamps of the first and last entry. Initial implementation
returns boots ordered starting with the current one and doesn't allow
any filtering (i.e. equivalent of --lines argument).