Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V254 stable batch #311

Merged
merged 77 commits into from
Aug 9, 2023
Merged

V254 stable batch #311

merged 77 commits into from
Aug 9, 2023

Conversation

keszybz
Copy link
Member

@keszybz keszybz commented Aug 9, 2023

No description provided.

bluca and others added 24 commits August 9, 2023 11:33
CID#1491292
CID#1491291
CID#1491290
CID#1491289
CID#1491284
CID#1491281
CID#1491280
CID#1491278

(cherry picked from commit d80cc39)
Spotted while fuzzing #27890.

=================================================================
==908098==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7f4efe6d81f5 in __interceptor_realloc.part.0 (/lib64/libasan.so.8+0xd81f5) (BuildId: dc689b05ca2577037af24700212bb5cce1f91c8a)
    #1 0x7f4efb8e3ace in greedy_realloc ../src/basic/alloc-util.c:70
    #2 0x7f4efb93b713 in extract_first_word ../src/basic/extract-word.c:62
    #3 0x7f4efb970d50 in set_put_strsplit ../src/basic/hashmap.c:1902
    #4 0x7f4efd76c27e in exec_context_deserialize ../src/core/execute-serialize.c:3341
    #5 0x7f4efd778dcb in exec_deserialize ../src/core/execute-serialize.c:4122
    #6 0x4032c0 in LLVMFuzzerTestOneInput ../src/core/fuzz-execute-serialize.c:60
    #7 0x403c58 in main ../src/fuzz/fuzz-main.c:50
    #8 0x7f4efecccb49 in __libc_start_call_main (/lib64/libc.so.6+0x27b49) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #9 0x7f4efecccc0a in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27c0a) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #10 0x402344 in _start (/home/mrc0mmand/repos/@systemd/systemd/build-san/fuzz-execute-serialize+0x402344) (BuildId: 195f382cf1e39b9ba48d6dcf5a90f786d72837a8)

SUMMARY: AddressSanitizer: 64 byte(s) leaked in 1 allocation(s).
Aborted (core dumped)

==911550==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 17 byte(s) in 1 object(s) allocated from:
    #0 0x4df281 in strdup (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x4df281) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #1 0x7fe4ae2b38fc in _set_put_strndup_full /home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/../src/basic/hashmap.c:1868:21
    #2 0x7fe4b0bad897 in exec_context_deserialize /home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/../src/core/execute-serialize.c:3914:29
    #3 0x7fe4b0b80592 in exec_deserialize /home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/../src/core/execute-serialize.c:4109:13
    #4 0x531d0f in LLVMFuzzerTestOneInput /home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/../src/core/fuzz-execute-serialize.c:59:16
    #5 0x440594 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x440594) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #6 0x43f9b9 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x43f9b9) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #7 0x440fd5 in fuzzer::Fuzzer::MutateAndTestOne() (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x440fd5) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #8 0x441955 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x441955) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #9 0x42e151 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x42e151) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #10 0x45a916 in main (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x45a916) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
    #11 0x7fe4ac449b49 in __libc_start_call_main (/lib64/libc.so.6+0x27b49) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #12 0x7fe4ac449c0a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x27c0a) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #13 0x422b74 in _start (/home/mrc0mmand/repos/@systemd/systemd/build-libfuzz/fuzz-execute-serialize+0x422b74) (BuildId: 4e58706e607b8be7972d83c421bc0b625d509ec6)
SUMMARY: AddressSanitizer: 17 byte(s) leaked in 1 allocation(s).

(cherry picked from commit 9b41270)
New meson says:
WARNING: Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated.

Inspired by systemd/systemd#28482.

(cherry picked from commit 4f3c90a)
If we don't have TPM support then `alg` is NULL and passing this to
table_new() means we'd get a table with only two columns instead of
three, leading up to a very confusing output:

$ build/systemd-analyze pcrs
System lacks full TPM2 support, not showing PCR state.
                 NR NAME
                  0 platform-code
                  - 1
    platform-config -
                  2 external-code
                  - 3
    external-config -
                  4 boot-loader-code
                  - 5
 boot-loader-config -
                  6 -
                  - 7
