We have code like `size_t max_size = sizeof(*private)` in three
places. This is evaluated at compile time so its safe to use. However
the new pointer-deref checker in coccinelle is not smart enough to know
this and will flag those as errors. To avoid these false positives
we have some options:
1. Reorder so that we do:
```C
size_t max_size = 0;
assert(private);
max_size = sizeof(*private);
```
2. Use something like `size_t max_size = sizeof(*ASSERT_PTR(private));`
3. Place the assert before the declaration
4. Workaround coccinelle via SIZEOF(*private) that we can then hide
via parsing_hacks.h
5. Fix coccinelle (OCaml, hard)
6. ... somehting I missed?
None of these is very appealing. I went for (4) but happy about
suggestions.
We have a test failure where the testsuite is calling
sd_varlink_idl_parse() with *ret being NULL. This is now an
assert error. So we could either fix the test or fix the code
Given that it seems genuinely useful to run sd_varlink_idl_parse()
without *ret to e.g. just check if the idl is valid I opted to
fix the code.
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.
This commit runs the check-coccinelle checker scripts with the
parsing_hacks.h. Because this was missing before there were some
issues that did not get flagged.
While at it it also adds some missing cleanup attributes and
iterators to get better results. Its a bit sad that there is no
(easy/obvious) way to detect when new things are needed for
parsing_hacks.h
When JSON_STREAM_INPUT_SENSITIVE is set before the first read,
input_buffer is NULL, input_buffer_size is 0, and input_buffer_index
is 0. The old condition '!INPUT_SENSITIVE && index == 0' would route
this case into the else branch which calls memcpy() with a NULL source
pointer, which is undefined behavior even when the length is zero, and
is caught by UBSan.
Fix by checking input_buffer_index == 0 first, then allowing the
GREEDY_REALLOC fast path also when input_buffer_size == 0, since
there is no sensitive data to protect from realloc() copying in that
case. The else branch is now only entered when there is actual data
to copy (input_buffer_size > 0), guaranteeing input_buffer is
non-NULL.
Follow-up for 6b1a59d594
When a service is configured with Delegate=yes and DelegateSubgroup=sub,
the delegated container may write domain controllers (e.g. "pids") into the
service cgroup's cgroup.subtree_control via its cgroupns root. On container
exit the stale controllers remain, and on service restart clone3() with
CLONE_INTO_CGROUP fails with EBUSY because placing a process into a cgroup
that has domain controllers in subtree_control violates the no-internal-
processes rule. The same issue affects systemctl clean, where cg_attach()
fails with EBUSY for the same reason.
Add unit_cgroup_disable_all_controllers() helper in cgroup.c that clears
stale controllers via cg_enable(mask=0) and updates cgroup_enabled_mask to
keep internal tracking in sync. Call it from service_start() and
service_clean() right before spawning, so that resource control is preserved
for any lingering processes from the previous invocation as long as possible.
Introduces JsonStream, a generic transport layer for JSON-line message
exchange over a pair of file descriptors. It owns the input/output
buffers, SCM_RIGHTS fd passing, the deferred output queue, the
read/write/parse step functions, sd-event integration (input/output/time
event sources), the idle timeout machinery, and peer credential caching,
but knows nothing about the specific JSON protocol on top — the consumer
drives its state machine via phase/dispatch callbacks supplied at
construction.
sd-varlink is reworked to delegate the entire transport layer to a
JsonStream owned by sd_varlink. The varlink struct drops every
transport-related field (input/output buffers and fds, output queue,
fd-passing state, ucred/pidfd cache, prefer_read/write fallback, idle
timeout, description, event sources) — all of that lives in JsonStream
now. What remains in sd_varlink is the varlink-protocol state machine
(state, n_pending, current/previous/sentinel, server linkage, peer
credentials accounting, exec_pidref, the varlink-specific quit and defer
sources) and a thin wrapper layer over the JsonStream API. The
should_disconnect / get_timeout / get_events / wait helpers all live in
JsonStream now and are driven by a JsonStreamPhase the consumer reports
via its phase callback.
With yeswehack.com suspended due to funding issues for triagers being
worked out, reports on GH are starting to pile up. Explicitly define
some ground rules to avoid noise and time wasting.
This is similar to DEFINE_POINTER_ARRAY_FREE_FUNC, but one
pointer chase less. The name of the outer and inner functions are
specified separately. The inner function does not free, so it'll
be generally something like 'foo_done', but the outer function
does free, so it can be called 'foo_array_free'.
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.
Replace the open-coded netlink_message_unref_many() function and its
DEFINE_TRIVIAL_CLEANUP_FUNC wrapper with DEFINE_ARRAY_DONE_FUNC.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
This is a helper macro that defines a function to drop elements of an
array but not the array itself. I used the "_many" suffix because it
most closely matches what happens here: we are calling the cleanup
function a bunch of times.
Otherwise you run into errors such as:
"""
../meson.build:2899:28: ERROR: File src/test/test-loop-util.c does not exist.
"""
when deleting a file in git without staging the deletion.
newa(t, n) already allocates sizeof(t) * n bytes, so previously we'd
actually allocate sizeof(t) * sizeof(t) * n bytes, which is ~16x more
(on x86_64) that we actually needed.
This is probably an oversight from a tree-wide change in
6e9417f5b4 that replaced alloca() with
newa().
Follow-up for 6e9417f5b4.
The shutdown interface is currently only exposed via dbus. This PR
adds a comparable varlink implementation. It is inspired by the existing
dbus methods and implements PowerOff, Reboot, Halt, Kexec, SoftReboot
as varlink methods on a new io.systemd.Shutdown interface.
It is (intentional) simpler than the dbus for now, i.e. no Can* yet,
mostly
because I want to get feedback first (happy to do that in a followup).
The only real difficulty here is what to do about
verify_shutdown_creds()
as this is needed by both dbus and varlink and its dbus only. I went for
an ugly but (hopefully) pragmatic choice (see the commit message for
details). But I can totally understand if a refactor instead is
preferred.
This error causes the computer to pass the emergency.target and go to
graphical.target.
Then, your window manager will have problems because it cant access any
directories, your network manager wont startup the network. In my case,
the screen just goes black. Ideally, you'd get an error message
explaining this edge scenario that's occuring to you, and an emergency
shell that makes it easy to run the necessary chmod 0755 / to proceed
with booting. IDK not sure if this is the correct way to implement this,
sorry it's my first contribution.
I ran
`meson test -C build`
and got
Ok: 1806
Fail: 26
Skipped: 25
on my cloned systemd repo before any changes, and got the same result
after my commit ¯\_(ツ)_/¯
So I hope I did that right.
Thanks
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.
dnssec_rsa_verify_raw() asserts that RSA_size(key) matches the RRSIG
signature size, and dnssec_ecdsa_verify_raw() asserts that
EC_KEY_check_key() succeeds. Both conditions depend on parsed DNS
record content. Replace with proper error returns.
The actual crypto verify calls (EVP_PKEY_verify / ECDSA_do_verify)
handle mismatches fine on their own, so the asserts were also redundant.
While at it, fix the misleading "EC_POINT_bn2point failed" log message
that actually refers to an EC_KEY_set_public_key() failure.
Fixes: https://github.com/systemd/systemd/issues/41569
Several issues were identified from analyzing logs of a large (52-commit) PR
review:
- Claude was batching multiple commits into a single review agent instead of
one per worktree. Strengthen the prompt to explicitly prohibit grouping.
- Claude was reading pr-context.json and commit messages before spawning
agents despite instructions not to, wasting time. Tighten the pre-spawn
rules to only allow listing worktrees/ and reading review-schema.json.
- Subagents were spawned with model "sonnet" instead of "opus". Add explicit
instruction to use opus.
- After agents returned, Claude spent 9 minutes re-verifying findings with
bash/grep/sed commands, duplicating the agents' work. Add instruction to
trust subagent findings and only read pr-context.json in phase 2.
- Subagents returned markdown-wrapped JSON instead of raw JSON arrays. Add
instruction requiring raw JSON output only.
- Each subagent was independently reading review-schema.json. Instead have
the main agent read it once and paste it into each subagent prompt.
- The "drop low-confidence findings" instruction was being used to justify
dropping findings that Claude itself acknowledged as valid ("solid cleanup
suggestions", "reasonable consistency improvement"). Remove the instruction.
- Simplify the deduplication instructions
- Stop adding the severity to the body in the post processing job as claude is
also adding it so they end up duplicated.
manager_set_dns_server() and dns_server_flush_cache() call dns_cache_flush()
unconditionally, wiping the entire cache even when StaleRetentionSec is
configured. This defeats serve-stale by discarding cached records that should
remain available during server switches and feature-level re-probes.
The original serve-stale commit (5ed91481ab) added a stale_retention_usec
guard to link_set_dns_server(), and a later commit (7928c0e0a1) added the
same guard to dns_delegate_set_dns_server(), but these two call sites in
resolved-dns-server.c were missed.
This is particularly visible with DNSOverTLS, where TLS handshake failures
trigger frequent feature-level downgrades and re-probes via
dns_server_flush_cache(), flushing the cache each time.
Add the same stale_retention_usec guard to both call sites so that cache
entries are allowed to expire naturally via dns_cache_prune() when
serve-stale is enabled.
Fixes: #40781
This commit was prepared with assistance from an AI coding agent (GitHub
Copilot). All changes have been reviewed for correctness and adherence to the
systemd coding style.
Add CpuUsage, MemoryUsage, IOReadBytes, IOReadOperations, and
TasksCurrent in a standalone socket-activated varlink service.
The new systemd-report-cgroup 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
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.
With the old version there was a potential connection count leak if
either of the two hashmap operations in count_connection() failed. In
that case we'd return from sd_varlink_server_add_connection_pair()
_before_ attached the sd_varlink_server object to an sd_varlink object,
and since varlink_detach_server() is the only place where the connection
counter is decremented (called through sd_varlink_close() in various
error paths later _if_ the "server" object is not null, i.e. attached to
the sd_varlink object) we'd "leak" a connection every time this
happened. However, the potential of abusing this is very theoretical,
as one would need to hit OOM every time either of the hashmap operations
was executed for a while before exhausting the connection limit.
Let's just increment the connection counter after any potential error
path, so we don't have to deal with potential rollbacks.
The check compared bLength against (size - sizeof(descriptor)), which
is an absolute limit unrelated to the current buffer position. Since
bLength is uint8_t (max 255), this can never exceed size - 9 for any
realistic input, making the check dead code.
Use (size - pos) instead so the check actually catches descriptors
that extend past the end of the read data.
Fixes: https://github.com/systemd/systemd/issues/41570
Move the push-based streaming compression API from import-compress.c
into compress.c and delete import-compress.c/h. This consolidates all
compression code in one place and makes all compression libraries
(liblzma, liblz4, libzstd, libz, libbz2) runtime-loaded via dlopen
instead of directly linked.
Introduce opaque Compressor/Decompressor types backed by a heap-
allocated struct defined only in compress.c, keeping all third-party
library headers out of compress.h.
Rewrite the per-codec fd-to-fd stream functions as thin wrappers around
the push API via generic compress_stream()/decompress_stream() taking a
Compression type parameter. Integrate LZ4 into this framework using the
LZ4 Frame API, eliminating all LZ4 special-casing.
Extend the Compression enum with COMPRESSION_GZIP and COMPRESSION_BZIP2
and add the corresponding blob, startswith, and stream functions for
both.
Rename the ImportCompress types and functions: ImportCompressType becomes
the existing Compression enum, ImportCompress becomes Compressor (with
Decompressor typedef), and all import_compress_*/import_uncompress_*
become compressor_*/decompressor_*. Rename dlopen_lzma() to dlopen_xz()
for consistency. Make compression_to_string() return lowercase by
default.
Add INT_MAX/UINT_MAX overflow checks for LZ4, zlib, and bzip2 blob
functions where the codec API uses narrower integer types than our
uint64_t parameters.
Migrate test-compress.c and test-compress-benchmark.c to the TEST()
macro framework, new assertion macros, and codec-generic loops instead
of per-codec duplication.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
The setting has fundamental flaws that can't be easily fixed
(see https://github.com/systemd/mkosi/pull/4273) so revert it's
use as we're dropping it in systemd. Image builds will take a bit
longer again until I figure out a proper fix for this.
This reverts commit 7a70c32368.
Don't run setfont(8) on consoles that don't support
fonts. systemd-vconsole-setup neither fails nor reports errors on such consoles
unlike setfont(8) which emits the following error [1]:
systemd-vconsole-setup[169]: setfont: ERROR kdfontop.c:183 put_font_kdfontop: Unable to load such font with such kernel version
The check already existed in setup_remaining_vcs() but it was performed too
late.
[1] this was simply ignored by setfont(8) until
https://github.com/legionus/kbd/commit/1e15af4d8b272ca50e9ee1d0c584c5859102c848
When there is a potential protocol upgrade we need to be careful that
we do not read beyond our json message as the custom protocol may be
anything. This was archived via a byte-by-byte read. This is of course
very inefficient. So this commit moves to use MSG_PEEK to find the
boundary of the json message instead. This makes the performance hit
a lot smaller.
Thanks to Lennart for suggesting this.
When the server side of a varlink connection supports connection
upgrades we need to go into single byte-read mode to avoid the
risk of a client that sends the json to protocol upgrade and then
immediately the custom protocol payload. This commit implements
this.
The next step is using MSG_PEEK to avoid the single-byte overhead.
This commit tweaks the API of sd_varlink_call_and_upgrade and
sd_varlink_reply_and_upgrade to return two independent fds even
if the internal {input,output}_fd are the same (e.g. a socket).
This makes the external API easier as there is no longer the risk
of double close. The sd_varlink_call_and_upgrade() is not in a
released version of systemd yet so I presume it is okay to update
it still.
This also allowed some simplifications in varlinkctl.c now that
the handling is easier.
Michael Vogt
Luca Boccassi
Nandakumar Raghavan
Frantisek Sumsal
Zbigniew Jędrzejewski-Szmek
Daan De Meyer
Lennart Poettering
Milan Kyselica
azureuser
Yaping Li
ipv6
Franck Bui