We use strtoul() which returns an "unsigned long", but then assign this
to int or unsigned in, i.e. drop 32bit silently on 64bit systems. Let's
clean this up a bit, and retain the right types.
This stuff is likely to fail in many setups (for example when quota is
not supported by the btrfs version), hence only log at debug
level. Previously we'd silently ignore things altogether which makes
things pretty hard to debug.
This changes the output a bit, as the previous multi-line output of each
inhibitor is changed to a single line, but it does unify the output look
with the one of our other tools. Moreover this adds proper sorting.
When we parse an "u" from an sd_bus_message then we need to do that into
a uint32_t, not a pid_t or uid_t, even if this is likely the same.
Also, let's count objects we keep in memory as size_t as usual.
In seccomp code, the code is changed to propagate errors which are about
anything other than unknown/unimplemented syscalls. I *think* such errors
should not happen in normal usage, but so far we would summarilly ignore all
errors, so that part is uncertain. If it turns out that other errors occur and
should be ignored, this should be added later.
In nspawn, we would count the number of added filters, but didn't use this for
anything. Drop that part.
The comments suggested that seccomp_add_syscall_filter_item() returned negative
if the syscall is unknown, but this wasn't true: it returns 0.
The error at this point can only be if the syscall was known but couldn't be
added. If the error comes from our internal whitelist in nspawn, treat this as
error, because it means that our internal table is wrong. If the error comes
from user arguments, warn and ignore. (If some syscall is not known at current
architecture, it is still silently ignored.)
Our logs are full of:
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldstat() / -10037, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call get_thread_area() / -10076, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call set_thread_area() / -10079, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldfstat() / -10034, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldolduname() / -10036, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call oldlstat() / -10035, ignoring: Numerical argument out of domain
Sep 19 09:22:10 autopkgtest systemd[690]: Failed to add rule for system call waitpid() / -10073, ignoring: Numerical argument out of domain
...
This is pointless and makes debug logs hard to read. Let's keep the logs
in test code, but disable it in nspawn and pid1. This is done through a function
parameter because those functions operate recursively and it's not possible to
make the caller to log meaningfully.
There should be no functional change, except the skipped debug logs.
We have "installed tests", but don't provide an easy way to run them.
The protocol is very simple: each test must return 0 for success, 77 means
"skipped", anything else is an error. In addition, we want to print test
output only if the test failed.
I wrote this simple script. It is pretty basic, but implements the functions
listed above. Since it is written in python it should be easy to add option
parsing (like running only specific tests, or running unsafe tests, etc.)
I looked at the following alternatives:
- Ubuntu root-unittests: this works, but just dumps all output to the terminal,
has no coloring.
- @ssahani's test runner [2]
It uses the unittest library and the test suite was implented as a class, and
doesn't implement any of the functions listed above.
- cram [3,4]
cram runs our tests, but does not understand the "ignore the output" part,
has not support for our magic skip code (it uses hardcoded 80 instead),
and seems dead upstream.
- meson test
Here the idea would be to provide an almost-empty meson.build file under
/usr/lib/systemd/tests/ that would just define all the tests. This would
allow us to reuse the test runner we use normally. Unfortunately meson requires
a build directory and configuration to be done before running tests. This
would be possible, but seems a lot of effort to just run a few binaries.
[1] 242c96addb/debian/tests/root-unittests
[2] https://github.com/systemd/systemd-fedora-ci/blob/master/upstream/systemd-upstream-tests.py
[3] https://bitheap.org/cram/
[4] https://pypi.org/project/pytest-cram/Fixes#10069.
"because blacklisted" is not grammatical, it should be something like
"becuase it is blacklisted", which in turn in too verbose. Let's use
a shorter form.
This effectively reverts 2bc54be485
and relevant changes in #9920, as it is used to determine the version
of udev, e.g., dracut.
Fixesdracutdevs/dracut#468.
This reverts commit d4e9e574ea.
(systemd.conf.m4 part was already reverted in 5b5d82615011b9827466b7cd5756da35627a1608.)
Together those reverts should "fix" #10025 and #10011. ("fix" is in quotes
because this doesn't really fix the underlying issue, which is combining
DynamicUser= with strict container sandbox, but it avoids the problem by not
using that feature in our default installation.)
Dynamic users don't work well if the service requires matching configuration in
other places, for example dbus policy. This is true for those three services.
In effect, distros create the user statically [1, 2]. Dynamic users make more
sense for "add-on" services where not creating the user, or more precisely,
creating the user lazily, can save resources. For "basic" services, if we are
going to create the user on package installation anyway, setting DynamicUser=
just creates unneeded confusion. The only case where it is actually used is
when somebody forgets to do system configuration. But it's better to have the
service fail cleanly in this case too. If we want to turn on some side-effect
of DynamicUser=yes for those services, we should just do that directly through
fine-grained options. By not using DynamicUser= we also avoid the need to
restart dbus.
[1] bd9bf30727
[2] 48ac1cebde/f/systemd.spec (_473)
(Fedora does not create systemd-timesync user.)
systemd-tmpfiles-setup.service needs to be ordered after
systemd-journald.service, so entries in /run/log/journal are already
created when systemd-tmpfiles tries to adjust its permissions.
This is specially problematic for setups using a volatile journal where
the initrd does not ship a machine-id (i.e. OSTree-based systems), where
logs from the initrd will be inaccessible for users in the
systemd-journal group. It also has a side effect of `journalctl --user`
failing with "No journal files were opened due to insufficient
permissions".
Fixes#10128.
We would create a useless empty directory under build/.
It seems we were lucky and all symlinks were installed into directories
which were alredy created because we installed something into the same
location earlier.
While at it, also add '-v' to 'mkdir -p'. This will print the names of
directories as they are created (just once), making it easier to see all of
what the install script is doing.
In order to shut down networkd properly, the delegated routes added
need to be removed properly, and as error reporting is wanted, the
network link is needed in the debug output.
Solve this by calling manager_dhcp6_prefix_remove_all(), which will
remove each prefix stored in the Manager structure, and while doing
that reference each link so that it isn't freed before the route
removal callback is called. This in turn causes the network link to
be referenced once more, and an explicit hashmap_remove() must be
called to remove the network link from the m->links hashmap.
Also, since the registered callback is not called when the DHCPv6
client is stopped with sd_dhcp6_client_stop(), an explicit call
to dhcp6_lease_pd_prefix_lost() needs to be made to clean up any
unreachable routes set up for the delegated prefixes.
Zbigniew Jędrzejewski-Szmek