Let's replace usage of fputc_unlocked() and friends by __fsetlocking(f,
FSETLOCKING_BYCALLER). This turns off locking for the entire FILE*,
instead of doing individual per-call decision whether to use normal
calls or _unlocked() calls.
This has various benefits:
1. It's easier to read and easier not to forget
2. It's more comprehensive, as fprintf() and friends are covered too
(as these functions have no _unlocked() counterpart)
3. Philosophically, it's a bit more correct, because it's more a
property of the file handle really whether we ever pass it on to another
thread, not of the operations we then apply to it.
This patch reworks all pieces of codes that so far used fxyz_unlocked()
calls to use __fsetlocking() instead. It also reworks all places that
use open_memstream(), i.e. use stdio FILE* for string manipulations.
Note that this in some way a revert of 4b61c87511.
Let's always escape strings we receive from the user before writing them
out to unit file settings that suppor specifier expansion, so that user
strings are transported as-is.
remote-cryptsetup-pre.target was designed as an active unit (that pulls in
network-online.target), the opposite of remote-fs-pre.target (a passive unit,
with individual provider services ordering itself before it and pulling it in,
for example iscsi.service and nfs-client.target).
To make remote-cryptsetup-pre.target really work, those services should be
ordered before it too. But this would require updates to all those services,
not just changes from systemd side.
But the requirements for remote-fs-pre.target and remote-cryptset-pre.target
are fairly similar (e.g. iscsi devices can certainly be used for both), so
let's reuse remote-fs-pre.target also for remote cryptsetup units. This loses
a bit of flexibility, but does away with the requirement for various provider
services to know about remote-cryptsetup-pre.target.
We want that cryptsetup can cache keys between multiple invocations, and
it does so via the root user's user keyring, hence let's share it among
services.
Replaces: #6286
This breaks things when the decrypted device is not immediately
`SYSTEMD_READY=1` (e. g. when a multi-device btrfs system is placed on
multiple cryptsetup devices).
Fixes#6537.
As a follow-up for db3f45e2d2 let's do the
same for all other cases where we create a FILE* with local scope and
know that no other threads hence can have access to it.
For most cases this shouldn't change much really, but this should speed
dbus introspection and calender time formatting up a bit.
It seems that there's a common pattern among the various generators. Let's add
a helper function for it and make use of it in cryptsetup-generator.
This fixes a bunch of theoretical memleaks in error paths, since *to wasn't
generally freed properly. Not thath it matters.
If the cryptsetup service unit and swap unit for a swap device
are not strictly ordered, it might happen that the swap unit
activates/mounts the swap device before its cryptsetup service unit
has a chance to run the 'mkswap' command (that it is programmed to).
This leads to the following error:
Starting Cryptography Setup for sda3_crypt...
[ OK ] Found device /dev/mapper/sda3_crypt.
Activating swap /dev/mapper/sda3_crypt...
[ OK ] Activated swap /dev/mapper/sda3_crypt.
[ OK ] Reached target Swap.
[FAILED] Failed to start Cryptography Setup for sda3_crypt.
See 'systemctl status systemd-cryptsetup@sda3_crypt.service' for
details.
[DEPEND] Dependency failed for Encrypted Volumes.
Which happens because the swap device is already mounted:
# systemctl status systemd-cryptsetup@sda3_crypt.service
<...>
Active: failed (Result: exit-code) since Mon 2017-02-27 14:21:43 CST;
54s ago
<...>
<...> systemd[1]: Starting Cryptography Setup for sda3_crypt...
<...> mkswap[2420]: mkswap: error: /dev/mapper/sda3_crypt is mounted;
will not make swapspace
<...>
So, modify cryptsetup-generator to include a 'Before=' option for the
respective 'dev-mapper-%i.swap' device in the cryptsetup service unit.
Now, correct ordering is ensured, and the error no longer occurs:
Starting Cryptography Setup for sda3_crypt...
[ OK ] Found device /dev/mapper/sda3_crypt.
[ OK ] Started Cryptography Setup for sda3_crypt.
Activating swap /dev/mapper/sda3_crypt...
[ OK ] Reached target Encrypted Volumes.
[ OK ] Activated swap /dev/mapper/sda3_crypt.
[ OK ] Reached target Swap.
This improves kernel command line parsing in a number of ways:
a) An kernel option "foo_bar=xyz" is now considered equivalent to
"foo-bar-xyz", i.e. when comparing kernel command line option names "-" and
"_" are now considered equivalent (this only applies to the option names
though, not the option values!). Most of our kernel options used "-" as word
separator in kernel command line options so far, but some used "_". With
this change, which was a source of confusion for users (well, at least of
one user: myself, I just couldn't remember that it's systemd.debug-shell,
not systemd.debug_shell). Considering both as equivalent is inspired how
modern kernel module loading normalizes all kernel module names to use
underscores now too.
b) All options previously using a dash for separating words in kernel command
line options now use an underscore instead, in all documentation and in
code. Since a) has been implemented this should not create any compatibility
problems, but normalizes our documentation and our code.
c) All kernel command line options which take booleans (or are boolean-like)
have been reworked so that "foobar" (without argument) is now equivalent to
"foobar=1" (but not "foobar=0"), thus normalizing the handling of our
boolean arguments. Specifically this means systemd.debug-shell and
systemd_debug_shell=1 are now entirely equivalent.
d) All kernel command line options which take an argument, and where no
argument is specified will now result in a log message. e.g. passing just
"systemd.unit" will no result in a complain that it needs an argument. This
is implemented in the proc_cmdline_missing_value() function.
e) There's now a call proc_cmdline_get_bool() similar to proc_cmdline_get_key()
that parses booleans (following the logic explained in c).
f) The proc_cmdline_parse() call's boolean argument has been replaced by a new
flags argument that takes a common set of bits with proc_cmdline_get_key().
g) All kernel command line APIs now begin with the same "proc_cmdline_" prefix.
h) There are now tests for much of this. Yay!
This stripping is contolled by a new boolean parameter. When the parameter
is true, it means that the caller does not care about the distinction between
initrd and real root, and wants to act on both rd-dot-prefixed and unprefixed
parameters in the initramfs, and only on the unprefixed parameters in real
root. If the parameter is false, behaviour is the same as before.
Changes by caller:
log.c (systemd.log_*): changed to accept rd-dot-prefix params
pid1: no change, custom logic
cryptsetup-generator: no change, still accepts rd-dot-prefix params
debug-generator: no change, does not accept rd-dot-prefix params
fsck: changed to accept rd-dot-prefix params
fstab-generator: no change, custom logic
gpt-auto-generator: no change, custom logic
hibernate-resume-generator: no change, does not accept rd-dot-prefix params
journald: changed to accept rd-dot-prefix params
modules-load: no change, still accepts rd-dot-prefix params
quote-check: no change, does not accept rd-dot-prefix params
udevd: no change, still accepts rd-dot-prefix params
I added support for "rd." params in the three cases where I think it's
useful: logging, fsck options, journald forwarding options.
There are more than enough calls doing string manipulations to deserve
its own files, hence do something about it.
This patch also sorts the #include blocks of all files that needed to be
updated, according to the sorting suggestions from CODING_STYLE. Since
pretty much every file needs our string manipulation functions this
effectively means that most files have sorted #include blocks now.
Also touches a few unrelated include files.
A variety of changes:
- Make sure all our calls distuingish OOM from other errors if OOM is
not the only error possible.
- Be much stricter when parsing escaped paths, do not accept trailing or
leading escaped slashes.
- Change unit validation to take a bit mask for allowing plain names,
instance names or template names or an combination thereof.
- Refuse manipulating invalid unit name
This file contains no privileged data — just names of devices to decrypt
and files containing keys. On a running system most of this can be inferred from
the device tree anyway.
This patch removes includes that are not used. The removals were found with
include-what-you-use which checks if any of the symbols from a header is
in use.
After all it is now much more like strjoin() than strappend(). At the
same time, add support for NULL sentinels, even if they are normally not
necessary.
We would ignore options like "fail" and "auto", and for any option
which takes a value the first assignment would win. Repeated and
options equivalent to the default are rarely used, but they have been
documented forever, and people might use them. Especially on the
kernel command line it is easier to append a repeated or negated
option at the end.
If the format string contains %m, clearly errno must have a meaningful
value, so we might as well use log_*_errno to have ERRNO= logged.
Using:
find . -name '*.[ch]' | xargs sed -r -i -e \
's/log_(debug|info|notice|warning|error|emergency)\((".*%m.*")/log_\1_errno(errno, \2/'
Plus some whitespace, linewrap, and indent adjustments.
As a followup to 086891e5c1 "log: add an "error" parameter to all
low-level logging calls and intrdouce log_error_errno() as log calls
that take error numbers", use sed to convert the simple cases to use
the new macros:
find . -name '*.[ch]' | xargs sed -r -i -e \
's/log_(debug|info|notice|warning|error|emergency)\("(.*)%s"(.*), strerror\(-([a-zA-Z_]+)\)\);/log_\1_errno(-\4, "\2%m"\3);/'
Multi-line log_*() invocations are not covered.
And we also should add log_unit_*_errno().
Fix a bug in systemd-cryptsetup-generator which caused the drop-in
setting the job timeout for the dm device unit to be written with a
name different than the unit name.
https://bugs.freedesktop.org/show_bug.cgi?id=84409
As special magic, don't create device dependencies for /dev/null. Of
course, there might be similar devices we might want to include, but
given that none of them really make sense to specify as password source
there's really no point in checking for anything else here.
https://bugs.freedesktop.org/show_bug.cgi?id=75816
