Skip to content

Commit ecde3c1

Browse files
committed
fix(rootfs/qemu-static): hold EX during state-check+disable; K-builder verifies state=1 after SH-NB
Addresses Codex P2 review feedback on PR #113 / armbian#9769: - Race in N-builder entry (line:239): EX-NB → release → re-take SH split let a K-builder slip in between observation and the state-check write, silently disabling qemu-arm while K relied on it. Restructure to hold EX during state check + echo 0, then atomically downgrade EX→SH on the same fd via Linux flock(2)'s flc_lock transition. SH waiters are granted only after our SH is in place and qemu-arm is already 0. - K-builder symmetric check: after acquiring the emulation-mode SH-NB, verify state=1; if 0, concurrent N-builders have disabled qemu-arm and the killswitch cannot honor its contract — bail with a clear error rather than running a broken anchor. - Cleanup-ordering invariant comment was inverted. add_cleanup_handler PREPENDS, so later registrations run FIRST. The invariant survives because setup is called before mount_chroot in both call sites; the comment now states the actual mechanism and documents the failure mode if a future call site inverts the order. Assisted-by: Claude:claude-opus-4.7
1 parent 5cab79e commit ecde3c1

1 file changed

Lines changed: 52 additions & 25 deletions

File tree

lib/functions/rootfs/qemu-static.sh

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,19 @@ function _native_armhf_setup_binfmt_elf() {
190190
if [[ -e /proc/sys/fs/binfmt_misc/qemu-arm ]] &&
191191
{ exec {_native_armhf_emul_lock_fd}< /proc/sys/fs/binfmt_misc/qemu-arm; } 2> /dev/null &&
192192
flock -s -n "${_native_armhf_emul_lock_fd}"; then
193-
add_cleanup_handler trap_handler_native_armhf_release_emul_lock
194-
display_alert "Native armhf via binfmt_elf" "killswitch active; emulation-mode SH-lock acquired (blocks concurrent native-armhf switchover)" "info"
193+
# K-builder invariant: qemu-arm must be enabled. If state=0 here,
194+
# concurrent N-builders have already disabled it — the emulation
195+
# anchor would be a lie and the K-builder's chroot would fail to
196+
# exec qemu-arm-static. Bail rather than silently coexist.
197+
if [[ "$(_native_armhf_observe_qemu_arm_state)" == "1" ]]; then
198+
add_cleanup_handler trap_handler_native_armhf_release_emul_lock
199+
display_alert "Native armhf via binfmt_elf" "killswitch active; emulation-mode SH-lock acquired (blocks concurrent native-armhf switchover)" "info"
200+
else
201+
exec {_native_armhf_emul_lock_fd}>&-
202+
unset _native_armhf_emul_lock_fd
203+
display_alert "Native armhf via binfmt_elf" "killswitch requested but qemu-arm already disabled by concurrent native-armhf builders" "err"
204+
exit_with_error "cannot honor NATIVE_ARMHF_ON_ARM64=no: concurrent native-armhf builders have disabled qemu-arm. Wait for them to finish or run on a separate host."
205+
fi
195206
else
196207
[[ -n "${_native_armhf_emul_lock_fd:-}" ]] && exec {_native_armhf_emul_lock_fd}>&-
197208
unset _native_armhf_emul_lock_fd
@@ -228,32 +239,41 @@ function _native_armhf_setup_binfmt_elf() {
228239
return 1
229240
fi
230241

231-
# EX-NB probe BEFORE acquiring our own SH (otherwise our own SH would
232-
# block the probe — flock counts per-OFD, our two fds on the same file
233-
# would interfere). EX-NB success means zero other SH-holders. Failure
234-
# with state="1" identifies a killswitch K-builder holding the
235-
# emulation-mode anchor — switching qemu-arm off would corrupt their
236-
# chroot exec routing. Failure with state="0" is a peer N-builder in
237-
# joiner-territory.
242+
# Take EX-NB first to determine whether we are alone or joining. While
243+
# we hold EX, no SH/EX can enter, so state observation and the disable
244+
# write are atomic w.r.t. concurrent K- or N-builders. Then downgrade
245+
# EX → SH on the same fd; Linux flock(2) performs the transition under
246+
# the inode's flc_lock, granting pending SH waiters only after our SH
247+
# is in place — by which time qemu-arm is already 0 and any joiner
248+
# sees joiner-territory (state=0).
249+
#
250+
# EX-NB failure means someone else holds SH. State distinguishes:
251+
# state=1 → killswitch K-builder holds the emulation-mode anchor;
252+
# switching qemu-arm off would corrupt its chroot exec.
253+
# state=0 → peer N-builder; we are a joiner, take SH without writing.
238254
if flock -x -n "${_native_armhf_lock_fd}"; then
239-
flock -u "${_native_armhf_lock_fd}"
255+
if [[ "$(_native_armhf_observe_qemu_arm_state)" == "1" ]]; then
256+
if ! echo 0 > /proc/sys/fs/binfmt_misc/qemu-arm 2> /dev/null; then
257+
display_alert "Native armhf via binfmt_elf" "could not disable qemu-arm (no CAP_SYS_ADMIN?); falling back to qemu-arm-static emulation" "wrn"
258+
exec {_native_armhf_lock_fd}>&-
259+
unset _native_armhf_lock_fd
260+
return 1
261+
fi
262+
fi
263+
if ! flock -s "${_native_armhf_lock_fd}"; then
264+
display_alert "Native armhf via binfmt_elf" "could not downgrade EX→SH on binfmt_misc/qemu-arm; falling back to qemu emulation" "wrn"
265+
exec {_native_armhf_lock_fd}>&-
266+
unset _native_armhf_lock_fd
267+
return 1
268+
fi
240269
elif [[ "$(_native_armhf_observe_qemu_arm_state)" == "1" ]]; then
241270
exec {_native_armhf_lock_fd}>&-
242271
unset _native_armhf_lock_fd
243272
display_alert "Native armhf via binfmt_elf" "concurrent build holds emulation-mode lock (NATIVE_ARMHF_ON_ARM64=no)" "err"
244273
exit_with_error "cannot enable native armhf: concurrent build with NATIVE_ARMHF_ON_ARM64=no holds emulation lock. Wait for it to finish or run on a separate host."
245-
fi
246-
247-
if ! flock -s -w 30 "${_native_armhf_lock_fd}"; then
248-
display_alert "Native armhf via binfmt_elf" "could not acquire shared flock on binfmt_misc/qemu-arm within 30s; falling back to qemu emulation" "wrn"
249-
exec {_native_armhf_lock_fd}>&-
250-
unset _native_armhf_lock_fd
251-
return 1
252-
fi
253-
254-
if [[ "$(_native_armhf_observe_qemu_arm_state)" == "1" ]]; then
255-
if ! echo 0 > /proc/sys/fs/binfmt_misc/qemu-arm 2> /dev/null; then
256-
display_alert "Native armhf via binfmt_elf" "could not disable qemu-arm (no CAP_SYS_ADMIN?); falling back to qemu-arm-static emulation" "wrn"
274+
else
275+
if ! flock -s -w 30 "${_native_armhf_lock_fd}"; then
276+
display_alert "Native armhf via binfmt_elf" "could not acquire shared flock on binfmt_misc/qemu-arm within 30s; falling back to qemu emulation" "wrn"
257277
exec {_native_armhf_lock_fd}>&-
258278
unset _native_armhf_lock_fd
259279
return 1
@@ -290,9 +310,16 @@ function trap_handler_native_armhf_release_emul_lock() {
290310
# the build's subshells (umount / SDCARD / MOUNT teardown). BSD flock is per-
291311
# OFD, so a forked subshell inheriting our SH-fd shares the same lock entry —
292312
# the LOCK_EX-NB probe below would falsely block on the inherited fd of a
293-
# still-alive child. add_cleanup_handler runs in registration order; the
294-
# umount handlers register first, so by the time we run, the docker container
295-
# is dead and its child-tree with it. Verified empirically (SIGINT mid-chroot).
313+
# still-alive child. add_cleanup_handler PREPENDS to the handler list and
314+
# run_cleanup_handlers iterates it in order, so later-registered handlers
315+
# run first. Our setup is invoked before mount_chroot in both call sites
316+
# (rootfs-create.sh and rootfs-image.sh), so mount_chroot's umount handlers
317+
# are registered after ours, sit at the head of the list, and execute before
318+
# us — by the time we run, the chroot is unmounted and inheriting subshells
319+
# have exited. Verified empirically (SIGINT mid-chroot). If a future call
320+
# site registers our setup AFTER mount_chroot, this invariant inverts and
321+
# the EX-NB probe will spuriously fail; the documented escape hatches are
322+
# POSIX F_SETLK on a helper fd or explicit descendant-kill.
296323
function trap_handler_native_armhf_restore_qemu_arm() {
297324
[[ -n "${_native_armhf_lock_fd:-}" ]] || return 0
298325
exec {_native_armhf_lock_fd}>&-

0 commit comments

Comments
 (0)