Switch the executable-owned call sites from the plain dlopen_<lib>()
helpers to the new DLOPEN_<LIB>() macros, so that each binary that loads
an optional library at runtime now carries its own .note.dlopen note
instead of relying solely on libsystemd-shared. Each call site passes a
priority reflecting whether it hard-depends on the library (required) or
degrades gracefully without it (recommended).
Co-developed-by: Claude Opus 4.8 <noreply@anthropic.com>
Drop the -fundamental suffix from src/fundamental/ headers in favor of names
that match their src/basic/ or src/shared/ counterparts (e.g.
macro-fundamental.h -> macro.h, assert-fundamental.h -> assert-util.h,
cleanup-fundamental.h -> cleanup-util.h). Rename src/basic/{btrfs,label}.{c,h}
to use the -util suffix to match the existing shared/btrfs-util and
shared/label-util siblings. Rename src/shared/mkdir-label.{c,h} to mkdir.{c,h}
and src/shared/tmpfile-util-label.{c,h} to tmpfile-util.{c,h} to match the
corresponding src/basic names.
This saves us from having to come up with separate names for files that do
the same thing across tiers, and it makes it easier to move stuff between
src/fundamental, src/basic and src/shared: consumers just #include "foo.h"
and pick up whichever tier their -I path resolves to first, so call sites
don't need to be updated when an API moves between layers.
Where a higher-tier wrapper exists (e.g. src/basic/macro.h wrapping
src/fundamental/macro.h), the wrapper uses an explicit "../fundamental/foo.h"
or "../basic/foo.h" relative include for the lower-tier header. We can't use
GCC's #include_next directive for this — when the wrapper is reachable both
via same-dir-as-source lookup and via -I (e.g. -Isrc/shared) for the
directory it lives in, #include_next advances by exactly one slot in libcpp's
internal directory chain and lands on the same physical directory it was
already in, never reaching the lower-tier sibling (see make_cpp_dir() in
gcc/libcpp/files.cc:1986).
To make sure the right headers are always picked up, the include directories
are reordered so that e.g. src/shared always takes priority over src/basic and
similar for the other directories.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
In order to allow FD Stores of user units to survive a user
session restart, propagate FDs received via the protocol up one
level from user to system manager via sd_notify.
And the other way around, propagate them down via LISTEN_FDS
tagging them with the unit name so that the child manager can
inject them in the appropriate unit.
Ensure units that are dead or not loaded can get FDs added to
their stores, and that they are correctly propagated once the
unit is started or loaded. When the unit is not loaded we don't
know what the FD max limit is, so simply increase it for each FD
injected, and then when the unit is realised prune it down to
match the unit's now available config in case the limit is lower
than the number of FDs in the store.
Each FD sent up or down is assigned a monotonic index, and the manager
also sends a JSON map that associates the index with the original
unit and FDNAME:
{
"unit-name.service": [
{ "name": "fdname1", "index": 1 },
{ "name": "fdname2", "index": 2 }
],
...
}
This allows the manager to assign back the FDs to the appropriate
unit using the appropriate name, given the FDNAMEs are not unique.
The logic that was tested in the previous commit is used to implement
the behaviour for unit-shell and other verbs without changes.
The compare-versions synopsis is shortened to "V1 [OP] V2" to make the
verb synopsis fit. Unusual capitalizaition of "Command" is changed to
"COMMAND" (it's a replace arg, not a fixed string), and some help
strings are adjusted. The order of options in --help is based on the
existing order in parse_argv(). The old order in --help was mostly
random. I think it might be good to figure out something more rational
here, but I'm leaving that as a separate step.
The urlification of dot(1) in the --help string is lost. It's hard to
do this with the help string being stored in a read-only section.
I think this is not worth the trouble to reimplement in the current
scheme.
Every dlopen_xxx() helper now takes an int log_level argument. It is
passed through to dlopen_many_sym_or_warn() (which in turn propagates it
to dlopen_verbose() for the library-not-installed case), and is used by
the fallback stub when support for the library is not compiled in to
emit a "<lib> support is not compiled in." message at the caller's level.
Callers pass LOG_DEBUG when gracefully degrading, or a higher level when
the failure should surface, and no longer need to log redundantly at the
call site.
As part of this, dlopen_bpf_full() (which already took a log_level) is
merged into dlopen_bpf() rather than keeping both.
The static inline fallbacks used to live in the headers, which required
pulling log.h in from every header that declared a dlopen_xxx(). Move
them into the .c files instead: the declaration is always outside the
#if HAVE_XXX block, the impl sits outside the outer #if HAVE_XXX wrap
with its own internal #if HAVE_XXX/#else/#endif, and apparmor-util.c,
idn-util.c, libmount-util.c and pam-util.c are now always compiled so
they can host their stubs.
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.
We don't want to return an error without printing something.
So for things which don't matter, explicitly suppress the error
with (void). In other cases, add the standard message.
We generally want to print an error message if table_print()
fails. Add a helper function for this and use it consistently.
This does one of the three things depending on the call site:
- a no-change reformatting of the code
- change from a custom message to the generic one
- addition of the error message where previously none was printed
In the third case, the actual use impact is very small, since the
table formatting is very unlikely to fail. But if it did, we would
often return an error without any message whatsoever, which we
never want to do.
Almost all callers of table_print() specify stdout or NULL (equivalent
to stdout) as the output stream. Simplify things by not requiring the
the stream to be specified.
In almost all cases, the printing of the table is surrounded by normal
printfs() that don't do explicit flushing and for which we don't check
the output stream status. Let's simplify most callers and skip this
step. The reason is not so much to avoid the extra step itself, but
instead to avoid the _handling_ of the potential failure. We generally
only want to print an error message for ENOMEM and other "internal"
errors, so strictly speaking we should filter out the errors from the
stream. By skipping the flush&check step we implicitly do this.
I guess this wasn't converted previously because verb_blame doesn't
support json output, the flags that are passed atm cannot contain real
json flags. That's OK, we can still use table_print_with_pager.
There are some unnecessary() if() checks for variables
that are already guarded by assert()s. Drop those.
Thanks to keszybz for clarifying the direction of this commit.
The fix in 8f1751a111 made me wonder if we could automatically detect
when pointers are accessed but when this might not be safe. Systemd
is already using a lot of `assert(dst)` and this change now forces
us to use them.
So this commit (ab)uses coccinelle to flag any pointer parameter
dereference not preceded by assert(param), ASSERT_PTR(param), or an
explicit NULL check. It adds integration into meson as a new "coccinelle"
test suite (just like clang-tidy) and is run in CI. The check is not
perfect but seems a reasonable heuristic.
For this RFC commit it is scoped to a subset, it excludes 25 dirs right
now and includes around 100. About 300 warnings left. Busywork that I am
happy to do if there is agreement that it is worth it.
With this in place we would have caught the bug from 8f1751a111 in CI:
```
FAIL: check-pointer-deref.cocci found issues in systemd/src/boot:
diff -u -p systemd/src/boot/measure.c /tmp/nothing/measure.c
--- systemd/src/boot/measure.c
+++ /tmp/nothing/measure.c
@@ -312,7 +312,6 @@ EFI_STATUS tpm_log_tagged_event(
if (err != EFI_SUCCESS)
return err;
- *ret_measured = true;
return EFI_SUCCESS;
}
```
This also adds a new POINTER_MAY_BE_NULL() for the cases when the
called function will do the NULL check (like `iovec_is_set()`).
In many cases it's essential to know if the firmware supports a TPM, but
in others we should accept it if the firmware doesn't have TPM support,
in particular if we want to run the OS with a software TPM.
Hence, add tpm2_is_mostly_supported() as function similar to
tpm2_is_fully_supported(), with the only difference that the former
doesn't insist on a firmware supported TPM. Then, change a number of
users over to this (but not all).
Typically, the top-level JSON object has to be an object, in any json
document we parse, hence let's add a simple way to enforce that.
Make use of this in various places.
(Note, various other JSON parsers insist on this logic right from the
beginning, but I actually thinking making this insisting optional like
this patch does it is the cleaner approach)
We often have a pattern where the same verb function is used for
multiple actions. This leads to an antipattern where we figure out what
action needs to be taken from argv[0] multiple times: often once in
parse_argv() to figure out what options are allowed, then once again
implicitly in dispatch_verb(), and then again in the action verb itself.
Let's allow passing a parameter into the verb to simplify this.
This matches a pattern we have in conf-parser.h, where we have both
void *userdata (more global) and void *data (per-config item). Here,
I opted for uintptr_t userdata. It seems that most of the time we'll
want to just pass an enum value. This works OK with no casts. I also
tried a void* and union. In both cases, much more boilerplate is needed:
either a cast or a macro to help avoid compiler warnings. uintptr_t
seems generic enough to cover foreseeable usecases with no fuss.
This is a noop refactoring. See next commit for an example.
Suppress warnings like the following from clang tidy:
```
../src/boot/addon.c:11:19: error: function 'efi_main' can be made static to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors]
11 | EFIAPI EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *system_table);
| ^
```
Some warnings are suppressed simply by setting comments to ignore the warning,
some are by making global variables static, or include a suitable header.
This does not yet support directory images properly
as systemd itself does not support unpriv directory
images properly yet.
The user profiles are a copy of the system profiles but without
DynamicUser=yes (can't be used by user managers) and without
ProtectHome=yes (this masks /home which breaks StateDirectory= which
is lcoated inside /home)
Let's introduce these for the following two reasons:
- Using them makes sure the parameter names match between declaration
and definition (if both use the corresponding macros).
- They make sure developers can't forget the _const_ and _pure_
attributes for the declarations.
This commit also includes some include sorting fixes
LUKS2 supports built-in integrity checking which may come very handy to
mitigate partial rollback attacks on the storage when only some specific
parts are restored to some old encrypted state. Specific use-cases like
Confidential VMs may want to mandate the usage of feature e.g. on the root
volume. Introduce "encryptedwithintegrity" image policy to support that.
Note, due to the current libcryptsetup limitations, checking whether the
feature is enabled or not for the 'file' case (e.g. DDI image as a raw file)
requires setting up a loop device. To avoid that and keep dissect fully
functional when working unpriviliged, implement a minimal custom LUKS header
parser.
Usually we gracefully ignore unknown configuration parameters, so that
service files can be written by upstreams and used across a variegated
range of distributions with various versions of systemd, to avoid
forcing users to the minimum common denominator and only adding settings
that are supported by the oldest distro supported.
Image policies do not behave like this, and any unknown partition or
policy designator causes the whole unit to fail to parse and a hard
error.
Change it so that parsing RootImagePolicy and friends via unit file or
D-Bus logs but otherwise ignores unknown specifiers, like other options
do.
This allows us to add new specifiers in the future, and users to adopt
them immediately.
Follow-up for d452335aa4
Latest glibc uses _Generic to have strstr() and other functions return
const char* or char* based on whether the input is a const char* or a
char*. This causes build failures as we previously always expected a char*.
Let's fix the compilation failures and add our own macros similar to glibc's
to have string functions that return a mutable or const pointer depending on
the input.
In all cases, everything that we list in 'extract', we also list in
'sources'. We can simplify things by automatically appending the first
list to the second.
In the listings, move 'extract' key right below 'sources', since now
they are both "sources", just with slightly different meanings.
Let's extract common capability parsing code into a generic function
parse_capability_set() with a comprehensive set of unit tests.
We also replace usages of UINT64_MAX with CAP_MASK_UNSET where
applicable and replace the default value of CapabilityBoundingSet
with CAP_MASK_ALL which more clearly identifies that it is initialized
to all capabilities.
AI (copilot) was used to extract the generic function and write the
unit tests, with manual review and fixing afterwards to make sure
everything was correct.
Let's not leak details from src/shared and src/libsystemd into
src/basic, even though you can't actually do anything useful with
just forward declarations from src/shared.
The sd-forward.h header is put in src/libsystemd/sd-common as we
don't have a directory for shared internal headers for libsystemd
yet.
Let's also rename forward.h to basic-forward.h to keep things
self-explanatory.
When HAVE_SECCOMP is not set, a build error happens:
../src/analyze/analyze-security.c: In function ‘get_security_info’:
../src/analyze/analyze-security.c:2449:13: error: unused variable ‘r’ [-Werror=unused-variable]
2449 | int r;
| ^
cc1: some warnings being treated as errors
Fix it by removing the sometimes unused variable.
Luca Boccassi
Lennart Poettering
Daan De Meyer
Zbigniew Jędrzejewski-Szmek
Michael Vogt
Yu Watanabe
Daan De Meyer
Lennart Poettering
Vitaly Kuznetsov
Luca Boccassi
Zbigniew Jędrzejewski-Szmek
David Tardon
Matteo Croce
Antonio Alvarez Feijoo