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.
This is a followup to https://github.com/systemd/systemd/pull/41400 with
the final set of tweaks so that the new coccinelle `check-pinters-deref`
checker runs without failures.
It also includes a commit with some fixes for redundant asserts and the
final commit to make the assert() in the inner loop of qsort
POINTER_MAY_BE_NULL which will become a no-op - that is probably the
most controversial one, I hope the commit message explains the
trade-offs, I'm happy to drop it, I have no strong opinion either way.
Please see the individual commits/commit messages for more details, most
is (hopefully) relatively boring/mechanical.
When running in a console mode (interactive, native, or read-only),
propagate the host's $TERM into the VM by adding TERM= and
systemd.tty.term.hvc0= to the kernel command line.
TERM= is picked up by PID 1 and inherited by services on /dev/console
(such as emergency.service). systemd.tty.term.hvc0= is used by services
directly attached to /dev/hvc0 (such as serial-getty@hvc0.service) which
look up $TERM via the systemd.tty.term.<tty> kernel command line
parameter.
While systemd can auto-detect the terminal type via DCS XTGETTCAP, not
all terminal emulators implement this, so explicitly propagating $TERM
provides a more reliable experience. We skip propagation when $TERM is
unset or set to "unknown" (as is the case in GitHub Actions and some
other CI environments).
Previously this was handled by mkosi synthesizing the corresponding
kernel command line parameters externally.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
The network generator currently supports many of the options described
by dracut.cmdline(7), but not everything.
This commit adds support for the BOOTIF= option (and the related
rd.bootif= option) used in PXE setups.
This is implemented by treating BOOTIF as a special name/placeholder
when used as an interface name, and expecting a MAC address to be set in
the BOOTIF= parameter. The resulting .network file then uses MACAddress=
in the [Match] section, instead of Name=.
terminal_prepare_query() is called from terminal_get_size() which
operates on an explicitly passed fd — typically /dev/console opened
directly by PID 1 via reset_dev_console_fd(), or a service's TTY via
exec_context_apply_tty_size(). Using terminal_is_dumb() here is wrong
because it additionally checks on_tty(), which tests whether stderr is
a tty. PID 1's stderr may not be a tty (e.g. connected to kmsg or the
journal), causing terminal_is_dumb() to return true and skip the ANSI
query even though the fd we're operating on is a perfectly functional
terminal.
Use getenv_terminal_is_dumb() instead, which only checks $TERM, matching
what terminal_reset_ansi_seq() already does.
Also use it in terminal_get_cursor_position(), which also receives fds
to operate on.
terminal_get_size() writes ANSI escape sequences (CSI 18 and DSR
queries) to the output fd to determine terminal dimensions. This is
called during early boot via reset_dev_console_fd() and from service
execution contexts via exec_context_apply_tty_size().
Previously, these writes used loop_write() on a blocking fd, which
could block indefinitely if the terminal is not consuming data — for
example on a serial console with flow control asserted, or a
disconnected terminal. This is the same problem that was solved for
terminal_reset_ansi_seq() in systemd/systemd#32369 by temporarily
setting the fd to non-blocking mode with a write timeout.
Apply the same pattern here: set the output fd to non-blocking in
terminal_get_size() before issuing the queries, and restore blocking
mode afterward. Change the loop_write() calls in
terminal_query_size_by_dsr() and terminal_query_size_by_csi18() to
loop_write_full() with a 100ms timeout so writes fail gracefully
instead of hanging.
Also introduce the CONSOLE_ANSI_SEQUENCE_TIMEOUT_USEC constant for all
timeouts used across all ANSI sequence writes and reads (vt_disallocate(),
terminal_reset_ansi_seq(), and the two size query functions). 333ms
is now used for all timeouts in terminal-util.c.
Also introduce a cleanup function for resetting a fd back to blocking
mode after it was made non-blocking.
Switch runtime virtiofs mount configuration from systemd.mount-extra=
kernel command line parameters to the fstab.extra credential. This
avoids consuming kernel command line space (which is limited) and
matches the approach used by mkosi.
Each mount is added as an fstab entry in the format:
{tag} {destination} virtiofs {ro|rw},x-initrd.mount
If the user already specified a fstab.extra credential via
--set-credential= or --load-credential=, the virtiofs mount entries
are appended to it rather than conflicting.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
hmac_sha256() leaves four stack buffers containing key-derived material
(inner_padding, outer_padding, replacement_key, hash state) on the stack
after returning. The inner_padding and outer_padding arrays contain
key XOR 0x36 and key XOR 0x5c respectively, which are trivially
reversible to recover the original HMAC key.
This function is called with security-sensitive keys including the LUKS
volume key (cryptsetup-util.c), TPM2 PIN (tpm2-util.c), and boot secret
(tpm2-swtpm.c). The key material persists on the stack until overwritten
by later unrelated function calls.
Add CLEANUP_ERASE() to all four local buffers, following the same
pattern applied to tpm2-util.c in commit 6c80ce6 (PR #41394).
Some functions are in the hot path of a sort. While it is more
correct to also use assert() here for the passed pointers it is
also a runtime cost that should never be needed when e.g. qsort()
is used as it will always pass non-NULL pointers.
So for these critical functions use POINTER_MAY_BE_NULL() which
is compiled out and add a comment to them. Not all _compare()
functions are changed, it was a bit of a judgement call because
some (like ether_addr_compare) are not called with that much data.
But happy to get advise here and tweak accordingly.
Add the needed assert changes to make the code clean
for the new check-pointer-deref script.
Note that for `_cmp()` functions its a bit of a question
if we want `assert()` or `POINTER_MAY_BE_NULL()` here. The
former is more correct but it adds a (small) runtime hit. If
we use POINTER_MAY_BE_NULL() it is compiled out but strictly
speaking it is wrong (so at least we would need a comment).
Since v257, terminal_fix_size() is called during early boot via
console_setup() → reset_dev_console_fd() to query terminal dimensions
via ANSI escape sequences. This has caused intermittent boot hangs
where the system gets stuck with a blinking cursor and requires a
keypress to continue (see systemd/systemd#35499).
The function tries CSI 18 first, then falls back to DSR if that fails.
Previously, each method independently opened a non-blocking fd, disabled
echo/icanon, ran its query, restored termios, and closed its fd. This
created two problems:
1. Echo window between CSI 18 and DSR fallback: After CSI 18 times out
and restores termios (re-enabling ECHO and ICANON), there is a brief
window before DSR disables them again. If the terminal's CSI 18
response arrives during this window, it is echoed back to the
terminal — where the terminal interprets \e[8;rows;cols t as a
"resize text area" command — and the response bytes land in the
canonical line buffer as stale input that can confuse the DSR
response parser.
2. Cursor left at bottom-right on DSR timeout: The DSR method worked by
sending two DSR queries — one to save the cursor position, then
moving the cursor to (32766,32766) and sending another to read the
clamped position. If neither response was received (timeout), the
cursor restore was skipped (conditional on saved_row > 0), leaving
the cursor at the bottom-right corner of the terminal. The
subsequent terminal_reset_ansi_seq() then moved it to the beginning
of the last line via \e[1G, making boot output appear at the bottom
of the screen — giving the appearance of a hang even when the system
was still booting.
This commit fixes both issues:
- terminal_fix_size() now opens the non-blocking fd and configures
termios once for both query methods, so echo stays disabled for the
entire CSI 18 → DSR fallback sequence with no gap. tcflush(TCIFLUSH)
is called before each query to drain any stale input from the tty
input queue.
- The DSR method now uses DECSC (\e7) / DECRC (\e8) to save and restore
the cursor position via hardware, instead of querying it with a
separate DSR round-trip. All four sequences (DECSC, CUP to
bottom-right, DSR query, DECRC) are sent in a single write, so the
terminal processes DECRC and restores the cursor regardless of whether
userspace ever reads the DSR response. This eliminates the
cursor-at-bottom-right artifact on timeout and simplifies the read
loop to only need a single DSR response instead of two.
- The repeated setup boilerplate (dumb check, verify_same, fd_reopen,
termios save/disable) is extracted into terminal_prepare_query(),
shared by terminal_get_size_by_csi18(), terminal_get_size_by_dsr(),
and terminal_fix_size().
Fixes: systemd/systemd#35499
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Add TERMIOS_NULL sentinel, TermiosResetContext, and CLEANUP_TERMIOS_RESET()
macro (modeled after CLEANUP_ARRAY()) to automatically restore terminal
settings when leaving scope, replacing manual goto+tcsetattr patterns.
Migrate ask_string_full(), terminal_get_cursor_position(),
get_default_background_color(), terminal_get_terminfo_by_dcs(),
terminal_get_size_by_dsr() and terminal_get_size_by_csi18() to use the new
cleanup macro, removing the goto-based cleanup labels and replacing them
with direct returns.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Coverity flags (uint64_t) sc * (uint64_t) ps as a potential
overflow. Add an assert to make the bounds explicit for static
analyzers.
CID#1548042
Follow-up for eefc66aa8f
Coverity flags ALIGN(sizeof(struct passwd/group)) + bufsize as
potential overflows in the getpw/getgr helpers. Add asserts to
make the bounds explicit for static analyzers.
CID#1548047
CID#1548049
CID#1548069
CID#1548070
Follow-up for 75673cd8ae
Coverity flags offsetof(DirectoryEntries, buffer) + DIRENT_SIZE_MAX * 8
as a potential overflow. All operands are compile-time constants, so add
an assert_cc() to prove this at build time.
CID#1548020
Follow-up for 6393b847f4
Coverity flags 2 * space as a potential overflow. The space value
is bounded by prior allocation success, but add an explicit assert
to document this for static analyzers.
CID#1548056
Follow-up for 5e592c66bd
Coverity flags the x->start + x->nr and y->start + y->nr additions
as potential overflows. These are safe because uid_range_add_internal()
validates start + nr <= UINT32_MAX before inserting entries. Add asserts
to document this invariant for static analyzers.
CID#1548015
Follow-up for 8530dc4467
Coverity thinks _mntidb can be used uninitialized, but this
is not the case when r == 0. Add a bool variable to make it
clearer instead of reusing 'r' later, and an assert to guide
static analyzers.
CID#1644850
Follow-up for 5817c73391
It's a very common case (vfat...), and it's just too much noise. After
all the whole function exists primarily to deal with O_TMPFILE not being
availeble everywhere...
Inspired by the discussion in #41161.
Also change the order of flags to be more logical. First the option
to specify at what fields we look, then the option to specify how we
return their name, the the value, and finally what to do if the value
is missing.
For some reason the IMDS PR for the first time triggers an issue with
the DEFER_VOID_CALL() logic relying on assert() and being places in
macro.h, let's hence move this elsewhere.
This is almost the same as inode_hash_ops, but also hashes + compares
all attributes that could affect the contents of a file. It ignores
"superficial"/"external" attributes such as ownership or access mode
however.
This doesn't really have any major benefit, but it does make this nicely
mirror stat_inode_same() which also checks this triplet for identifying
identical inodes.
In one of the reviews one of the LLMs noticed that the pragma is set but
never unset, so it remains in effect for the rest of the translation
unit. From the comment, it's not clear how old those "old compilers"
were, so let's try if things work without this workaround.
It has been observed on some systems[1] that ssh-issue may print out:
Try contacting this VM's SSH server via 'ssh vsock%4294967295' from host.
i.e. it suggests connecting with VMADDR_CID_ANY, which is not valid. It
seems that IOCTL_VM_SOCKETS_GET_LOCAL_CID may return VMADDR_CID_ANY in
some cases, e.g. when vsock is not full initialized or so.
Treat VMADDR_CID_ANY as special in vsock_get_local_cid(), the same as
VMADDR_CID_LOCAL and VMADDR_CID_HOST, and return an error.
[1] https://launchpad.net/bugs/2145027
This is very similar to write_string_file_atomic(), but is intentionally
kept separate (after long consideration). It focusses on arbitrary
struct iovec data, not just strings, and hence also doesn't do stdio at
all. It's hence a lot more low-level.
We might want to consider moving write_string_file*() on top of
write_data_file_atomic_at(), but for now don't.
In one of the reviews one of the LLMs noticed that the pragma is set but
never unset, so it remains in effect for the rest of the translation
unit. From the comment, it's not clear how old those "old compilers" were,
so let's try if things work without this workaround.
Add a birthDate field to the JSON user record, stored internally as a
struct tm with INT_MIN/negative sentinels for unset fields. The field
is serialized as a YYYY-MM-DD string in JSON and validated via
parse_birth_date(), which shares its core logic with
parse_calendar_date() through a new parse_calendar_date_full()
function.
For birth dates, timegm() is called directly (rather than
mktime_or_timegm_usec) to support pre-epoch dates. The wday field is
used to distinguish timegm() failure from a valid (time_t) -1 return.
birthDate is excluded from user_record_self_modifiable_fields(), so
only administrators can set or change it via homectl. The field
remains in the regular (non-privileged) JSON section, keeping it
readable by the user and applications.
Move the YYYY-MM-DD date parsing and validation logic from
sysupdate-resource.c into a shared parse_calendar_date() function
in time-util, so it can be reused by other subsystems.
If this new scheme of adding dependencies is supposed to be used more
widely we need to start making it easy to add them. So add a new
self-contained header that projects can simply include without the need
to link against libsystemd itself. This will allow them to generate
`.notes.dlopen` sections:
```
> readelf -p .note.dlopen ./l2md
String dump of section '.note.dlopen':
[ a] |@FDO
[ 10] [{"feature":"manifest-json","description":"Manifest-based change detection via gzip and JSON parsing","priority":"suggested","soname":["libz.so.1","libsystemd.so.0"]}]
[ c2] |@FDO
[ c8] [{"feature":"manifest-http","description":"HTTP transport for lore.kernel.org manifest fetch","priority":"suggested","soname":["libcurl.so.4"]}]
```
The SYSTEMD_COLORS=true/1/yes no longer forced colors
when stdout was not a TTY (e.g. piped), because the COLOR_TRUE bypass
of the terminal_is_dumb() check was accidentally dropped.
Restore the old behavior by guarding the TTY check with
`m != COLOR_TRUE`, so an explicit boolean "true" value continues to
unconditionally force color output regardless of whether stdout is a TTY
or whether $NO_COLOR is set.
Switch all internal callers from the private ELF_NOTE_DLOPEN() macro to
the new public SD_ELF_NOTE_DLOPEN() API from sd-dlopen.h, and remove
the now-redundant macro definitions from dlfcn-util.h.
Signed-off-by: Christian Brauner <brauner@kernel.org>
asprintf is nice to use, but the _documented_ error return convention is
unclear:
> If memory allocation wasn't possible, or some other error occurs,
> these functions will return -1, and the contents of strp are undefined.
What exactly "undefined" means is up for debate: if it was really
undefined, the caller wouldn't be able to meaningfully clean up, because
they wouldn't know if strp is a valid pointer. So far we interpreted
"undefined" — in some parts of the code base — as "either NULL or a
valid pointer that needs to be freed", and — in other parts of the
codebase — as "always NULL". I checked glibc and musl, and they both
uncoditionally set the output pointer to NULL on failure.
There is also no information _why_ asprintf failed. It could be an
allocation error or format string error. But we just don't have this
information.
Let's add a wrapper that either returns a good string or a NULL pointer.
Since there's just one failure result, we don't need a separate return
value and an output argument and can simplify callers.
Yaping Li
Daan De Meyer
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
Michal Rybecky
Michael Vogt
Nick Rosbrook
Luca Boccassi
Mike Yuan
Dylan M. Taylor
Nandakumar Raghavan
Christian Brauner