Restore the guest X8 when the shim drops its frame

Merged
JO
1mo ago
+1492-94

Fixes #379.

A signal delivered while the vCPU is at EL0 with PC on an svc that has not
executed yet is saved with that PC, correctly and without the drop-frame marker.
The handler returns, signal_rt_sigreturn restores all 31 GPRs from the frame
and then writes the marker 2 over X8, and exec_drop_frame pops and ERETs
without restoring a register. The svc resumes with X8 = 2 and runs as
syscall 2. Dispatch finds nothing and answers -ENOSYS to a call the guest made
as something else.

The value goes in the frame's own X8 slot, and exec_drop_frame reloads X8
from it before the pop. That slot is EL1-only memory on this vCPU's exception
stack, unshared, holding nothing live once the frame is being dropped, and
retired by the pop, so it cannot collide the way a register does. No second
register can carry it instead: every GPR on that tail is meant to be final EL0
state, which is why X7's ptrace request is already left unwritten there.

The slot alone does not close the window, which was measured rather than argued.
A signal delivered after the rt_sigreturn but before the vCPU resumes
snapshots the live registers, where X8 is still the marker and ELR_EL1 is
the unexecuted svc. rt_sigreturn therefore also parks the value it published
and deliver_signal_locked takes it in place of the register, bounded by a
forget at the single hv_vcpu_run call site. An inline ptrace stop that moves
the PC re-keys the record. With only the slot in place the 32 nested rounds fail
and nothing else does; with the slot and the park but no forget, the rounds pass
and the stale-park phase fails.

handle_brk had the same defect on a tail of its own and takes the same reload.
That reload needs a publisher too: vcpu_handle_brk publishes the live X8
into the slot when a ptrace stop returns, so an X8 the tracer wrote at a BRK
stop is the one the tracee resumes with. tests/test-ptrace-interrupt.c closes
on that stop and fails without the publish.

tests/test-shim-sigreturn-x8.c is deterministic and needs no race: 64 rounds
on the syscall route, two rounds that reach the same drop tail from HVC #9, a
BRK phase, and a stale-park phase. On afdcfce all 64 rounds fail, both
HVC #9 rounds enter their handler with 0x2 where the guest held 0xa5, and
the BRK phase reads 0x2 where the guest held 0xc3; every phase passes
here. Every assertion is plain Linux signal ABI, so it is registered in
tests/test-matrix.sh rather than exempted. It was run against the qemu
reference lane by hand and passes there. A full lane was not run, so neither
EXPECTED_BASELINES floor moves.

The reproduction the issue carries, a raw FUTEX_WAIT loop with a signal
landing on it, was run interleaved so both binaries met the same machine: 5
failures in 400 runs on afdcfce against 0 in 400 here, and 161 in 400 against
0 in 400 under eight concurrent compile jobs. That machine is shared, so the
unloaded column is a floor rather than a rate: the same pair read 53 in 400
while another build was running. The loaded column is the one to read.

scripts/check-svc-tails.py gains the structural rules. A frame-dropping tail
must load X8 from the frame before it pops, at the offset the gate reads out
of signal.c rather than trusting the two sides to match by eye, and it must
still pop. exec_drop_frame and handle_brk are both held to that. Every
hv_vcpu_run call site must forget the parked value first. The gate cannot hold
the substitution itself, and the commit says so: that half rests on the test,
which fails deterministically on it.

Two pre-existing neighbours are recorded rather than fixed. A signal delivered
in the epilogue of a page-table syscall records the TLBI wire values as the
guest's X8 through X11, which is tracked as #384. A ptrace stop taken inside
the rt_sigreturn epilogue shows the tracer the marker through
PTRACE_GETREGSET.

Summary by cubic

Fixes #379: a signal delivered while the vCPU has PC on an unexecuted svc no longer resumes that svc with the drop-frame marker (2) in X8, so the guest's syscall runs as its own number instead of as syscall 2 and returning -ENOSYS.

Bug Fixes

  • exec_drop_frame reloads X8 from the frame's own X8 slot before popping; signal_rt_sigreturn publishes the restored value there, and the write is bounded to the per-thread EL1 stack region so a bad SP_EL1 logs instead of corrupting the shim-globals cache.
  • signal_rt_sigreturn also parks the restored X8; a delivery in the same epilogue takes the parked value, the run loop forgets it before every hv_vcpu_run(), and signal_repark_sigreturn_x8() re-keys it after a PTRACE_INTERRUPT stop changes the resume PC.
  • handle_brk now keeps its frame live across the HVC, reloads X8 from the slot, then pops; a ptrace stop there publishes the X8 the tracer wrote so the guest resumes with it.
  • Both HVC #9 W^X tails branch to the same drop tail, so their SIGSEGV handlers now enter with the guest's X8 rather than the marker.
  • Pre-existing shadowing on the syscall-return path is left as-is: delivery after a page-table syscall still hands X9 through X11 to EL0, and ptrace stops on this tail still show the marker via PTRACE_GETREGSET.

Validation

  • Adds deterministic tests/test-shim-sigreturn-x8.c covering plain and nested returns, the stale-park window, both W^X tails, and the BRK tail; tests/test-ptrace-interrupt.c now closes on a BRK stop whose tracer writes X8.
  • scripts/check-svc-tails.py now checks the X8 reload offset against signal.c, requires signal_forget_sigreturn_x8() above every hv_vcpu_run(), and holds both exec_drop_frame and handle_brk to the reload.
  • The reproduced FUTEX_WAIT loop failed 5/400 on base vs 0/400 here, and 161/400 vs 0/400 under concurrent build load.
  • The qemu matrix lane wasn't run, so EXPECTED_BASELINES is unchanged.