...

Let's name the header in this case with a simple dash, as it's going
to be hidden anyway, to make the table nice again:

$ build/systemd-analyze pcrs
System lacks full TPM2 support, not showing PCR state.
NR NAME
 0 platform-code
 1 platform-config
 2 external-code
 3 external-config
 4 boot-loader-code
 5 boot-loader-config
 6 -
 7 secure-boot-policy
...

(cherry picked from commit 9fe4e68)
(cherry picked from commit 24c0078)
Follow-up for #28511

Fixes #28550

(cherry picked from commit 7893a54)
The PE header size calculation failed to take the PE magic and coff
header size into account, which will lead to header truncation if we are
writing only 5 sections.

(cherry picked from commit ee91e06)
The stub image may not have enough sections to cause enough PE header
space to be free for later expansion. Given that the stub is guaranteed
to be expanded we should always reserve enough sections for it.

This also bumps the reservation to 15. It doesn't add more space
compared to current builds but it more closely reflects the amount of
sections that a UKI could have in total if all optional sections are
used.

Fixes: #28593
(cherry picked from commit d0bbe32)
For readability that 'done' and 'fd' are always consistent with each other.

- dir_fd == AT_FDCWD:
  - path is absolute:
    - previous: fd = open("/")
    - current:  fd = openat(AT_FDCWD, "/")
  - path is relative:
    - previous: fd = openat(AT_FDCWD, ".")
    - current:  fd = openat(AT_FDCWD, ".")
- dir_fd >= 0:
  - dir_fd points to "/":
    - previous: fd = openat(dir_fd, ".")
    - current:  fd = openat(dir_fd, "/")
  - dir_fd does not point to "/":
    - previous: fd = openat(dir_fd, ".")
    - current:  fd = openat(dir_fd, ".")

Hence, this should not change any behavior. Just refactoring.

(cherry picked from commit 00a050b)
(cherry picked from commit 5f0bae7)
The flag will be anyway dropped in chaseat(), but let's shortcut.

(cherry picked from commit b7e957d)
When 'need_absolute' is true, 'done' should always contain "/" at the
beginning, and thus should not be NULL.

(cherry picked from commit 1c13bdf)
Should not change any behavior.

(cherry picked from commit 4de5b4e)
In chaseat() we call dir_fd_is_root() several places, and the final
result depends on it. If the root path specified to `chase()` is not
normalized but points to "/", e.g. "/../", assertions in `chaseat()` or
`chase()` may be triggered.

(cherry picked from commit 83c57d8)
Otherwise, if it fails, chaseat() may return unexpected result and
triggers an assertion in chase().

(cherry picked from commit 4445242)
The logs would give no hint about the answer to most interesting question: why
we decided to return true or false from the program. If we find batteries
that are low or uncertain, log at info level.

(cherry picked from commit 02f7f8a)
The descriptions of various options are reworked: first say what protocol
actually is, i.e. describe what type of notification the manager waits
for. Only after that describe various steps and things the service should
do. Also, apply some paragraph breaks.

Instead of recommending Type=simple, recommend Type=exec. Say explicitly that
Type=simple, Type=forking are not recommended. Type=simple ignores failure in a
way that doesn't make any sense except as a historical accident. We introduced
'exec' instead of changing 'simple' to keep backwards-compatiblity, but
'simple' is not very useful. 'forking' works, but is inefficient: correctly
programming the interface requires a lot of work, and at runtime, the
additional one or two forks are just a waste of CPU resources. Furthermore, we
now understand that because of COW traps, they may also increase memory
requirements. There is really no reason to use 'forking', except if it's
already implemented and the code cannot be changed to use 'notify'.

Also, remove the recommendations to use Type=simple to avoid delaying boot. In
most cases, if the service can support notifications about startup, those
should be done.

Overall, for new services, "notify", "notify-reload", and "dbus" are the
types that make sense.

(cherry picked from commit 377d3a3)
We don't have type 'ready'.

