Skip to content

portal: periodic poll heartbeat + narrow the sysctl parent-dir guard - #96

Open
lacraig2 wants to merge 2 commits into
mainfrom
portal-periodic-poll-and-sysctl-guard
Open

portal: periodic poll heartbeat + narrow the sysctl parent-dir guard#96
lacraig2 wants to merge 2 commits into
mainfrom
portal-periodic-poll-and-sysctl-guard

Conversation

@lacraig2

@lacraig2 lacraig2 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Two independent portal fixes, both found while bringing up a firmware whose
service manager is driven by a watchdog heartbeat and which exposes a vendor
bootloader environment under /proc/sys. Each is generally useful; neither is
device-specific.

1. portal_sysctl: refuse only fs-backed subtrees, not every new sysctl dir

handle_op_sysctl_create_file guarded a pre-5.0 register_sysctl() crash by
refusing to create an entry whose parent sysctl directory did not already
exist
. That is far too broad — register_sysctl() creates a genuinely new
sysctl directory just fine on those kernels.

The crash it protects against is specific to filesystem-backed, non-sysctl
subtrees (/proc/sys/fs/binfmt_misc being canonical): the intermediate component
can't be created, the call returns -EROFS, and the cleanup path then faults in
drop_sysctl_table()->rb_erase().

The failure mode this caused is worse than a cosmetic drop: embedded firmware
commonly exposes a bootloader/board environment as its own /proc/sys subtree and
then spins in a boot script waiting for it to appear, so silently dropping the
registration hangs the boot forever instead of degrading.

Fix: narrow the guard to an explicit fs-backed list via a new
igloo_sysctl_dir_is_fs_backed(), mirroring the Python _NON_SYSCTL_SUBTREES list
in hyperfile/sysctl.py (which normally rejects these before they reach the
driver — this is the driver-side backstop). igloo_sysctl_dir_exists() is retained
as __maybe_unused.

Verified on a mipseb/4.10 guest: sysctl-registration failures 3 → 0, and a boot
that previously hung waiting for a vendor sysctl node proceeds.

2. portal_devfs: add an optional periodic poll heartbeat timer

Some firmware main loops are driven entirely by a device that delivers a periodic
hardware event
(a watchdog is the common case): the loop sits in epoll_wait()
with timeout=-1 on a set containing that device and advances its service state
machine once per tick.

Neither existing poll behaviour models that:

This cannot be fixed host-side: with timeout=-1 the kernel only re-invokes
->poll() after a wake on the device's wait queue, so the periodic wake must
originate from a kernel timer in the driver.

Adds an optional poll_interval_ms to the devfs create request. When > 0, the
entry gets a self-rearming timer that sets poll_ready and wakes poll_wq every
interval; the poll proxy reports POLLIN|POLLRDNORM for exactly one poll() per
tick (consumed via atomic_xchg) and not-ready in between.

  • POLLOUT is deliberately not reported — a constantly-writable mask would
    re-spin a loop that also watches EPOLLOUT.
  • poll_interval_ms == 0 preserves current behaviour exactly, so this is inert
    for every existing node
    .
  • timer_setup()/from_timer() landed in 4.14, so the callback is
    version-guarded and falls back to setup_timer() with the entry pointer in
    ->data.

Verified with a guest whose service manager polls a watchdog: both the spin
(139k failed-read log lines) and the deadlock disappear, and the loop advances at
the configured cadence.

Build

./build.sh --versions 4.10 --targets mipseb+++ SUCCESS: mipseb 4.10 on
current main.

Unrelated snag worth a separate fix: build.sh runs under set -u but
dereferences $PENGUIN_HOST_MOUNT_FROM/_TO unconditionally in
rewrite_mount(), so a plain local ./build.sh aborts with
PENGUIN_HOST_MOUNT_FROM: unbound variable unless both are exported (even
empty). The comment above it says the unset case is meant to work.

Consumer

The host half (poll: {model: periodic, interval_ms: N}) is a separate penguin PR;
this driver change is what makes it possible. A rehosting that depends on both is
staged as a draft.

handle_op_sysctl_create_file guarded against a pre-5.0 register_sysctl() crash by
refusing to create an entry whose parent sysctl directory did not already exist.
That guard was far too broad: register_sysctl() creates a genuinely new *sysctl*
directory just fine on those kernels. The crash it was protecting against is
specific to filesystem-backed, non-sysctl subtrees (/proc/sys/fs/binfmt_misc is
the canonical one), where the intermediate component cannot be created, the call
returns -EROFS, and the cleanup path then faults in
drop_sysctl_table()->rb_erase().

The over-broad check silently dropped legitimate vendor sysctl directories. That
is not a cosmetic loss: embedded firmware commonly exposes a bootloader/board
environment as its own /proc/sys subtree and then spins in a boot script waiting
for it to appear, so dropping the registration hangs the boot forever rather than
degrading.

Narrow the guard to an explicit filesystem-backed list via a new
igloo_sysctl_dir_is_fs_backed() helper, mirroring the Python _NON_SYSCTL_SUBTREES
list in hyperfile/sysctl.py (which normally rejects these before they reach the
driver; this is the driver-side backstop). igloo_sysctl_dir_exists() is kept but
marked __maybe_unused so the traversal helper is still available.

Verified on a mipseb/4.10 guest: sysctl-registration failures 3 -> 0, and a boot
that previously hung waiting for a vendor sysctl node now proceeds.
Some firmware main loops are driven entirely by a device that delivers a periodic
hardware event -- a watchdog being the common case. Such a loop typically sits in
epoll_wait() with timeout=-1 on a set containing that device and advances its
state machine once per tick.

Neither existing poll behaviour models this. Always-ready makes epoll return
immediately every iteration, so the loop spins and starves the emulator.
Never-ready (the issue #77 wait-queue path) parks the loop forever, so it
deadlocks after the last event it was waiting on. Crucially this cannot be fixed
host-side: with timeout=-1 the kernel only re-invokes ->poll() after a wake on the
device's wait queue, so the periodic wake has to originate from a kernel timer in
the driver.

Add an optional poll_interval_ms to the devfs create request. When it is > 0 the
entry gets a self-rearming timer that sets poll_ready and wakes poll_wq every
interval; the poll proxy then reports POLLIN|POLLRDNORM for exactly one poll() per
tick (consumed with atomic_xchg) and not-ready in between. POLLOUT is deliberately
not reported: a constantly-writable mask would re-spin a loop that also watches
EPOLLOUT. 0 keeps the previous behaviour exactly, so this is inert for every
existing node.

timer_setup()/from_timer() landed in 4.14, so the callback is version-guarded and
falls back to setup_timer() with the entry pointer in ->data on older kernels.

Built for mipseb/4.10. Verified with a guest whose service manager polls a
watchdog: the spin (139k failed-read log lines) and the deadlock both disappear
and the loop advances at the configured cadence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant