sd-future: rework cancellation, add slot callbacks and future groups - #15
Open
daandemeyer wants to merge 947 commits into
Open
sd-future: rework cancellation, add slot callbacks and future groups#15daandemeyer wants to merge 947 commits into
daandemeyer wants to merge 947 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if (r < 0) | ||
| return r; | ||
|
|
||
| return sd_future_result(target); |
There was a problem hiding this comment.
Claude: suggestion: After sd_fiber_suspend() returns a non-negative value, sd_fiber_await() now returns sd_future_result(target). If the fiber was woken by something other than target resolving (e.g. an unrelated sd_fiber_resume(self, 0) wired up by the caller), target is still PENDING and sd_future_result() will log an assert_return failure and return -EBUSY. The previous sd_fiber_suspend() path returned the resume value directly without this trap. This also feeds back into sd_future_cancel_wait_unref(), whose loop calls sd_fiber_await(f): a spurious non-negative wakeup turns into q = -EBUSY, which then gets re-queued onto the calling fiber as a bogus interruption. Consider guarding on sd_future_state(target) == SD_FUTURE_RESOLVED before returning its result, or returning the raw suspend value when the target hasn't resolved.
Owner
Author
|
@claude review |
Owner
Author
|
@claude review |
Owner
Author
|
@claude review |
Owner
Author
|
@claude review |
Owner
Author
|
@claude review |
Owner
Author
|
@claude review |
Owner
Author
|
@claude review |
Both already an option in udevadm and systemd-tmpfiles.
While keyring_describe() allocates a 64 byte buffer it still passes the value of c as the buffer length. It's -1 on the first iteration. So keyctl_describe_key() copies the whole description whenever buflen >= its length. So any key whose description exceeding 64 bytes overflows. Pass the actual buffer size. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org> [zjs: adjust comment]
Newly allocated new_res variable was left unreferenced if memory allocation failed in ndisc_option_add_encrypted_dns_internal(). Co-authored-by: Siteshwar Vashisht <svashisht@redhat.com>
For a Type=dbus service we sometimes want to keep treating it as running while its bus name has briefly gone away (for example, due to a broker restart). SERVICE_RUNNING_REVALIDATING is active and keeps its main process like SERVICE_RUNNING, but it's deliberately not part of the runtime timer set, so RuntimeMaxSec= is paused while we sit in it.
If dbus dies, it is then immediately reactivated by dbus.socket as soon as a client reconnects. In that case pid1's own connection to the bus drops and is reestablished by manager_recheck_dbus() (which comes from the unit_notify() when dbus.services goes back to being running). As part of bringing the bus back up, bus_setup_api() issues GetNameOwner() for every name we watch on behalf of a BusName= unit. For a Type=dbus service this is a problem. Consider this scenario: 1. spearmint.service is Type=dbus, owns com.example.Spearmint, and is running. 2. The broker crashes and is re-activated through dbus.socket. 3. spearmint's client connection dropped too, so it is now reconnecting to the new broker to grab its name again. 4. pid1 racily reconnects first given it is triggered the moment dbus.service reaches SERVICE_RUNNING, whereas spearmint is an ordinary client that must first notice its own disconnection and thus when we do GetNameOwner() com.example.Spearmint is reported as having no owner. 5. service_bus_name_owner_change() sets bus_name_good = false, and since service_good() returns false for a Type=dbus service without its name, service_enter_running() stops the (perfectly healthy, other than dbus kicking the bucket temporarily) service. In other words, a broker restart can tear down running Type=dbus services that did nothing wrong, really. So, to fix this, we can use the fact that a NameOwnerChanged signal is definitive. That is, the name's owner changed while we were connected, so a service that crashes or drops its name can still be stopped immediately. By contrast a GetNameOwner() reply is less authoritative. There is, importantly, no change to how losing the name on a live connection works, we only give grace on reconnect.
…unnable probe Make sure the tests can tell the difference between an actual denial and a probe that simply cannot be run. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Centralize the checks for RestrictFileSystemAccess= requirements. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Right now RestrictFileSystemAccess= checks that a file comes from a trusted dm-verity device, but it doesn't guard against mapping that file and making use of COW modifications. So a MAP_PRIVATE mapping of a trusted binary can be made writable, modified, and executed, while the vma still points to the trusted file and the pages are attacker-controlled anonymous copies. Enforce W^X: deny any mapping that is writable and executable at once, and refuse to make a private file mapping executable once it has been modified through copy-on-write. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
PTRACE_POKETEXT and PTRACE_POKEDATA use FOLL_FORCE to forcibly write into a tracee's memory. That includes executable read-only pages. If we restrict execution to trusted binaries PID 1 must enforce that it's impossible to abuse this to get untrusted code execution. Note, that usermodehelpers are not subject to PID 1's seccomp filters. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Test seccomp_restrict_ptrace(). Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
…enabled PTRACE_POKETEXT and PTRACE_POKEDATA use FOLL_FORCE to forcibly write into a tracee's memory. That includes executable read-only pages. If we restrict execution to trusted binaries PID 1 must enforce that it's impossible to abuse this to get untrusted code execution. Note, that usermodehelpers are not subject to PID 1's seccomp filters. Install a ptrace seccomp filter from PID 1 if RestrictFileSystemAccess= is requested. Every process it spawns will inherit it. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
…cess= is enabled
In addition to PTRACE_POKE{TEXT,DATA} /proc/<pid>/mem can also be used
to FOLL_FORCE write into a trusted process's executable pages. Require
proc_mem.force_override=never.
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
The kernel silently ignores command line options it doesn't know. So a kernel without CONFIG_PROC_MEM_* would pass the command line check while still allowing FOLL_FORCE memory overwrites. Write one byte through /proc/self/mem into a private read-only anonymous page. That can only succeed with FOLL_FORCE writes supported. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Describe the system-wide W^X rule and the new proc_mem.force_override=never and seccomp prerequisites, and drop the advice to block writable and executable mappings by other means, which the policy now does itself. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Add tests for all new W^X protection mechanisms. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
…ystemd#43511) Currently `RestrictFileSystemAccess` isn't trying hard enough to enforce W^X and thus there are still places where an executable and writable mapping can be created: * `/proc/<pid>/mem` `FOLL_FORCE` writes * `PTRACE_POKE{TEXT,DATA}` `FOLL_FORCE` writes * `MAP_PRIVATE | MAP_EXECUTABLE` memory mappings Let's block all of these mechanisms. Do note that `RestrictFileSystemAccess` is still in very active development and also cannot meaningfully be used on general purpose distros and systems. Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Cancellation propagation: - sd_fiber_resume() now stashes the resume value even when the target fiber isn't suspended yet (INITIAL or running/READY); the next fiber_swap() consumes it without yielding to the event loop. Interruptions are sticky once queued so a concurrent async wakeup (an io_uring CQE, a later timer) can't silently override them: both -ECANCELED and -ETIME resist being overwritten by a normal completion, with -ECANCELED outranking -ETIME — a cancellation may escalate a pending timeout, but a timeout may neither downgrade a pending cancellation nor be lost. - Each fiber carries a monotonic interruption counter, exposed as sd_fiber_interrupt_count(), bumped only when an interruption (a cancellation or an SD_FIBER_TIMEOUT firing) is delivered to it, never by ordinary future completions. - sd_future_cancel_wait_unref() uses that counter to forward interruptions without fabricating phantom ones. Around each internal sd_fiber_await() it snapshots the counter and remembers the await's value only when the count advanced — i.e. only a value delivered as part of an interruption targeting the calling fiber, not the awaited future's own (possibly negative, e.g. -ECANCELED) resolution surfacing through the resume callback. Once the future has finally resolved it re-queues the held value via sd_fiber_resume(self, ...) so the fiber's next sd_fiber_suspend() / sd_fiber_yield() observes it. The counter is what distinguishes a genuine interruption from the future merely resolving negative — which the return value and the future's state alone cannot, even when the two coincide in a single loop iteration — and it keeps a later iteration's completion from clobbering an interruption seen earlier. Together these let sd_future_cancel_wait_unref() forward interruptions (cancellations / outer SD_FIBER_TIMEOUT firings) instead of silently swallowing them, while never re-queueing a value that wasn't actually an interruption. FIBER_STATE_CANCELLED is dropped in the process: fiber_cancel() now queues -ECANCELED through the same sd_fiber_resume() path (with an idempotency check so fiber_on_exit()'s two-pass arm-then-dispatch cycle still works), and fiber_swap() delivers it via the standard result_pending branch instead of a dedicated state-machine check. Net effect: one enum value gone, the cancel path goes through exactly the same plumbing as any other async wakeup. Decouple unreferencing a future from resolving it: futures must now be resolved by the time they're unreffed. To make this easier we introduce sd_future_cancel_unref() (plus _unrefp / array variants), which cancels and then unrefs. We use it everywhere where we can't assume we're on a fiber and thus can't wait for the future to resolve after cancelling. All future kinds we support today except fibers cancel synchronously, so this isn't a problem in practice. Even fibers cancel synchronously after creation until they're scheduled once by the event loop, after which they cancel asynchronously. Slot-based callbacks: Replaces the single-callback-per-future model (sd_future_set_callback, sd_future_get_userdata, and the wait_future indirection in sd_fiber_await) with explicit, awaiter-owned slots: - sd_future_add_callback() returns a refcounted sd_future_slot the caller owns via _cleanup_(sd_future_slot_unrefp). A future can have many slots; dropping a slot deregisters that one callback cleanly without resolving or otherwise touching the future. NULL ret_slot makes the slot 'floating' — its lifetime is bound to the future. - Callbacks never run inline. Each slot owns a one-shot defer event source and a one-shot exit event source on the future's event loop; the one applicable to the loop's current phase is armed when the future resolves — or at add time when the slot is attached to an already-RESOLVED future. Slots inherit the calling fiber's priority when added from inside one. Deferring unconditionally keeps the resolver free of reentrancy concerns (slot-set mutation during iteration, callbacks dropping the future's last ref) and frees callers from having to reason about whether a callback might fire before sd_future_add_callback() returns. - sd_fiber_await() registers a slot on the target directly and drops it on scope exit — no more wait_future allocation, no auto-set callback on sd_future_new(). sd_fiber_timeout() and sd_fiber_sleep() likewise install their resume callback explicitly via sd_future_add_callback(). - sd_future_new_wait() / sd_future_set_callback() / sd_future_get_userdata() are gone; existing callers in sd-bus, qmp-client, event-future, and fiber-io migrate to the slot API. sd_future_resume_callback() is exposed as a shared helper for the common "resume my fiber when this future settles" wiring. sd_future_new() now takes the sd_event the future belongs to, and sd_future_get_event() exposes it. This lets the rest of the API (slot dispatch, group child tracking, defer futures) avoid plumbing the event through separately. Future groups and defer futures: Adds sd_future_group, modelled on asyncio.TaskGroup: aggregate N child futures with a policy (default = wait-all-fail-fast; SD_FUTURE_GROUP_WAIT_ANY resolves on the first settled child; SD_FUTURE_GROUP_IGNORE_ERRORS collects all results without cancelling siblings on error). Policy is configured up-front via sd_future_group_set_policy() before any child is added — once children are in flight, the resolution mechanics are locked in. On group failure, the parent fiber that created the group is cancelled so it observes the failure even if it hasn't started awaiting yet — and not cancelled when it's already inside sd_future_group_await(), nor when the parent itself is the fiber driving the cancel. Finalize-time draining ensures the group only resolves once every child has actually settled, even if siblings need a round-trip through the event loop to honor a cancel. Also adds sd_future_new_defer() for one-shot 'resolve on next event loop iteration' futures. Tests cover the queued-while-running behavior of sd_fiber_resume() and the stickiness of -ECANCELED and -ETIME (including -ECANCELED outranking -ETIME); the propagation guarantees of sd_future_cancel_wait_unref() for both timeouts and external cancellations (from main and from a peer fiber), plus the counter-based discrimination — no phantom cancellation when the future resolves -ECANCELED on its own, a genuine cancellation preserved when it collides with the future resolving in the same iteration, and an interruption not clobbered by a later iteration's negative resolution; and a new test-future-group.c with coverage for each policy, parent-cancel propagation (including the parent-drives-cancel skip path), child-drain-on-resolve, add-rejected-during-finalize, slot lifecycle, and the already-resolved add_callback path. Co-developed-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Daan De Meyer <daan@amutable.com>
daandemeyer
force-pushed
the
push-tpznxutwnzzt
branch
from
September 9, 2026 07:56
4924a75 to
794868d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cancellation propagation:
sd_fiber_resume() now stashes the resume value even when the target fiber isn't suspended yet (INITIAL or running/READY); the next fiber_swap() consumes it without yielding to the event loop. -ECANCELED is sticky once queued, so a concurrent async wakeup can't override a pending cancellation.
sd_future_cancel_wait_unref() remembers the last non-zero return from its internal sd_fiber_await() across loop iterations and, once the future has finally resolved, re-queues it via sd_fiber_resume(self, ...) so the calling fiber's next sd_fiber_suspend() / sd_fiber_yield() observes it. Previously those values were discarded.
Together these let sd_future_cancel_wait_unref() forward interruptions (cancellations / outer SD_FIBER_TIMEOUT firings) instead of silently swallowing them.
FIBER_STATE_CANCELLED is dropped in the process: fiber_cancel() now queues -ECANCELED through the same sd_fiber_resume() path (with an idempotency check so fiber_on_exit()'s two-pass arm-then-dispatch cycle still works), and fiber_swap() delivers it via the standard result_pending branch instead of a dedicated state-machine check. Net effect: one enum value gone, the cancel path goes through exactly the same plumbing as any other async wakeup.
Decouple unreferencing a future from resolving it: futures must now be resolved by the time they're unreffed. To make this easier we introduce sd_future_cancel_unref() (plus _unrefp / array variants), which cancels and then unrefs. We use it everywhere where we can't assume we're on a fiber and thus can't wait for the future to resolve after cancelling. All future kinds we support today except fibers cancel synchronously, so this isn't a problem in practice. Even fibers cancel synchronously after creation until they're scheduled once by the event loop, after which they cancel asynchronously.
Slot-based callbacks:
Replaces the single-callback-per-future model (sd_future_set_callback, sd_future_get_userdata, and the wait_future indirection in sd_fiber_await) with explicit, awaiter-owned slots:
sd_future_new() now takes the sd_event the future belongs to, and sd_future_get_event() exposes it. This lets the rest of the API (slot dispatch, group child tracking, defer futures) avoid plumbing the event through separately.
Future groups and defer futures:
Adds sd_future_group, modelled on asyncio.TaskGroup: aggregate N child futures with a policy (default = wait-all-fail-fast; SD_FUTURE_GROUP_WAIT_ANY resolves on the first settled child; SD_FUTURE_GROUP_IGNORE_ERRORS collects all results without cancelling siblings on error). Policy is configured up-front via sd_future_group_set_policy() before any child is added — once children are in flight, the resolution mechanics are locked in. On group failure, the parent fiber that created the group is cancelled so it observes the failure even if it hasn't started awaiting yet — and not cancelled when it's already inside sd_future_group_await(), nor when the parent itself is the fiber driving the cancel. Finalize-time draining ensures the group only resolves once every child has actually settled, even if siblings need a round-trip through the event loop to honor a cancel.
Also adds sd_future_new_defer() for one-shot 'resolve on next event loop iteration' futures.
Tests cover the queued-while-running behavior of sd_fiber_resume(), the propagation guarantees of sd_future_cancel_wait_unref() for both timeouts and external cancellations (from main and from a peer fiber), and a new test-future-group.c with coverage for each policy, parent-cancel propagation (including the parent-drives-cancel skip path), child-drain-on-resolve, add-rejected-during-finalize, slot lifecycle, and the already-resolved add_callback path.