fix(rootfs/qemu-static): close 2 more races flagged by codex review of 3d4c8cd

Codex re-review of fix2 (commit 3d4c8cd) confirmed the original three
races are properly closed but identified two new ones, both producing
the same class of runtime failure: a parallel builder ends up running
chroot exec against globally-enabled qemu-arm with no qemu binary
inside its chroot.

1. Disable-failure path left a live owner. When 'echo 0 > qemu-arm'
   itself fails (no CAP_SYS_ADMIN), fix2 chose to leave the owner lock
   and the prior-state file in place, on the theory that the cleanup
   handler would no-op the restore at exit. But the owner stays
   counted as live for the entire build duration, so a concurrent
   builder enters its own setup, sees live_count > 1, takes the join
   branch, skips qemu binary deployment, and fails at chroot exec
   time. Fix: a failed disable now performs a full abort under the
   held control lock — rm state file, release owner, drop control,
   return 1. The aborted setup leaves no observable native window for
   anyone to join.

2. Join path did not verify the actual kernel state. fix2 trusted that
   live_count > 1 implies qemu-arm is disabled. But an external agent
   (admin running 'update-binfmts --enable qemu-arm', a stray service)
   can flip it back on during the active window. Our pre-flight
   arch-test then passes as a false positive — qemu interprets the
   ARM stub — and we skip qemu binary deployment, hitting the same
   runtime failure as case 1. Fix: in the join branch, observe the
   current qemu-arm state explicitly. If it is enabled, this is not
   a normal join — abort to qemu fallback with a warning naming the
   external mutator. This is a one-shot invariant check at join time,
   not a periodic re-verify (which codex previously argued against).

