journald: fix ignored filtering patterns for delegated cgroups
If a service defines Delegate=yes, its subcgroup won't inherit the
LogFilterPatterns= option, because the option is stored on the unit's
cgroup attributes, not on the subcgroup.
Fixed by using the unit's cgroup attributes instead.
From a given cgroup path, cg_path_get_unit() allows to retrieve the
unit's name. Although, this removes the path to the unit's cgroup,
preventing the result to be used to fetch xattrs.
Introduce cg_path_get_unit_path() which provides the path to the unit's
cgroup. This function behave similarly to cg_path_get_unit() (checking
the validity and escaping the unit's name).
manager: improve message about Reload/Reexec requests
If we fail to get the necessary information, let's just not print that
part of the message. 'n/a' looks pretty ugly.
I used a bunch of ternary operators instead of seperate log lines because
with two components that might or might not be there, we need four different
combinations.
Also, the unit name doesn't need to be quoted, it's always printable.
manager: "downgrade" message about command vanishing from the unit file
We would print "Current command vanished from the unit file, execution of
the command list won't be resumed." as a warning, but most of the time there
is nothing to resume, because a unit has just one command. So let's detect
the case where the command that was active is the last command in the sequence
and skip the warning.
I was considering how to store the information that the command is last. An
important consideration is not to use a format that would confuse older versions
of systemd. (It wouldn't be a big problem if older systemd just refused the
new serialization, since we require systemd to be newer, but we should avoid
the case where the deserialization is "successful", but actually incorrect.)
Similarly, the deserialization from the old systemd must not confuse new systemd.
For this command, we have a list of arguments at the end, so just adding a
new field either in the middle or at the end is problematic because it's hard
to ensure that we don't mix up the positional and variable arguments.
We actually need to store just one bit of information, so '+' is prefixed on
the index of the last command and used by new systemd to skip the warning.
When deserializing from older systemd, '+' is not present, so we detect all
commands as "not last", and still emit the warning, so we err on the side of
caution. If the user were to deserialize from newer to older systemd, nothing
untoward would happen, because the '+' is ignored. (Users shouldn't do this,
but we know that this occasionally happens with initrds or exitrds and package
downgrades.)
Luca Boccassi [Wed, 8 Feb 2023 02:10:28 +0000 (02:10 +0000)]
cryptenroll: do not implicitly verify with default tpm policy signature
If it was not requested to use a tpm2 signature file when enrolling, do
not fallback to the default /run/systemd/tpm2-pcr-signature.json as it
likely will be unrelated if it exists.
Jan Macku [Tue, 7 Feb 2023 14:40:47 +0000 (15:40 +0100)]
ci: remove `if: github.event.issue.pull_request` from `labeler.yml`
`github.event.issue.pull_request` is an object, not a boolean.
This is the root cause of why the step that is supposed to remove labels
is always skipped. Having this condition in place is not necessary since
the workflow is run on the `pull_request_target` event.
logs-show: rename "ts" paramater/variable to "display_ts"
When displaying log data we deal with two kind of timestamps: the one we
use for display (typically the source timestamp if available), and the
one we use internally (typically the reception timestamp of journald).
The user-facing output modes generally use the display timestamp, the
ones intended for further processing (i.e. json + export outputs) do
not, and directly query the timestamps of the entry, ignoring the
source. This gets a bit confusing, since it's not always clear why we
use which timestamp where. Let's address that by renaming the generic
"ts" parameter/variable to "display_ts" to emphasize that the stored
timestamp are "corrected" timestamps for display only.
resolve: adjust message for NXDOMAIN lookup result
Previously, we reported:
nx.example.org: resolve call failed: 'nx.example.org' not found
But the call did succeed, and in fact all communication with the upstream
servers was successful, and we got an authoritative negative answer.
So instead of saying that the call fail, just say that the host doesn't exist:
nx.example.org: Name 'nx.example.org' not found
I wanted to keep the prefix of "<name>: ", to keep the output uniform. But
it'd look a bit strange to say "<name>: <name> not found", so I added "Name "
to make the output more readable. (Another option would be to not display
the error string received from resolved, but that seems risky: even if right
now resolved uses just one message format, it could start doing something else
in the future, so it's better to display the error as received.)
resolve: define normal macros for BUS_ERROR_DNS error codes
This result is identical after cpp is done, so we don't save anything
by not having the usual macros. And with the usual macros it's easier to
grep and code-crossreferencing works better.
The change was dropped due to the process name was not correctly logged,
but the issue was fixed by dd15e4cb57129b915e01495e113696bfe0b70214.
Let's set the child process name again.
Yu Watanabe [Fri, 3 Feb 2023 09:29:36 +0000 (18:29 +0900)]
argv-util: also update program_invocation_short_name
Our logging uses program_invocation_short_name. Without this patch,
logs from forked client may become broken; spuriously truncated or
the short invocation name is not completely shown in the log.
This will warn if fake flexible arrays are re-introduced. I'm not using
-Werror=… because we may still get warnings when compiling against old kernel
headers. We can crank this up to error later.
meson: enable -Warray-bounds and -fstrict-flex-arrays
-fstrict-flex-arrays means that the compiler doesn't have to assume that any
trailing array is a flex array. I.e. unless the array is declared without a
specified size, only indices in the declared range are valid.
-Warray-bounds turns on the warnings about out-of-bounds array accesses.
-Warray-bounds=2 does some more warnings, with higher false positive rate. But
it doesn't seem to yield any false positives in our codebase, so enable it.
clang supports -Warray-bounds, but not -Warray-bounds=2.
gcc supports both.
gcc-13 supports -fstrict-flex-arrays.
See https://people.kernel.org/kees/bounded-flexible-arrays-in-c for a long
discussion of use in the kernel.
I tried to use DECLARE_FLEX_ARRAY like the kernel does, but it does not work
for anonymous structs (they cannot be declared inline), so an open-coded
version is used.
[2/3] Compiling C object systemd-repart.p/src_partition_repart.c.o
../src/partition/repart.c: In function ‘context_open_copy_block_paths’:
../src/partition/repart.c:5194:41: warning: ‘devno’ may be used uninitialized [-Wmaybe-uninitialized]
5194 | source_fd = r = device_open_from_devnum(S_IFBLK, devno, O_RDONLY|O_CLOEXEC|O_NONBLOCK, &opened);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/partition/repart.c:5188:31: note: ‘devno’ was declared here
5188 | dev_t devno;
| ^~~~~
This is with gcc-13.0.1-0.2.fc38.x86_64, -O2. I'm pretty sure the code
is correct. I also tried adding some asserts where errno is used for the return
value, but that didn't help. I think resolve_copy_blocks_auto() is just too long
for gcc to understand.
test: drop whitespace after shell redirection operators
(The one case that is left unchanged is '< <(subcommand)'.)
This way, the style with no gap was already dominant. This way, the reader
immediately knows that ' < ' is a comparison operator and ' << ' is a shift.
In a few cases, replace custom EOF replacement by just EOF. There is no point
in using someting like "_EOL" unless "EOF" appears in the text.
Jan Macku [Fri, 3 Feb 2023 09:25:51 +0000 (10:25 +0100)]
ci: Fix Development Freeze Automation
Due to the limitation of `GITHUB_TOKEN` when running workflows from forks,
it's required to split the `development_freeze` workflow in two.
* First workflow will run on the `pull_request` trigger and save the PR
number in the artifact. This workflow is running with read-only permissions
on `GITHUB_TOKEN`.
* Second workflow will get triggered on `workflow_run`. It will be run
directly in the `systemd/systemd` context and can get permission to be
able to create comments on PR.
If the input timestamp is too old (say, 1min since 1970-01-01), then
parse_timestamp() may fail on a timezone with positive shift e.g.
JST (UTC+9). Moreover, even if parse_timestamp() succeeds, its result
'y' and 'usec_sub_unsigned(x, 2 * USEC_PER_DAY)' are both zero, and
the assertion will be triggered.
docs: tweak rsync flags for moving existing home dir to systemd-homed
The documentation on moving an existing homedir into a systemd-homed managed
one suggests using rsync(1) with a bunch of flags to preserve as much metadata
as possible: permissions, xattrs, timestamps, etc. The previously suggested
flags were:
rsync -aHAXv --remove-source-files …
… which does include mtimes, but not ctimes and atimes, because -a does not
include those:
--archive, -a archive mode is -rlptgoD (no -A,-X,-U,-N,-H)
This change adds the -N and -U flags to preserve even more file timestamps,
turning the command into:
rsync -aHANUXv --remove-source-files …
The new flags are:
--crtimes, -N preserve create times (newness)
--atimes, -U preserve access (use) times
Dan Streetman [Tue, 24 Jan 2023 00:52:56 +0000 (19:52 -0500)]
tpm2: use ref counter for Tpm2Context
This will be used by Tpm2Handle instances, which is added in later patches.
The refcounting allows the context to be retained until all Tpm2Handles have
been cleaned up, and the initial ref is released, before cleaning the context.
Dan Streetman [Fri, 9 Dec 2022 22:20:24 +0000 (17:20 -0500)]
tpm2: rename tpm2 alg id<->string functions
The 'pcr_bank' functions operate on hash algs, and are not specific to the PCR
banks, while the 'primary_alg' functions operate on asymmetric algs, and are
not specific to primary keys.
Jan Janssen [Tue, 31 Jan 2023 17:53:28 +0000 (18:53 +0100)]
ci: Test with secure boot enabled under mkosi
This gives us some nice test coverage for secure boot enrolling and the
stub secure boot workound. The authenticated EFI variables are already
created by mkosi, all we need to do is request secure boot to be used.