Currently, when a socket unit specifies SmackLabel=,
the label is not applied to the underlying Unix socket file or its file
descriptor.
This change ensures that the SMACK label is applied both to the
Unix socket path on the filesystem and to all associated socket FDs
when the socket is created.
Testing:
- Tested on Fedora 43 with kernel 6.17.7 with SMACK enabled.
- Created a systemd socket unit:
[Unit]
Description=UNIX socket
[Socket]
ListenStream=/run/test.sock
SmackLabel=label
[Install]
WantedBy=sockets.target
- Created a corresponding service:
[Unit]
Description=UNIX socket service
Requires=test.socket
[Service]
ExecStart=/usr/bin/socat -v - -
StandardInput=socket
[Install]
WantedBy=multi-user.target
- Verified SMACK labels using getfattr:
- Without SmackLabel:
```
# file: run/test.sock
security.SMACK64="_"
# file: proc/<pid>/fd/*
security.SMACK64="*"
```
- With SmackLabel=label:
```
# file: run/test.sock
security.SMACK64="label"
# file: proc/<pid>/fd/*
security.SMACK64="label"
```
In all cases, everything that we list in 'extract', we also list in
'sources'. We can simplify things by automatically appending the first
list to the second.
In the listings, move 'extract' key right below 'sources', since now
they are both "sources", just with slightly different meanings.
When a socket unit specifies SmackLabel=, the label was previously
not applied to the underlying Unix socket file or its file descriptor.
This change ensures that the SMACK label is applied both to the
socket path on the filesystem and to the opened socket FD.
The function socket_address_listen() is declared at shared/socket-label.c,
however its prototype was in basic/socket-util.h. This moves the
prototype to shared/socket-label.h.
Also, enum SocketAddressBindIPv6Only is not used anymore in basic/*.[ch].
Let's move the definition and its string table to shared/socket-label.[ch].
Follow-up for b25a930f0e.
When inheriting file descriptors for stdout/stderr (either from stdin or
when making stderr inherit from stdout), we previously just assumed they
would be writable and dup'd them. This could lead to broken setups if
the inherited FD was actually opened read-only.
Before dup'ing any inherited FDs to stdout/stderr, verify they are
actually writable using the new fd_is_writable() helper. If not, fall
back to /dev/null (or reopen the terminal in the TTY case) with a
warning, rather than silently creating a broken setup where output
operations would fail.
Add support for an empty Gateway= in [Network] to clear the existing
routes. This change will allow users to remove the default route from a
drop-in file.
Looking at the list, "test" or "testing" seems to be a fairly generic entry
that is missing from the list of suggestions. I went with "testing" because it
fits better with the other item, e.g. "staging".
In https://github.com/systemd/systemd/issues/38743 "laboratory" was also
suggested. I didn't include this because that is more about the location, not
deployment type. Any of the other deployments could be in a "laboratory".
Closes https://github.com/systemd/systemd/issues/38743.
This series fixes two distinct, pretty bad bugs in `ASSERT_SIGNAL`.
These bugs can allow failing tests to pass, and can also cause the test
runner to silently terminate prematurely in a way that looks like
success.
This is not theoretical, see
https://github.com/systemd/systemd/pull/39674#discussion_r2540552699 for
a real case of this happening.
---
Bug 1: Parent process hallucinates it is the child and re-executes the
expression being tested
Previously, assert_signal_internal() returned 0 in two mutually
exclusive states:
1. We are the child process (immediately after fork()).
2. We are the parent process, and the child exited normally (status 0).
The macro failed to distinguish these cases. If a child failed to crash
as expected, the parent received 0, incorrectly interpreted it as it
being the child, and re-executed the test expression inside the parent
process.
This can cause tests to falsely pass. The parent would successfully run
the expression (which wasn't supposed to crash in the parent), succeed,
and call _exit(EXIT_SUCCESS).
The second consequence is silent truncation. When the parent called
_exit(), it terminated the entire test runner immediately. Any
subsequent tests in the same binary were never executed.
---
Bug 2: Conflation of exit codes and signals
The harness returned the raw si_status without checking si_code. This
meant that an exit code was indistinguishable from a signal number. For
example, if a child process failed and called exit(6), the harness
reported it as having been killed by SIGABRT (signal 6).
---
This PR both fixes the bugs and reworks the ASSERT_SIGNAL infrastructure
to ensure this is very unlikely to regress:
- assert_signal_internal now returns an explicit control flow enum
(FORK_CHILD / FORK_PARENT) separate from the status data. This makes it
structurally impossible for the parent to hallucinate that it is the
child.
- The output parameter is only populated with a signal number if si_code
confirms the process was killed by a signal. Normal exits return 0.
assert_signal_internal() returns 0 in two distinct cases:
1. In the child process (immediately after fork returns 0).
2. In the parent process, if the child exited normally (no signal).
ASSERT_SIGNAL fails to distinguish these cases. When a child exited
normally (case 2), the parent process receives 0, incorrectly interprets
it as meaning it is the child, and re-executes the test expression
inside the parent process. Goodness gracious!
This causes two severe test integrity issues:
1. False positives. The parent can run the expression, succeed, and call
_exit(EXIT_SUCCESS), causing the test to pass even though no signal
was raised.
2. Silent truncation. The _exit() call in the parent terminates the test
runner prematurely, preventing subsequent tests in the same file from
running.
Example of the bug in action, from #39674:
ASSERT_SIGNAL(fd_is_writable(closed_fd), SIGABRT)
This test should fail (fd_is_writable does not SIGABRT here), but with
the bug, the parent hallucinated being the child, re-ran the expression
successfully, and exited with success.
Fix this by refactoring assert_signal_internal() to be much more strict
about separating control flow from data.
The signal status is now returned via a strictly typed output parameter,
guaranteeing that determining whether we are the child is never
conflated with whether the child exited cleanly.
ASAN installs signal handlers to catch crashes like SIGSEGV or SIGILL.
When these signals are raised, ASAN traps them, prints an error report,
and then typically terminates the process with a different signal (often
SIGABRT) or a non-zero exit code.
This interferes with ASSERT_SIGNAL when checking for specific crash
signals (for example, checking that a function raises SIGSEGV). In such
a case, the test harness sees the ASAN termination signal rather than
the expected signal, causing the test to fail.
Fix this by resetting the signal handler to SIG_DFL in the child process
immediately before executing the test expression. This ensures the
kernel kills the process directly with the expected signal, bypassing
ASAN's interceptors.
When a child process exits normally (si_code == CLD_EXITED),
siginfo.si_status contains the exit code. When it is killed by a signal
(si_code == CLD_KILLED or CLD_DUMPED), si_status contains the signal
number. However, assert_signal_internal() returns si_status blindly.
This causes exit codes to be misinterpreted as signal numbers.
This allows failing tests to silently pass if their exit code
numerically coincides with the expected signal. For example, a test
expecting SIGABRT (6) would incorrectly pass if the child simply exited
with status 6 instead of being killed by a signal.
Fix this by checking si_code. Only return si_status as a signal number
if the child was actually killed by a signal (CLD_KILLED or CLD_DUMPED).
If the child exited normally (CLD_EXITED), return 0 to indicate that no
signal occurred.
When inheriting file descriptors for stdout/stderr (either from stdin
or when making stderr inherit from stdout), we previously just assumed
they would be writable and dup'd them. This could lead to broken setups
if the inherited FD was actually opened read-only.
Before dup'ing any inherited FDs to stdout/stderr, verify they are
actually writable using the new fd_is_writable() helper. If not, fall
back to /dev/null (or reopen the terminal in the TTY case) with a
warning, rather than silently creating a broken setup where output
operations would fail.
This checks whether a file descriptor is valid and opened in a mode that
allows writing (O_WRONLY or O_RDWR). This is useful when we want to
verify that inherited FDs can actually be used for output operations
before dup'ing them.
The helper explicitly handles O_PATH file descriptors, which cannot be
used for I/O operations and thus are never writable.
We were getting a list of invocation IDs, picking one at random,
and then querying the unit. This is obviously racy.
TEST-74-AUX-UTILS.sh[2873]: + varlinkctl call /run/systemd/io.systemd.Manager io.systemd.Unit.List
'{"invocationID": "2052c9a5-7983-4f72-9910-c49e38c91dab"}'
TEST-74-AUX-UTILS.sh[3707]: Method call io.systemd.Unit.List() failed: io.systemd.Unit.NoSuchUnit
The complicated varlink + jq callout is replaced by a simple systemctl call.
I think that's better to avoid a complicated jq expression.
Fixes https://github.com/systemd/systemd/issues/38647.
The ASSERT_SIGNAL macro uses a fixed variable name, `_r`. This prevents
nesting the macro (like ASSERT_SIGNAL(ASSERT_SIGNAL(...))), as the inner
instance would shadow the outer instance's variable.
Switch to using the UNIQ_T helper to generate unique variable names at
each expansion level. This allows the macro to be used recursively,
which is required for upcoming regression tests regarding signal
handling logic.
Symbols exported by libshared can't get pruned by the linker, so
every unused exported symbol is effectively dead code we ship to users
for no good reason. Let's add a script to analyze how many such symbols
we have.
We also add a meson test to run the script on all of our binaries.
Since it detects unused symbols and still has a few false positives,
don't enable the test by default similar to the clang-tidy tests.
The script was 100% vibe coded by Github Copilot with Claude Sonnet 4.5
as the model.
Current results are (without the unused symbols list):
Analysis of libsystemd-shared-259.so
======================================================================
Total exported symbols: 4830
(excluding public API symbols starting with 'sd_')
Used symbols: 4672
Unused symbols: 158
Usage rate: 96.7%
In logs in the Fedora OpenQA CI:
Nov 17 22:20:06 fedora systemd-ssh-generator[4117]: Failed to query local AF_VSOCK CID: Cannot assign requested address
Nov 17 22:20:06 fedora (generato[4088]: /usr/lib/systemd/system-generators/systemd-ssh-generator failed with exit status 1.
Nov 17 22:20:06 fedora systemd[1]: sshd-vsock.socket: Unit configuration changed while unit was running, and no socket file descriptors are open. Unit not functional until restarted.
AF_VSOCK is not configured there and systemd-ssh-generator should just exit
quietly. vsock_get_local_cid() already does some logging at debug level, so we
don't need to.
There is also a second bug, we report modifications to the unit have just
created. I think we have an issue open for this somewhere, but cannot find it.
`systemd.network(5)` recommends “that each filename is prefixed with a number
smaller than "70" (e.g. 10-eth0.network)”.
Reduce that used by the example accordingly, but stay above the number (`50`)
used in the earlier example for static configuration, so that would take
precedence over the dynamic one if both match for the same network.
This updates example output in systemd-analyze's man page after the
tool's output was changed in a previous commit.
Additionally bash completion is added for `systemd-analyze filesystems`
and improved for `systemd-analyze calendar`.
Before:
$ build/systemd-creds --uid=asdf
Failed to resolve user 'asdf': No such process
Now:
$ build/systemd-creds --uid=asdf
Failed to resolve user 'asdf': Unknown user
$ sudo build/systemd-run --uid=asdf whoami
$ journalctl -e
(whoami)[1007784]: run-p1007782-i5200512.service: Failed to determine user credentials: No such process
(whoami)[1007784]: run-p1007782-i5200512.service: Failed at step USER spawning /usr/sbin/whoami: No such process
systemd[1]: run-p1007782-i5200512.service: Main process exited, code=exited, status=217/USER
systemd[1]: run-p1007782-i5200512.service: Failed with result 'exit-code'.
Now:
(whoami)[1013204]: run-p1013202-i5205932.service: Failed to determine credentials for user 'asdf': Unknown user
(whoami)[1013204]: run-p1013202-i5205932.service: Failed at step USER spawning /usr/sbin/whoami: Invalid argument
systemd[1]: run-p1013202-i5205932.service: Main process exited, code=exited, status=217/USER
systemd[1]: run-p1013202-i5205932.service: Failed with result 'exit-code'.
Before:
$ sudo build/systemd-run --scope --uid=asdf whoami
Failed to resolve user asdf: No such process
Now:
$ sudo build/systemd-run --scope --uid=asdf whoami
Failed to resolve user 'asdf': Unknown user
From a boot with a dracut initrd:
systemd-tmpfiles[242]: /usr/lib/tmpfiles.d/tpm2-tss-fapi.conf:2: Failed to resolve user 'tss': No such process
systemd-tmpfiles[242]: Failed to parse ACL "default:group:tss:rwx", ignoring: Invalid argument
systemd-tmpfiles[242]: /usr/lib/tmpfiles.d/tpm2-tss-fapi.conf:4: Failed to resolve user 'tss': No such process
systemd-tmpfiles[242]: Failed to parse ACL "default:group:tss:rwx", ignoring: Invalid argument
systemd-tmpfiles[242]: /usr/lib/tmpfiles.d/tpm2-tss-fapi.conf:6: Failed to resolve group 'tss': No such process
systemd-tmpfiles[242]: /usr/lib/tmpfiles.d/tpm2-tss-fapi.conf:7: Failed to resolve group 'tss': No such process
We cannot just use %m, because strerror returns a confusing error message
for ESRCH or ENOEXEC. udev code was doing a good job, but the error handling
was very verbose. Let's encapsulate the customized error messages in a
helper.
No functional change, except that the error messages have a slightly different
form now. The old messages were a bit better, but we don't have as much
flexibility in the new scheme. "Failed to resolve user 'foo': Unknown user"
should be good enough.