Reported-by: codex (PR #9769 lock-impl re-review)
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Igor Velkov <325961+iav@users.noreply.github.com>

Author: iav

lib/functions/rootfs/qemu-static.sh

@@ -267,10 +267,31 @@ function _native_armhf_setup_binfmt_elf() {
 	live_count="$(_native_armhf_live_owner_count_locked)"
 
 	if [[ "${live_count}" -eq 1 ]]; then
-		# We are the only live owner — first-owner. Record current qemu-arm
-		# state so cleanup can restore it state-preserving (see codex review:
-		# never blindly write 1 — if prior was disabled, leave disabled; if
-		# prior was missing, don't try to recreate the handler).
+		# We are the only live owner — first-owner. Before recording our
+		# prior_state, reconcile any stale state file from a previous
+		# active window that ended without running cleanup (e.g. SIGKILL /
+		# OOM kill / `kill -9` of the build between the post-disable echo 0
+		# and the trap firing). Without this branch the crashed window's
+		# left-behind disabled state would be observed as our new
+		# prior_state, and our own clean shutdown would never re-enable
+		# qemu-arm — turning a single crash into a permanent disablement.
+		if [[ -f "${_NATIVE_ARMHF_PRIOR_STATE}" ]]; then
+			declare stale_prior
+			stale_prior="$(cat "${_NATIVE_ARMHF_PRIOR_STATE}" 2> /dev/null || echo missing)"
+			if [[ "${stale_prior}" == "1" && -e /proc/sys/fs/binfmt_misc/qemu-arm ]]; then
+				declare crashed_observed
+				crashed_observed="$(_native_armhf_observe_qemu_arm_state)"
+				if [[ "${crashed_observed}" != "1" ]]; then
+					echo 1 > /proc/sys/fs/binfmt_misc/qemu-arm 2> /dev/null || true
+					display_alert "Native armhf via binfmt_elf" "stale active window detected (previous build crashed pre-cleanup, prior=${stale_prior}); qemu-arm reconciled to enabled before our first-owner observation" "wrn"
+				fi
+			fi
+			rm -f "${_NATIVE_ARMHF_PRIOR_STATE}"
+		fi
+
+		# Record current qemu-arm state so cleanup can restore it state-
+		# preserving (codex: never blindly write 1 — if prior was disabled,
+		# leave disabled; if prior was missing, don't recreate the handler).
 		declare prior_state
 		prior_state="$(_native_armhf_observe_qemu_arm_state)"
 		echo "${prior_state}" > "${_NATIVE_ARMHF_PRIOR_STATE}"
@@ -289,18 +310,41 @@ function _native_armhf_setup_binfmt_elf() {
 			1)
 				if ! echo 0 > /proc/sys/fs/binfmt_misc/qemu-arm 2> /dev/null; then
 					display_alert "Native armhf via binfmt_elf" "could not disable qemu-arm (no CAP_SYS_ADMIN?); falling back to qemu-arm-static emulation" "wrn"
+					# Full abort: failed disable must NOT leave us as a live
+					# native-mode owner. Otherwise a concurrent builder sees
+					# live_count > 1, takes the join branch, skips qemu
+					# binary deployment, and runs its chroot against still-
+					# enabled qemu-arm without the binary inside — runtime
+					# failure (codex review of fix2). Cleanup is performed
+					# UNDER control so prune+count by other builders sees
+					# the abort atomically.
 					rm -f "${_NATIVE_ARMHF_PRIOR_STATE}"
-					exec {control_fd}>&-
 					_native_armhf_release_owner_lock
+					exec {control_fd}>&-
 					return 1
 				fi
 				;;
 		esac
 	else
-		# Another live owner already disabled qemu-arm. Just join — no state
-		# to save (the first owner's state file is authoritative), no
-		# disable to perform. Cleanup is still registered so this owner's
-		# release decrements live_count back toward zero.
+		# Another live owner already disabled qemu-arm — or so we think.
+		# Verify the kernel state matches: if qemu-arm is observably
+		# enabled here despite a live first owner, an external agent
+		# (admin / `update-binfmts --enable qemu-arm`) flipped it back on
+		# during the active window. Our pre-flight arch-test then passes
+		# as a false positive (qemu interprets the stub) and we'd skip
+		# the qemu binary deployment, then run chroot exec against
+		# globally-enabled qemu-arm with no binary inside the chroot —
+		# runtime failure (codex review of fix2).
+		declare observed_join
+		observed_join="$(_native_armhf_observe_qemu_arm_state)"
+		if [[ "${observed_join}" == "1" ]]; then
+			display_alert "Native armhf via binfmt_elf" "${live_count} live owner(s) but qemu-arm is observably enabled (external agent re-enabled it); aborting join, falling back to qemu-arm-static emulation" "wrn"
+			_native_armhf_release_owner_lock
+			exec {control_fd}>&-
+			return 1
+		fi
+		# Cleanup is still registered so this owner's release decrements
+		# live_count back toward zero.
 		add_cleanup_handler trap_handler_native_armhf_restore_qemu_arm
 		display_alert "Native armhf via binfmt_elf" "joining ${live_count} existing native-armhf owner(s); shared qemu-arm state preserved" "info"
 	fi
@@ -311,13 +355,13 @@ function _native_armhf_setup_binfmt_elf() {
 	# arm64-compat-vdso, PR #9284) — qemu-arm interprets the stub.
 	if ! arch-test armhf > /dev/null 2>&1; then
 		display_alert "Native armhf via binfmt_elf" "post-disable verification failed (host kernel lacks COMPAT_VDSO — see extensions/arm64-compat-vdso); restoring and falling back to emulation" "wrn"
-		# Restore is via cleanup handler when owner_lock releases. Mark
-		# ourselves as not-owner-of-native-mode so the cleanup invocation
-		# that runs immediately on this return path triggers restore.
+		# Drop control lock so the handler can re-acquire it. Do NOT
+		# release the owner lock here — the handler must release it UNDER
+		# control so live_count reflects only OTHER owners and the
+		# state-preserving restore decision uses a consistent snapshot.
+		# Releasing owner outside the control section was the race codex
+		# flagged in the previous round.
 		exec {control_fd}>&-
-		_native_armhf_release_owner_lock
-		# Manually invoke the same restore path so the failed setup leaves
-		# binfmt_misc as we found it; subsequent owners (if any) re-evaluate.
 		trap_handler_native_armhf_restore_qemu_arm
 		return 1
 	fi
@@ -344,22 +388,30 @@ function _native_armhf_release_owner_lock() {
 # disabled state. Restore writes the prior state observed at first-owner
 # time (1 / 0 / missing) — never blindly 1.
 function trap_handler_native_armhf_restore_qemu_arm() {
-	# Drop our own ownership first so the live-count below reflects only
-	# OTHER live owners. Skip cleanup entirely if we never acquired.
+	# Skip cleanup entirely if we never acquired an owner lock.
 	[[ -n "${_native_armhf_owner_fd:-}" ]] || return 0
-	_native_armhf_release_owner_lock
 
-	# Take the control lock to make prune+count+restore atomic with respect
-	# to any new builder racing to become an owner right now.
+	# Take the control lock BEFORE releasing our owner lock. The previous
+	# revision did the reverse and codex flagged the race: between fd
+	# close and control acquire, a fresh setup could enter, see itself as
+	# the only live owner (live_count == 1), observe the now-disabled
+	# qemu-arm and write that as its prior_state — so our subsequent
+	# restore would read the overwritten state and fail to re-enable.
+	# Holding control across release+count+restore makes the transition
+	# atomic with respect to any concurrent setup.
 	declare control_fd
 	exec {control_fd}> "${_NATIVE_ARMHF_CONTROL_LOCK}"
-	flock "${control_fd}" || {
-		# Lock failed — best effort cleanup without restore. Worst case is
-		# qemu-arm stays in whatever state it is now; next build will
-		# reconcile.
+	if ! flock "${control_fd}"; then
+		# Lock failed — best effort. Drop our owner so we are not counted
+		# live forever; next build will reconcile.
 		exec {control_fd}>&-
+		_native_armhf_release_owner_lock
 		return 0
-	}
+	fi
+
+	# Drop our own ownership UNDER control. live_count below now reflects
+	# only OTHER live owners.
+	_native_armhf_release_owner_lock
 
 	declare live_count
 	live_count="$(_native_armhf_live_owner_count_locked)"