Jun 11 14:29:12 krowka systemd[1]: /etc/systemd/system/workingdir.service:6: = path is not normalizedWorkingDirectory: /../../etc
↓
Jun 11 14:32:12 krowka systemd[1]: /etc/systemd/system/workingdir.service:6: WorkingDirectory= path is not normalized: /../../etc
Since bb28e68477 parsing failures of
certain unit file settings will result in load failures of units. This
introduces a new load state "bad-setting" that is entered in precisely
this case.
With this addition error messages on bad settings should be a lot more
explicit, as we don't have to show some generic "errno" error in that
case, but can explicitly say that a bad setting is at fault.
Internally this unit load state is entered as soon as any configuration
loader call returns ENOEXEC. Hence: config parser calls should return
ENOEXEC now for such essential unit file settings. Turns out, they
generally already do.
Fixes: #9107
The load_error is only valid in some load_state cases, lets generate
prettier messages for other cases too, by reusing the
bus_unit_validate_load_state() call which does jus that.
Clients (such as systemctl) ignored LoadError unles LoadState was
"error" before. With this change they could even show LoadError in other
cases and it would show a useful name.
Let's use a switch() statement, cover more cases with pretty messages.
Also let's rename it to "validate", as that's more specific that
"check", as it implies checking for a "valid"/"good" state, which is
what this function does.
This makes two changes: first of all we will now explicitly check
whether a domain to test against an NSEC record is actually below the
signer's name. This is relevant for NSEC records that chain up the end
and the beginning of a zone: we shouldn't alow that NSEC record to match
against domains outside of the zone.
This also fixes how we handle NSEC checks for domains that are prefixes
of the NSEC RR domain itself, fixing #8164 which triggers this specific
case. The non-wildcard NSEC check is simplified for that, we can
directly make our between check, there's no need to find the "Next
Closer" first, as the between check should not be affected by additional
prefixes. For the wild card NSEC check we'll prepend the asterisk in
this case to the NSEC RR itself to make a correct check.
Fixes: #8164
oss-fuzz flags this as:
==1==WARNING: MemorySanitizer: use-of-uninitialized-value
0. 0x7fce77519ca5 in ascii_is_valid systemd/src/basic/utf8.c:252:9
1. 0x7fce774d203c in ellipsize_mem systemd/src/basic/string-util.c:544:13
2. 0x7fce7730a299 in print_multiline systemd/src/shared/logs-show.c:244:37
3. 0x7fce772ffdf3 in output_short systemd/src/shared/logs-show.c:495:25
4. 0x7fce772f5a27 in show_journal_entry systemd/src/shared/logs-show.c:1077:15
5. 0x7fce772f66ad in show_journal systemd/src/shared/logs-show.c:1164:29
6. 0x4a2fa0 in LLVMFuzzerTestOneInput systemd/src/fuzz/fuzz-journal-remote.c:64:21
...
I didn't reproduce the issue, but this looks like an obvious error: the length
is specified, so we shouldn't use the string with any functions for normal
C-strings.
This introduces a has_data boolean field in struct unit_files which can
be used to detect the end of the array.
Use a _cleanup_ for struct unit_files in acquire_time_data and its
callers. Code for acquire_time_data is also simplified by replacing
goto's with straight returns.
Tested: By running the commands below, also checking them under valgrind.
- build/systemd-analyze blame
- build/systemd-analyze critical-chain
- build/systemd-analyze plot
Fixes: Coverity finding CID 996464.
Let's add some protection for split horizon setups, where different
zones are visible on the same global DNS servers depending on where you
come from.
Fixes: #9196
This simplifies the code a bit and hopefully fixes Coverity finding
CID 1382966. There was not actually a resource leak here (Coverity
seemed to be confused by thinking log_oom() could actually return 0),
but the fix doesn't hurt and should make this code more resilient to
future refactorings.
Tested: builds fine, manually called scsi_id, seems to work ok.
This fixes an insecure use of tainted data as argument to functions that
allocate memory and read from files, which could be tricked into getting
networkctl to allocate a large amount of memory and fill it with file
data.
This was uncovered by Coverity. Fixes CID 1393254.
This should fix a leak of the allocated Prefix if sd_radv_prefix_new
fails for some reason.
The code was already initializing prefix to NULL and using TAKE_PTR to
return it, so only the _cleanup_ was missing.
Fixes Coverity finding CID 1382976.
This shouldn't be necessary, since read() should never return a size
larger than the size of the buffer passed in, but Coverity doesn't seem
to understand that.
We could possibly fix this with a model file for Coverity, but given
changing the code is not that much of a biggie, let's just do that
instead.
Fixes CID 996458: Overflowed or truncated value (or a value computed
from an overflowed or truncated value) `pos` used as array index.
Tested: `ninja -C build/ test`, builds without warnings, test cases pass.
The refactor in #9200 inadvertently dropped the variable assignment to
traverse the device and its hierarchy in add_matches_for_device().
This was uncovered by Coverity (CID #1393310).
Fix that by restoring the assignment.
Tested: `journalctl /dev/sda` now filters journalctl output again.
This is mostly fall-out from d1a1f0aaf0,
however some cases are older bugs.
There might be more issues lurking, this was a simple grep for "%m"
across the tree, with all lines removed that mention "errno" at all.
Previously the enumerate() callback defined for each unit type would do
two things:
1. It would create perpetual units (i.e. -.slice, system.slice, -.mount and
init.scope)
2. It would enumerate units from /proc/self/mountinfo, /proc/swaps and
the udev database
With this change these two parts are split into two seperate methods:
enumerate() now only does #2, while enumerate_perpetual() is responsible
for #1. Why make this change? Well, perpetual units should have a
slightly different effect that those found through enumeration: as
perpetual units should be up unconditionally, perpetually and thus never
change state, they should also not pull in deps by their state changing,
not even when the state is first set to active. Thus, their state is
generally initialized through the per-device coldplug() method in
similar fashion to the deserialized state from a previous run would be
put into place. OTOH units found through regular enumeration should
result in state changes (and thus pull in deps due to state changes),
hence their state should be put in effect in the catchup() method
instead. Hence, given this difference, let's also separate the
functions, so that the rule is:
1. What is created in enumerate_perpetual() should be started in
coldplug()
2. What is created in enumerate() should be started in catchup().
This reworks how device units are "powered on".
This makes sure that any device changes that might have happened while
we were restarting/reloading will be noticed properly. For that we'll
now properly serialize/deserialize both the device unit state and the
device "found" flags, and restore these initially in the "coldplug"
phase of the manager deserialization. While enumerating the udev devices
during startup we'll put together a new "found" flags mask, which we'll
the switch to in the "catchup" phase of the manager deserialization,
which follows the "coldplug" phase.
Note that during the "coldplug" phase no unit state change events are
generated, which is different for the "catchall" phase which will do
that. Thus we correctly make sure that the deserialized state won't pull
in new deps, but any device's change while we were reloading would.
Fixes: #8832
Replaces: #8675
This is very similar to the existing unit method coldplug() but is
called a bit later. The idea is that that coldplug() restores the unit
state from before any prior reload/restart, i.e. puts the deserialized
state in effect. The catchup() call is then called a bit later, to
catch up with the system state for which we missed notifications while
we were reloading. This is only really useful for mount, swap and device
mount points were we should be careful to generate all missing unit
state change events (i.e. call unit_notify() appropriately) for
everything that happened while we were reloading.