(cherry picked from commit ffe7ddb)
We declare clang >= 10 is supported. The workaround is easy enough, so let's
just do that. Fixes #28598. With Debian clang version 11.0.1-2:

[267/384] Compiling C object test-bitfield.p/src_test_test-bitfield.c.o
FAILED: test-bitfield.p/src_test_test-bitfield.c.o
clang -Itest-bitfield.p -I. -I.. -Isrc/basic -I../src/basic -Isrc/fundamental -I../src/fundamental -Isrc/systemd -I../src/systemd -I../src/libsystemd/sd-bus -I../src/libsystemd/sd-device -I../src/libsystemd/sd-event -I../src/libsystemd/sd-hwdb -I../src/libsystemd/sd-id128 -I../src/libsystemd/sd-journal -I../src/libsystemd/sd-netlink -I../src/libsystemd/sd-network -I../src/libsystemd/sd-resolve -Isrc/shared -I../src/shared -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O0 -g -Wno-missing-field-initializers -Wno-unused-parameter -Warray-bounds -Wdate-time -Wendif-labels -Werror=format=2 -Werror=implicit-function-declaration -Werror=implicit-int -Werror=incompatible-pointer-types -Werror=int-conversion -Werror=missing-declarations -Werror=missing-prototypes -Werror=overflow -Werror=override-init -Werror=return-type -Werror=shift-count-overflow -Werror=undef -Wfloat-equal -Winit-self -Wmissing-include-dirs -Wmissing-noreturn -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-aliasing=2 -Wstrict-prototypes -Wunused-function -Wwrite-strings '-Wno-error=#warnings' -Wno-string-plus-int -fdiagnostics-show-option -fno-common -fstack-protector -fstack-protector-strong --param=ssp-buffer-size=4 -Wno-typedef-redefinition -Wno-gnu-variable-sized-type-not-at-end -Werror=shadow -fno-strict-aliasing -fvisibility=hidden -fno-omit-frame-pointer -include config.h -DTEST_CODE=1 -MD -MQ test-bitfield.p/src_test_test-bitfield.c.o -MF test-bitfield.p/src_test_test-bitfield.c.o.d -o test-bitfield.p/src_test_test-bitfield.c.o -c ../src/test/test-bitfield.c
../src/test/test-bitfield.c:216:9: error: static_assert failed due to requirement '__builtin_constant_p(({
    static_assert(sizeof(unsigned char) <= sizeof(unsigned long long), "sizeof(uint8_t) <= sizeof(unsigned long long)");
    static_assert(__builtin_choose_expr(__builtin_constant_p(1), 1, 0) < (int)(sizeof(unsigned char) * 8), "__builtin_choose_expr(__builtin_constant_p(1), 1, 0) < (int)(sizeof(uint8_t) * 8)");
    __builtin_choose_expr(__builtin_constant_p(1), ((unsigned char)1) << (1), ({
        int __unique_prefix__i751 = (1);
        do {
            if ((__builtin_expect(!!(!(__unique_prefix__i751 < (int)sizeof(unsigned char) * 8)), 0)))
                log_assert_failed("UNIQ_T(_i, 751) < (int)sizeof(uint8_t) * 8", (&"../src/test/test-bitfield.c"[(sizeof ("..") - sizeof(const char)) + 1]), 216, __func__);
        } while (0);
        ((unsigned char)1) << __unique_prefix__i751;
    }));
}))' "__builtin_constant_p(INDEX_TO_MASK(uint8_t, 1))"
        assert_cc(__builtin_constant_p(INDEX_TO_MASK(uint8_t, 1)));
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/fundamental/macro-fundamental.h:109:25: note: expanded from macro 'assert_cc'
                        ^             ~~~~
/usr/include/assert.h:143:24: note: expanded from macro 'static_assert'
                       ^
...

(cherry picked from commit 6425dec)
We usually reset setting when an emptry string is specified.

(cherry picked from commit b8e898a)
KERNEL_INSTALL_BYPASS should only bypass verbs that actually change
the system, not harmless verbs such as "inspect".

