XO_SOCKET asks for the opened inode to be verified as a socket, analogous to
the existing XO_REGULAR. A new fd_verify_socket() helper is added to
stat-util to perform the check on an opened fd.
XO_TRIGGER_AUTOMOUNT asks O_PATH opens to trigger automounts by going via
open_tree() without OPEN_TREE_CLONE, falling back to openat() if the kernel
or sandbox rejects open_tree().
Some flags don't reasonably apply to chase_and_unlinkat() (because we
open the parent inode of an inode to delete, which is always a dir),
hence let's catch these flags when misused.
(I ran into this, and it was very confusing to debug, hence let's make
it easier)
If a journal file contains a truncated ZSTD frame (i.e. a frame with
Frame_Content_Size > 0, but with not enough data in Data_Block),
ZSTD_decompressStream() would return a non-zero, non-error value. This
would then skip the error path in the ZSTD_isError() branch and we'd hit
the following assert:
$ build-local/journalctl -o cat --file zstd-truncated.journal
Assertion 'output.pos >= prefix_len + 1' failed at src/basic/compress.c:1236, function decompress_startswith_zstd(). Aborting.
Aborted (core dumped) build-local/journalctl -o cat --file zstd-truncated.journal
Let's handle this situation gracefully and return EBADMSG instead.
Also, add another journalctl invocation to the corrupted-journals test
that goes through the sd_journal_get_data() -> decompress_startswith_zstd()
code path which, among other things, covers the issue when run on the
provided journal file.
vsnprintf() can, under some circumstances, return negative value, namely
during encoding errors when converting wchars to multi-byte characters.
This would then wreak havoc in the arithmetics we do following the
vsnprintf() call. However, since we never do any wchar shenanigans in
our code it should never happen.
Let's encode this assumption into the code as an assert(), similarly how
we already do this in other places (like strextendf_with_separator()).
The naming is consistent with strv_extend().
This also
- introduces tiny iovw_extend_iov() wrapper,
- refuse when the source and target points to the same object,
- check the final count before extending in iovw_extend_iovw().
This is similar to iovw_to_cstring(), but allows embedded NUL, as this
just concat multiple iovec, the result may not be a string.
Now, iovw_to_cstring() internally uses iovw_concat().
Split out of #41543.
These don't make too much sense on their own, but they also don't really
hurt. They are preparation for #41543, but in order to make things
either to review I split these four commits out, since they are not
directly part of what the PR shall achieve
Suppress the following message:
```
$ sudo valgrind --leak-check=full build/networkctl dhcp-lease wlp59s0
==175708== Memcheck, a memory error detector
==175708== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==175708== Using Valgrind-3.26.0 and LibVEX; rerun with -h for copyright info
==175708== Command: build/networkctl status wlp59s0
==175708==
==175708== Conditional jump or move depends on uninitialised value(s)
==175708== at 0x4BC33D1: inode_same_at (stat-util.c:610)
==175708== by 0x4BF1972: inode_same (stat-util.h:86)
==175708== by 0x4BF48FE: running_in_chroot (virt.c:817)
==175708== by 0x4B16643: running_in_chroot_or_offline (verbs.c:37)
==175708== by 0x4B175CE: _dispatch_verb_with_args (verbs.c:136)
==175708== by 0x4B17868: dispatch_verb (verbs.c:160)
==175708== by 0x407CBB: networkctl_main (networkctl.c:249)
==175708== by 0x407D06: run (networkctl.c:263)
==175708== by 0x407D39: main (networkctl.c:266)
==175708==
```
Not sure if it is an issue in valgrind or glibc, but at least there is
nothing we can do except for working around it.
This simply mirrors the same overflow check we already have in
strrepa(), in case someone passed us a sufficiently long string.
strrep() is currently used only in tests, so this is just hardening.
This simply mirrors the same overflow check we already have in
strrepa(), in case someone passed us a sufficiently long string.
strrep() is currently used only in tests, so this is just hardening.
All non-test users iovec_wrapper define the struct as a field in a
bigger structure, so we never free it individually. Let's simplify the
code and assume it is never null.
I promised in https://github.com/systemd/systemd/pull/41426 that its the
final update for coccinelle pointer deref checks. However it turned out
there is this coccinelle/parsing_hacks.h that I wasn't aware of. The
file missed some important things like _cleanup_(x) that prevented
coccinelle to check a bunch of functions.
This PR adds some missing defines to the parsing_hacks.h and fixes the
missing asserts(). I apologize that its a bit long (and frankly boring)
and that I missed this earlier.
The last commit contains one small behavior change (ret in
sd_varlink_idl_parse() is now really optional) but the big one is very
mechanical.
Check S_ISDIR() before S_ISLINK() for all stat_verify_xyz() helpers
first, where we check them. Just to ensure we systematically return the
same errors.
We already have checks in place during compression that limit the data
we compress, so they shouldn't decompress to anything larger than
DATA_SIZE_MAX unless they've been tampered with. Let's make this
explicit and limit all our decompress_blob() calls in journal-handling
code to that limit.
One possible scenario this should prevent is when one tries to open and
verify a journal file that contains a compression bomb in its payload:
```
$ ls -lh test.journal
-rw-rw-r--+ 1 fsumsal fsumsal 1.2M Apr 12 15:07 test.journal
$ systemd-run --user --wait --pipe -- build-local/journalctl --verify --file=$PWD/test.journal
Running as unit: run-p682422-i4875779.service
000110: Invalid hash (00000000 vs. 11e4948d73bdafdd)
000110: Invalid object contents: Bad message
File corruption detected at /home/fsumsal/repos/@systemd/systemd/test.journal:272 (of 1249896 bytes, 0%).
FAIL: /home/fsumsal/repos/@systemd/systemd/test.journal (Bad message)
Finished with result: exit-code
Main processes terminated with: code=exited, status=1/FAILURE
Service runtime: 48.051s
CPU time consumed: 47.941s
Memory peak: 8G (swap: 0B)
```
Same could be, in theory, possible with just `journalctl --file=`, but
the reproducer would be a bit more complicated (haven't tried it, yet).
Lastly, the change in journal-remote is mostly hardening, as the maximum
input size to decompress_blob() there is mandated by MHD's connection
memory limit (set to JOURNAL_SERVER_MEMORY_MAX which is 128 KiB at the
time of writing), so the possible output size there is already quite
limited (e.g. ~800 - 900 MiB for xz-compressed data).
Replace the duplicated magic byte signatures in decompressor_detect()
with a call to the new compression_detect_from_magic() helper and use a
switch statement to initialize the appropriate decompression context.
We generally assume that valid times returned by clock_gettime() are > 0
and < USEC_INFINITY. If this wouldn't hold all kinds of things would
break, because we couldn't distuingish our niche values from regular
values anymore.
Let's hence encode our assumptions in C, already to help static
analyzers and LLMs.
Inspired by: https://github.com/systemd/systemd/pull/41601#pullrequestreview-4094645891
The updated parsing_hacks.h file uncovered a bunch of extra
things that the check-pointer-deref coccinelle script flags.
This commit fixes them to make the tree check-pointer-deref clean.
Coverity flags range->n_entries - j as a potential underflow
in the memmove size calculation. Add assert(range->n_entries > 0)
before decrementing n_entries, which holds since the loop condition
guarantees j < n_entries.
CID#1548015
Follow-up for 8dcc66cefc
As mentioned in the grandfather commit, I want to use the _many
suffix for freeing of the contents of an array, so the functions
to free the array to get the suffix _array.
Add CpuUsage, MemoryUsage, IOReadBytes, IOReadOperations, and
TasksCurrent in a standalone socket-activated varlink service. These
metrics are gathered from the kernel via cgroup files and PID1's only
role is mapping unit names to cgroup paths — a separate process can
query PID1 once for that mapping and then read the cgroup files
directly, minimizing PID1 involvement.
The new systemd-report-cgroup-metrics service listens at
/run/systemd/report/io.systemd.CGroup and exposes:
- io.systemd.CGroup.CpuUsage
- io.systemd.CGroup.IOReadBytes
- io.systemd.CGroup.IOReadOperations
- io.systemd.CGroup.MemoryUsage (with type=current/available/peak)
- io.systemd.CGroup.TasksCurrent
This is spun out of #41078 and based on top of it. Will rebase once
that's merged.
Multiple callers of cg_get_keyed_attribute() follow the same pattern of
reading a single keyed attribute and then parsing it as uint64_t with
safe_atou64(). Add a helper that combines both steps.
Convert all existing single-key + uint64 call sites in cgtop, cgroup.c,
and oomd-util.c to use the new helper.
Daan De Meyer
Lennart Poettering
Frantisek Sumsal
Christian Brauner
Yu Watanabe
Zbigniew Jędrzejewski-Szmek
Ivan Kruglov
Michael Vogt
Luca Boccassi
Yaping Li