Xi Ruoyao [Tue, 9 May 2023 17:41:28 +0000 (01:41 +0800)]
sd-bus: bus_message_type_from_string is not pure
GCC document [1] says:
The pure attribute prohibits a function from modifying the state
of the program that is observable by means other than inspecting
the function’s return value.
And there is an example:
`int hash (char *) __attribute__ ((pure));`
... Even though hash takes a non-const pointer argument it must
not modify the array it points to, ...
But we are modifying the object pointed to by the pointer u, which is
clearly a violation of the semantic of pure.
With -ftrivial-auto-var-init (enabled by -Dmode=release), on some
targets (GCC 12.2 on AArch64 and GCC 13.1 on x86_64) performs an
optimization: as the variable "u" in bus_match_parse has been
zero-initialized (by the -ftrivial-auto-var-init option) and never
modified (because a "pure" bus_message_type_from_string is not allowed
to modify it), "u" will be always 0.
Then 0 is used to initialize .value_u8 field of struct
bus_match_component. This then causes a infinite event loop, so
"systemctl restart" never stops, and pam_systemd timeouts communicating
with logind, etc.
Michal Sekletar [Fri, 5 May 2023 10:50:15 +0000 (12:50 +0200)]
core/mount: replace invalid UTF-8 code points in "what" and "options"
Previously, these strings were copied w/o any modification. However, if
mount table contained e.g. remote fs mount like NFS or CIFS that was
mounted from server running with non UTF-8 locale then those strings
might have contained some non UTF-8 characters.
If then client asked about status of such mount unit we tried to forward
these non UTF-8 strings over D-Bus. That is the violation of the
protocol and we ended up kicked from the bus.
Passing 0 to log_xxx_errno() leads to an assertion, so let's not do that:
$ NEWPASSWORD="" build-san/systemd-cryptenroll --unlock-key-file=/tmp/password --password "$img"
/tmp/password has 0644 mode that is too permissive, please adjust the ownership and access mode.
Assertion '(_error) != 0' failed at src/cryptenroll/cryptenroll-password.c:164, function enroll_password(). Aborting.
Aborted (core dumped)
test: merge unit file related tests into TEST-23-UNIT-FILE
Rename TEST-23-TYPE-EXEC to TEST-23-UNIT-FILE and merge it with
following tests:
- TEST-37-RUNTIMEDIRECTORYPRESERV
- TEST-40-EXEC-COMMAND-EX
- TEST-41-ONESHOT-RESTART
- TEST-42-EXECSTOPPOST
- TEST-57-ONSUCCESS-UPHOLD
This gets rid of the all-but-one remaining uses of perl. I tested the new code
on my machine, so I'm fairly confident that it works as expected.
install_iscsi() has one majestic perl invocation, but we can't get rid of it
easily: it extends the code of tgt-admin to print some list of files. Obviously
this only works because tgt-admin is written in perl, and perl will be installed
if tgt-admin is installed. install_iscsi() is used in TEST-64-UDEV-STORAGE
conditionally if tgtadm is installed, so this can stay as is.
README: require python >= 3.7, clean up module descriptions
libpython was added in 2cc86f094a8c316f7feb0336df3827a3264b116d, it seems
because of python-systemd module that we built. But libpython by itself
is not enough for actual python programs, and now we also list python itself,
so let's drop libpython from the list.
meson requires >= 3.7. We have CI that runs on CentOS8 with Python 3.6, but
let's not provide official support for an EOL Python version. Individual
distributions can provide backports, but we don't need to mention that in
the user-facing docs. According to [1], 3.7 is on life support and 3.6 is EOL.
test/run-unit-tests, TEST-02: skip tests where the interpeter is not installed
When the interpeter is missing, we get an exit code of 127. Let's treat those
tests as skipped too. If we could run the test far enough so that it could do
the check itself, it would return 77 anyway.
$ test/asdf; echo $?
exec: Failed to execute process 'test/asdf': The file specified the interpreter '/bin/asdf', which is not an executable command.
127
$ test/asdf; echo $?
/usr/bin/env: ‘/bin/asdf’: No such file or directory
127
This should resolve the problem that TEST-02 fails or Debian's 'unit-tests' fail
when python3 is not installed. Installing python3 via the mechanism that is
used to construct TEST images, i.e. the dracut dependency chasing scheme, would
be a lot of work for python with its modules in multiple locations and hundreds
of little files. So I think it OK to just skip the test there, and also in
other cases where python is not available.
As part of the build, we would populate build/test/sys/ using
sys-script.py, and then udev-test.p[ly] would create a tmpfs instance
on build/test/tmpfs and copy the sys tree to build/test/tmpfs/sys.
Also, we had udev-test.p[ly] which called test-udev. test-udev was
marked as a manual test and installed, but neither udev-test.p[ly] or
sys-script.py were.
test-udev is renamed to udev-rule-runner, which reduces confusion and
frees up the test-udev name. udev-test.py is renamed to test-udev.py.
All three files are now installed.
test-udev.py is modified to internally call sys-script.py to set up the
sys tree. Copying and creating it from scratch should take the same
amount of time. We avoid having a magic directory, everything is now
done underneath a temporary directory.
test-udev.py is now a normal installed test, and run-unit-tests.py will
pick it up. When test-udev.py is invoked from meson, the path to
udev-rule-runner is passed via envvar; when it is invoked via
run-unit-tests.py or directly, it looks for udev-rule-runner in a relative
path.
The goal of this whole change is to let Debian drop the 'udev' test.
It called sys-script.py and udev-test.pl from the source directory and
had to recreate a bunch of the logic. Now test-udev.py will now be called
via 'upstream'.
I tried to keep this a 1:1 rewrite with the same field names.
Nevertheless, some changes were made:
- exp_add_error and exp_rem_error are dropped. Those fields meant that
"./test-udev add <devpath>" actually succeeded, but symlinks were not
created, and exp_links was ignored and could contain bogus content.
Instead, exp_links and not_exp_links are adjusted to not contain
garbage and the tests check that "./test-udev add" succeeds and that
the links are as expected from exp_links and not_exp_links.
- cleanup was only used in one rule, and that rule was expected to fail,
so cleanup wasn't actually necessary. So the cleanup field and the
logic to call cleanup from individual tests is removed.
- a bunch of fields were set, but didn't seem to be connected to any
implementation: not_exp_name, not_exp_test. e62acc3159935781f05fa59c48e5a74e85c61ce2 did a rewrite of some of the
tests and it seems that not_exp_test was added by mistake and
not_exp_name was left behind by mistake.
In Python, the field list is declared in the class, so it's harder to
assign an unused attribute. Those uses were converted to not_exp_links.
- in most rules, r"""…""" is used, so that escaping is not necessary.
- the logic to generate devices was only used in one place, and the
generator function also had provisions to handle arguments that were
never given. all_block_devs() is made much simpler.
- Descriptions that started with a capital letter were shortened
and lowercased.
- no special test case counting is done. pytest just counts the cases
(Rules objects).
- the output for failures is also removed. If something goes wrong, the
user can use pytest --pdb or such to debug the issue.
- perl version used a semaphore to manage udev runners, and would fork,
optionally wait a bit, and then start the runner. In the python
version, we just spawn them all and wait for them to exit. It's not
very convenient to call fork() from python, so instead the runner
was modified (in previous commit) to wait.
The test can be called as:
(cd build && sudo pytest -v ../test/udev-test.py)
sudo meson test -C build udev-test.py -v
I think this generally provides functionality that is close to the perl
version. It seems some of the checks are now more fully implemented.
Support for strace/gdb/valgrind is missing.
check-includes: print path relative to project root
Instead of /home/zbyszek/src/systemd-work/build/../src/xdg-autostart-generator/xdg-autostart-service.h:11,
print just src/xdg-autostart-generator/xdg-autostart-service.h:11.
This is a bit annoying that this requires so much verbosity, but the output
with the full names was too annoying.
Mike Yuan [Mon, 8 May 2023 16:07:45 +0000 (00:07 +0800)]
core: refuse dbus activation if dbus is not running
dbus-broker issues StartUnit directly for activation requests,
so let's add a check on bus state in bus_unit_queue_job to refuse
that if dbus is not running.
This undoes the effect of 1394a3ec351048bae008627a0775d1f9a6c46294 partially.
We print the fairly verbose output of the build commands, so let's also
print the commands themselves. This makes it much easier to understand what
is going on.
(The style was copied from other scripts where we do 'set -x' for one command.)
Dan Streetman [Wed, 7 Dec 2022 16:23:59 +0000 (11:23 -0500)]
tpm2: move policy building out of policy session creation
This retains the use of policy sessions instead of trial sessions
in most cases, based on the code comment that some TPMs do not
implement trial sessions correctly. However, it's likely that the
issue was not the TPMs, but our code's incorrect use of PolicyPCR
inside a trial session; we are not providing expected PCR values
with our call to PolicyPCR inside a trial session, but the spec
indicates that in a trial session, the TPM *may* return error if
the expected PCR value(s) are not provided. That may have been the
source of the original confusion about trial sessions.
More details:
https://github.com/systemd/systemd/pull/26357#pullrequestreview-1409983694
Also, future commits will replace the use of trial sessions with
policy calculations, which avoids the problem entirely.
Ronan Pigott [Fri, 5 May 2023 19:33:29 +0000 (12:33 -0700)]
zsh: remove usage of PREFIX in _systemctl
The usage of PREFIX in this completion is mostly counter to the intended
usage of compsys in zsh. It is generally expected that completion code
provide the available completions and tags in that word position so that
compsys, with user configuration, can filter them to the appropriate set.
One egregious error caused by the usage of PREFIX here is the caching of
SYS_ALL_UNITS, which stored only the unit names prematurely filtered by
the completion prefix, affecting all future completions. For example,
$ systemctl cat nonsense<TAB>
might find no matching units if nonsense* has no matches, but now
$ systemctl cat <TAB>
will fail in all future completions even though every unit file
is a valid match, because the cached set has been erroneously filtered
by the last prefix.
Nick Rosbrook [Tue, 2 May 2023 16:30:31 +0000 (12:30 -0400)]
basic/audit-util: make a test request before enabling use of audit
If a container manager does not follow the guidance in
https://systemd.io/CONTAINER_INTERFACE/ regarding audit capabilities,
then the current check may not be sufficient to determine that audit
will function properly. In particular, when calling bind() on the audit
fd, we will get EPERM if running in a user-namespaced container.
Expand the check to make an AUDIT_GET_FEATURE request on the audit fd to
test if it is working. If this fails with ECONNREFUSED, we know it is
because the kernel does not support the use of audit outside of the
initial user namespace.
Note that the approach of this patch was suggested here:
https://github.com/systemd/systemd/pull/19443#issuecomment-829566659
As in mkosi(1), let's describe the config file and commandline options
together. This is nice for us, because we don't need to duplicate descriptions
and we're less likely to forget to update one place or the other. This is also
nice for users, because they can easily figure out what can be configured
where.
The options are now ordered by config file section.
test_ukify: rework how --flakes argument is appended
The usual approach is to put 'addopts = --flakes' in setup.cfg. Unfortunately
this fails badly when pytest-flakes is not installed:
ERROR: usage: test_ukify.py [options] [file_or_dir] [file_or_dir] [...]
test_ukify.py: error: unrecognized arguments: --flakes
pytest-flakes is not packaged everywhere, and this test is not very important,
so let's just do it only if pytest-flakes is available. We now detect if
pytest-flakes is available and only add '--flakes' conditionally. This
unfortunately means that when invoked via 'pytest' or directly as
'src/ukify/test/test_ukify.py', '--flakes' will not be appended automatically.
But I don't see a nice way to achieve previous automatic behaviour.
(I first considered making 'setup.cfg' templated. But then it is created
in the build directory, but we would need it in the source directory for
pytest to load it automatically. So to load the file, we'd need to give an
argument to pytest anyway, so we don't gain anything with this more complex
approach.)
Note to self: PEP 585 introduced using collection types as types,
and is available since 3.9. PEP 604 allows writing unions with "|",
but is only available since 3.10, so not yet here because we maintain
compat with 3.9.
test-kernel-install: test 60-ukify.install and 90-uki-copy.install
We install a kernel with layout=uki and uki_generator=ukify, and test
that a UKI gets installed in the expected place. The two plugins cooperate,
so it's easiest to test them together.
60-ukify: kernel-install plugin that calls ukify to create a UKI
60-ukify.install calls ukify with a config file, so singing and policies and
splash will be done through the ukify config file, without 60-ukify.install
knowing anything directly.
In meson.py, the variable for loaderentry.install.in is used just once, let's
drop it. (I guess this approach was copied from kernel_install_in, which is
used in another file.)
The general idea is based on cvlc12's #27119, but now in Python instead of
bash.
ukify: rework option parsing to support a config file
In some ways this is similar to mkosi: we have a argparse.ArgumentParser()
with a bunch of options, and a configparser.ConfigParser() with an
overlapping set of options. Many options are settable in both places, but
not all. In mkosi, we define this in three places (a dataclass, and a
function for argparse, and a function for configparser). Here, we have one
huge list of ConfigItem instances. Each instance specifies the full metadata
for both parsers. Argparse generates a --help string for all the options,
and we also append a config file sample to --help based on the ConfigItem
data:
With --summary, existence of input paths is not checked. I think we'll
want to show them, instead of throwing an error, but in red, similarly to
'bootctl list'.
This also fixes tests which were failing with e.g.
E FileNotFoundError: [Errno 2] No such file or directory: '/ARG1'
=========================== short test summary info ============================
FAILED ../src/ukify/test/test_ukify.py::test_parse_args_minimal - FileNotFoun...
FAILED ../src/ukify/test/test_ukify.py::test_parse_args_many - FileNotFoundEr...
FAILED ../src/ukify/test/test_ukify.py::test_parse_sections - FileNotFoundErr...
=================== 3 failed, 10 passed, 3 skipped in 1.51s ====================
projects / thirdparty / systemd.git / log