(cherry picked from commit b4afa94)
@keszybz keszybz requested a review from bluca August 9, 2023 11:42
@bluca
Copy link
Member

bluca commented Aug 9, 2023

I would skip watchdog: Allow the watchdog to be disabled at runtime as this area has been a bit flaky in the past (because it's very hard to test), so the potential for regression here is high and we don't have any test coverage

@keszybz
Copy link
Member Author

keszybz commented Aug 9, 2023

I dropped "unit: make udev rules take precesence over tmpfiles" because it's reverted in systemd/systemd@41521e3.

I'll drop the watchdog commit too.

Copy link
Member

@bluca bluca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, let's wait for the full CI run though

Follow-up for #28551.

(cherry picked from commit 1a572fd)
YHNdnzj and others added 17 commits August 9, 2023 13:51
Follow-up for 99299d0

is_device_node() calls lstat(), causing device node
symlinks under /dev/disk/ not being compared correctly
using devnode_same().

Fixes #28585

(cherry picked from commit cc1e1bb)
bus_unit_queue_job_one has two callers:
- bus_unit_queue_job which would do the appropriate transormations
  to turn JOB_TRY_RESTART into JOB_TRY_RELOAD,
- and method_enqueue_marked_jobs which did not.
In effect, method_enqueue_marked_jobs() would queue restart jobs for
units which has Markers= needs-reload or needs-restart.

When the chunk of code which does the transformations is moved from
bus_unit_queue_job to bus_unit_queue_job_one, there is no change for
bus_unit_queue_job, and method_enqueue_marked_jobs is fixed.

The additional checks that are done seem reasonable to do from
method_enqueue_marked_jobs: we shouldn't be restarting units which are
configured to not allow that, or force unwanted start of dbus-broker.

(cherry picked from commit 8ea8e23)
We'll need this on each read() again, hence let's just allocate this
once and then reuse it for subsequent read()s.

Follow-up for: #28639

(cherry picked from commit 3c86805)
This partially reverts the commit 684bce3
and fixes the issue introduced by it.

Fixes #28711.

(cherry picked from commit beddf8b)
Fixes a bug introduced by 998db58.

Fixes #28671.

(cherry picked from commit 074d909)
The systemctl invocations used for these completions match the ones used
for the _sys_really_all_units parameter, so we should really just use
the cached parameter rather than recomputing the result.

(cherry picked from commit c8e2cd7)
The systemctl completion previously made use of PREFIX as a pattern
argument to list-unit-files and list-units. This had the problem of
erroneously filtering the results that were stored in the cache, and
erroneously filtering results that might have been requested according
to the users configuration (e.g. _correct completer, certain
matcher-lists or tag-orders, etc.).

Unfortunately, the runtime of list-unit-files increases when no pattern
argument is provided, and systemctl show, used to filter those units,
can become unacceptably slow when provided with too many units to
describe.

Let's re-introduce the pattern argument to list-unit-files and
list-units where necessary in order to alleviate these bottlenecks
without poisining the cache. A 'use-pattern' style is introduced that
may be used to disable this behavior if it is undesired. We can still
expect that certain completions, like `systemctl start <TAB>` will be
slow, like before. To fix this we will need systemd to learn a more
efficient way of filtering the units than parsing systemctl show.

(cherry picked from commit 2cbda74)
If emergency.target is started while initrd-parse-etc.service/start is queued,
the initrd-parse-etc job did not get canceled. In parallel to the emergency
units, it eventually runs the service, which starts initrd-cleanup.service,
which in turn isolates initrd-switch-root.target. This stops the emergency
units and effectively starts the initrd boot process again, which likely
fails again like the initial attempt. The system is thus stuck in an endless
loop, never really reaching emergency.target.

With this conflict added, starting emergency.target automatically cancels
initrd-parse-etc.service/start, avoiding the loop.

(cherry picked from commit 327cd2d)
(cherry picked from commit 653c90e)
…ithout value

Otherwise, manager_parse_dns_server_string_and_warn() or
manager_parse_search_domains_and_warn() will trigger assertion.

(cherry picked from commit 91acee9)
Otherwise, we silently ignore the received packet and that makes hard to
debug issue.

(cherry picked from commit 809da72)
Fixes a bug introduced by 08b04ec and
953006d.

Fixes #28725.

(cherry picked from commit 685e0dd)
Decrease devlink priority for encrypted partitions, and make the priority for
decrypted DM devices relatively higher. This is for the case that an encrypted
partition and its decrypted DM device have the same label.

(cherry picked from commit c4521fc)
This reverts commit 33b9130.

The commit b42482a dropped
'--exclude-prefix=/dev' from systemd-tmpfiles-setup.service. So, the
possibly later invocation of the service changes the permission set by
udevd.

As commmented in the head of this file, settings should be consistent
with udev rules. Only missing entry here is vfio. Let's re-add the
entry for the device.

Addresses systemd/systemd#28681 (comment).

(cherry picked from commit ca15b59)
Otherwise, the check below is always fail.
```
if (FLAGS_SET(query_flags, SD_RESOLVED_NO_STALE) && j->until_valid < current)
```

Follow-up for 5ed9148.

(cherry picked from commit 6756b61)
Kernel patch [1] fixed bugs in rfkill handling on MSI Wind U100. Now
that the HW rfkill reports the correct state, and the SW rfkill is
controllable from userspace, it's necessary to mute KEY_WLAN and
KEY_BLUETOOTH generated on HW rfkill state changes. Otherwise, the
userspace will react to these keys and toggle the SW rfkill as well,
which is not desired, because the user may end up with non-functional
radios if HW and SW rfkills are out of sync.

Blocking these keycodes doesn't impair user experience, because the
desktop environment can still react to HW rfkill events and act
accordingly (for example, show notifications).

While at it, use "unknown" instead of "reserved" to mute keys, to avoid
the "atkbd serio0: Unknown key pressed" flood in dmesg.

[1]: https://lore.kernel.org/all/[email protected]/

(cherry picked from commit fa8216e)
@keszybz
Copy link
Member Author

keszybz commented Aug 9, 2023

Somebody broke the internet:

+ git clone --quiet --depth=1 https://salsa.debian.org/ci-team/autopkgtest.git /tmp/autopkgtest
error: RPC failed; curl 16 Error in the HTTP2 framing layer

@bluca
Copy link
Member

bluca commented Aug 9, 2023

Somebody broke the internet:

+ git clone --quiet --depth=1 https://salsa.debian.org/ci-team/autopkgtest.git /tmp/autopkgtest
error: RPC failed; curl 16 Error in the HTTP2 framing layer

yes it happens every now and then, I'll hit the retry buttons in 10 minutes or so

@keszybz
Copy link
Member Author

keszybz commented Aug 9, 2023

The three jammy runners as still doing their thing, but it's been a couple of hours, so I'll merge this without waiting. Some other ubuntu-based CI passed, so I don't expect any issues to be uncovered.

@keszybz keszybz merged commit 208a218 into v254-stable Aug 9, 2023
43 checks passed
@keszybz keszybz deleted the v254-stable-batch branch August 9, 2023 14:05
@keszybz keszybz temporarily deployed to github-pages August 9, 2023 14:06 — with GitHub Pages Inactive
bluca pushed a commit to bluca/systemd-stable that referenced this pull request Nov 9, 2023
Preparation for systemd#311

(cherry picked from commit 002db03)
bluca pushed a commit to bluca/systemd-stable that referenced this pull request Nov 9, 2023
bluca pushed a commit to bluca/systemd-stable that referenced this pull request Nov 9, 2023
Preparation for systemd#311

(cherry picked from commit 002db03)
bluca pushed a commit to bluca/systemd-stable that referenced this pull request Nov 9, 2023
bluca pushed a commit that referenced this pull request Nov 9, 2023
Preparation for #311

(cherry picked from commit 002db03)
bluca pushed a commit that referenced this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.