The EDG parser version 6.8 now supports C23 enums with fixed underlying
types, and VSCode ships with it, so this workaround can now be removed
without breaking VSCode's internal parser.
https://en.cppreference.com/c/compiler_support/23
This reverts commit e01ca3afdb.
When compiler optimization or LTO (Link Time Optimization) is enabled,
dlopen helper functions (such as dlopen_libfoo()) can be aggressively
inlined into their callers or cloned for specific call sites.
While this is beneficial for production builds, it alters or completely
removes the original function symbols from the final executable.
Consequently, this breaks the test (to be added in this series) that
checks whether the dlopen_libfoo() function corresponding to a dlopen
ELF note is actually called.
To resolve this, introduce the `_dlopen_loader_` macro. Under developer
builds, it applies the `_noclone_` and `_noinline_` attributes to guarantee
that helper symbols remain intact and discoverable by the test suite. In
production builds, the macro evaluates to nothing, allowing full compiler
optimization to proceed unimpeded.
By chance (because I had this mistake in my own commit) I noticed
that there are a bunch of duplicated works like "the the" in the
code and man-pages.
This commit fixes them and some similar issues with "and and" etc.
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>
To make this work, ABS() is made generic so it also works on
floats and doubles.
While at it, fold the __ABS_INTEGER indirection and the
assert_cc(sizeof(long long) == sizeof(intmax_t)) away. The previous
form switched between __builtin_llabs (clang) and __builtin_imaxabs
(gcc), with the assert keeping the two paths behaviorally identical
on every platform we build for. imaxabs was originally chosen because
intmax_t is conceptually the widest signed integer type the platform
exposes, but the _Generic ABS already casts to (long long) before the
call, so the extra width imaxabs could in theory carry was being
narrowed away immediately anyway. With both paths collapsed to
__builtin_llabs((long long) (a)), the size relationship between
long long and intmax_t is no longer relevant.
Also add explicit unsigned long long / unsigned long / unsigned int
cases that pass the argument through unchanged. The previous default
branch cast unsigned values to (long long); for values above LLONG_MAX
this reinterprets them as negative, and __builtin_llabs(LLONG_MIN) is
UB. Unsigned values are already non-negative, so passing them through
is both correct and avoids the narrowing. Smaller unsigned types
(unsigned char, unsigned short) still go through the default branch
but promote to int first and fit in long long losslessly.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
systemctl_main() is moved to systemctl.c to allow fuzz-systemctl-parse-argv
to compile. It needs systemctl_help(), which needs the verb table, with the
expected groups. Once we provide that, the linker needs all the verb_*
functions. So add dummy implementations in fuzz-systemctl-parse-argv to
allow the link to happen.
The alternative would be to provide an empty option table, but that
seems to be more complicated, and also can simulate parsing of the whole
command line with the full verb set, so it seems better to test with the
real verb table.
$ nm build/fuzz-systemctl-parse-argv | rg 0000000000418885
0000000000418885 T verb_add_dependency
0000000000418885 T verb_bind
0000000000418885 T verb_cancel
0000000000418885 T verb_cat
0000000000418885 T verb_clean_or_freeze
0000000000418885 T verb_edit
0000000000418885 T verb_enable
0000000000418885 T verb_get_default
0000000000418885 T verb_import_environment
0000000000418885 T verb_is_active
0000000000418885 T verb_is_enabled
0000000000418885 T verb_is_failed
0000000000418885 T verb_is_system_running
0000000000418885 T verb_kill
0000000000418885 T verb_list_automounts
0000000000418885 T verb_list_dependencies
0000000000418885 T verb_list_jobs
0000000000418885 T verb_list_machines
0000000000418885 T verb_list_paths
0000000000418885 T verb_list_sockets
0000000000418885 T verb_list_timers
0000000000418885 T verb_list_unit_files
0000000000418885 T verb_list_units
0000000000418885 T verb_log_setting
0000000000418885 T verb_mount_image
0000000000418885 t verb_noop
0000000000418885 T verb_preset_all
0000000000418885 T verb_reset_failed
0000000000418885 T verb_service_log_setting
0000000000418885 T verb_service_watchdogs
0000000000418885 T verb_set_default
0000000000418885 T verb_set_environment
0000000000418885 T verb_set_property
0000000000418885 T verb_show
0000000000418885 T verb_show_environment
0000000000418885 T verb_start_special
0000000000418885 T verb_start_system_special
0000000000418885 T verb_switch_root
0000000000418885 T verb_trivial_method
0000000000418885 T verb_whoami
UEFI firmware can report the currently-active keyboard layout via
EFI_HII_DATABASE_PROTOCOL.GetKeyboardLayout(). The layout descriptor
includes an RFC 4646 / BCP 47 language tag (e.g. "en-US"). Query this
from sd-boot/sd-stub and write it to a new LoaderKeyboardLayout EFI
variable, advertised through a new EFI_LOADER_FEATURE_KEYBOARD_LAYOUT
feature bit.
On the OS side, systemd-vconsole-setup reads the variable as a
lowest-priority fallback for the console keymap. To map the BCP 47
tag to a vconsole keymap we extend /usr/share/systemd/kbd-model-map
with an optional sixth column listing the comma-separated BCP 47 tags
each row covers; a new find_vconsole_keymap_for_bcp47() helper walks
the file, preferring an exact tag match and otherwise falling back to
the row whose tag matches the input's primary subtag. Credentials,
/etc/vconsole.conf, and vconsole.keymap= on the kernel command line
continue to take precedence.
bootctl status surfaces the new variable, printing the language tag
or "n/a (not reported by firmware)" when sd-boot advertises the
feature but the firmware HII database didn't expose a layout (common
on QEMU without a USB keyboard, since EDK2's PS/2 driver does not
register an HII keyboard layout).
DEFINE_POINTER_ARRAY_CLEAR_FUNC() generates a helper of the form
helper_array_clear(T *array, size_t n) that drops each element but does
not free the array itself, parallel to DEFINE_POINTER_ARRAY_FREE_FUNC()
for cases where the array has automatic storage duration.
CLEANUP_ELEMENTS() pairs with these helpers to provide a _cleanup_-like
attribute for fixed-size arrays: the bound is taken from ELEMENTSOF(),
and the helper is invoked across the elements at scope exit. Compared to
CLEANUP_ARRAY(), the storage is neither freed nor zeroed.
Migrate various logic across the tree over to the new macros.
sd-device: use DEFINE_POINTER_ARRAY_CLEAR_FUNC() for sd_device_unref_array_clear()
Replace the local device_unref_many() helper with the macro-generated
equivalent.
format-table: switch help-table arrays to CLEANUP_ELEMENTS()
Generate table_unref_array_clear() via DEFINE_POINTER_ARRAY_CLEAR_FUNC()
and convert the help-table arrays in bootctl, cryptenroll, nspawn,
repart and vmspawn to CLEANUP_ELEMENTS(). The arrays no longer need a
trailing NULL slot, so the size matches ELEMENTSOF() of the groups
array.
firewall-util: switch netlink message arrays to CLEANUP_ELEMENTS()
Generate sd_netlink_message_unref_array_clear() via
DEFINE_POINTER_ARRAY_CLEAR_FUNC() in place of the NULL-terminated
sd_netlink_message_unref_many(), and convert the two stack arrays of
sd_netlink_message pointers to CLEANUP_ELEMENTS().
Previously, we had a nice third 'UKI PE Section' column with the section
names. This is now moved into the help strings, which means that the nice
alignment is lost. Previous behaviour could be restored by constructing
the table manually, but I'm not sure if this is worth the trouble.
Co-developed-by: Claude Opus 4.7 <noreply@anthropic.com>
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.
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.
Sometimes we want need to diff two unsigned numbers, which is awkward
because we need to cast them to something with a sign first, if we want
to use abs(). Let's add a helper that avoids the function call
altogether.
Also drop unnecessary parens arounds args which are delimited by commas.
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.
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.
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>
In some builds (package builds, so with optimization and lto, but I
haven't been able to pin down the exact combination on options that
matters), we end up with items in the verbs array reordered. The order
matters (because of groups, but also because we have some specific order
for display), so this reordering is something that we don't want.
From what I was able to read, the compiler + linker generally keep the
order within a single translation unit, but this is more of a convention
and implementation choice than a guarantee. Add this attribute [1]. It
seems to have the desired effect in CI.
[1] https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-no_005freorder-function-attribute
After having some more experience with how this works, I think some
changes are in order. So there are a handful of preparatory patches and
then conversion of a few progs that make use of the new functionality.
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()`).
This mirrors the idea and implementation for options.
Previously, the Verb struct was named as verb_func_data_nn, but
with the group marker entry, we don't have 'verb_func', so let's
just call the item verb_data_nn. Using the verb here is a complication
that is not needed.
If length is exactly 16, the loop would finish with length == 0,
but we'd carry on to the memcmp() check, where the 'p + 16' passed
would be invalid memory. memcmp() demands valid pointers even if
size is specified to 0, hence let's catch this ourselves.
With the uintptr_t data parameter, it is actually quite nice to have
VERB(do_impl, "name-a", …)
VERB(do_impl, "name-b", …)
int do_impl(…) { … }
To make this work, the do_impl_data struct needs to have a unique name and
we also need to suppress the warning about the forward declaration for
do_impl being repeated. I think it's fine to suppress the warning, it's
not needed for anything. If somebody declares the function with the same
name by mistake, the implementations are going to conflict too.
This attribute was introduced in gcc 11, and our baseline is currently
8.4. So let's allow using _retain_ everywhere, but make it into a noop
if not supported.
Using __has_attribute was suggested, but with gcc-11.5.0-14.el9.x86_64,
__has__attribute(__retain__) is true, but we get a warning when the
attribute is actually used.
Motivation:
Currently, when setting the default boot pattern, boot assessment status
is not taken into account. This means that with boot assessment enabled,
when an explicit boot entry is configured as the default entry using an
EFI var, as is common for instance in A/B boot schemes, the configured
entry will be booted indefinitly, regardless of the entry's boot
assessment status.
In order to allow for this use case in combination with boot assessment,
we introduce a new `preferred` keyword, both in the config file and in the
bootctl CLI, that acts very similar to the existing `default` keyword but
takes boot assessment into account and never selects any entries that
have been marked as bad.
If the preferred pattern does not resolve to any bootable entry, and a
default pattern is also specified, then the default pattern will be
considered next, and we may then still select a known-bad entry to be
booted.
Fixes: https://github.com/systemd/systemd/issues/31215
Fixes: https://github.com/systemd/systemd/issues/40192
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
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.
shim is often used as part of the EFI boot chain with Linux kernels.
shim has an option to disable all verification of binaries it loads.
This can be performed by end-user using mokutil / mokmanager EFI app,
which set BootServices only variable MokSBState. shim honors that, and
mirrors it as readonly MokSBStateRT for the post-ExitBootService access.
Thus presense of MokSBStateRT is an indicator that shim was used during
boot chain.
Some OEM vendors are known to set MokSBState variable, without user
having done so.
When verification is disabled, one should assume secureboot is insecure,
because any EFI binary was allowed to run, including but not limited to
unsigned or revoked:
- grub
- systemd-boot
- UKI
- linux kernel
Linux kernel also has code to check this variable, and correctly report
that Secure Boot is disabled, see:
-
https://github.com/torvalds/linux/blob/3a8660878839faadb4f1a6dd72c3179c1df56787/drivers/firmware/efi/libstub/secureboot.c#L57
With this change bootctl output changes like this:
```diff
System:
Firmware: n/a (n/a)
Firmware Arch: x64
- Secure Boot: enabled (user)
+ Secure Boot: disabled (insecure)
TPM2 Support: yes
Measured UKI: no
Boot into FW: supported
```
This implementation is trying to mimic mokutil behaviour like this one:
```
$ mokutil --sb-state
SecureBoot enabled
SecureBoot validation is disabled in shim
```
As well as the linux kernel behavior of:
```
$ journalctl -b | grep 'Secure boot disabled'
kernel: Secure boot disabled
```
Note that MokSBState is extended into PCR7 as well as also into PCR14.
For more details see https://github.com/rhboot/shim/blob/main/README.tpm
Luca Boccassi
Yu Watanabe
Michael Vogt
Paul Meyer
Lennart Poettering
Daan De Meyer
Zbigniew Jędrzejewski-Szmek
Mike Yuan
r-vdp
Daan De Meyer
Lennart Poettering
Dimitri John Ledkov