s390x: keep the EXRL prefetch inside the code buffer - #89
Conversation
s390_irgen_EXRL reads its target straight out of the buffer being translated, at exrl_bytes + offset * 2, with nothing bounding the result. Nothing constrains the displacement field either, so a bogus or truncated EXRL points anywhere within 4GB of the buffer: lifting the seven bytes a75bffff c650ff as s390x is enough to kill the process. The field is also read as unsigned, unlike every other RIL-b handler, which turns a negative displacement into a read about 8GB ahead of the buffer and puts that address in the emitted lookup as well. Record the buffer base in disInstr_S390, sign-extend the displacement, and prefetch only when the whole six-byte target lies inside the buffer. Leaving last_execute_target at zero makes s390_irgen_EX emit the generic run-time lookup, which is what it already does when the target is unknown. The buffer is normally larger than the block being translated, so VexControl gains guest_bytes_size for the client to say how much of it may be read; without it only guest_max_bytes counts.
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Validation record for head
EXRL survey over the 67 s390x ELFs in angr/binaries
Other front ends are untouched by construction, and measured so: 27,918 lifts over X86, AMD64, ARMEL, AARCH64, MIPS32, MIPS64, PPC32, PPC64 and S390X from the same angr/binaries revision are byte-identical on baseline and head. Caveats: no s390x hardware was involved; this is translation evidence only. A target inside the code buffer but before |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Three s390x Before — the backward angr masterAfter — all three lift; the out-of-buffer targets fall back to the run-time lookup with this change |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS A corpus measurement, plus three pieces of provenance that make this change It is a rule this file already follows everywhere else. s390_irgen_EXRL guest_IA_curr_instr + offset * 2UL /* UInt, unsigned */
s390_irgen_LARL guest_IA_curr_instr + ((ULong)(Long)(Int)i2 << 1)
s390_irgen_BRASL guest_IA_curr_instr + ((ULong)(Long)(Int)i2 << 1)
Upstream fixed exactly this five years ago. Valgrind's It has been known here since 2018. angr/vex issue 21, "s390 EXLR will How often it fires. 300 objects drawn uniformly at random from the 7,354 The sign fix alone would not be enough, which is why the bounds check earns its Measured against a build of this branch with the client half applied: at the The corpus is not redistributable, so the objects are described by architecture session: sharpen |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS A second corpus draw, seeded independently of the one above, and the half that Arms. Two trees materialised with
Six bytes. Each lift in its own child so a fatal signal is visible, five Corpus. 300 s390x objects drawn uniformly at random from the catalogue,
What the 13 do here. Three finish Control. 258 of the 300 completed Four of the 300 are public. They are byte-identical to files tracked in One footnote on the line number. The rest of the corpus is not redistributable, so those objects are described session: sharpen |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Two notes from the pyvex work. Neither has changed this branch. Measured on this branch (vex
session: sharpen |
1 participant
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS
Problem
s390_irgen_EXRLreads its execute target straight out of the buffer beingtranslated, and nothing bounds where it reads. Six well-formed bytes take the
process down —
c660ffffffffisexrl %r6,.-2, the backward form compilersemit for literal-pool-relative
MVCandCLC:The sign of the displacement is the whole difference. A truncated encoding does
the same:
a75bffffc650ffisaghi %r5,-1followed by the first three bytes ofan
exrl, which is what a scanning CFG hands the lifter routinely. There is noexception to catch — the client process dies.
Root cause
The displacement is read unsigned, and the resulting pointer is never checked
against the buffer:
offsetisUInt, so a negative RIL-b displacement becomes a forward offset ofup to 8 GB. Of the 26 pc-relative RIL handlers in
priv/guest_s390_toIR.cthisis the only one that reads the field unsigned; 24 spell it
(ULong)(Long)(Int)i2 << 1. Where the read happens to land on mapped memory itdoes not fault at all: it fabricates an execute target out of whatever bytes are
there and bakes that into the IR.
Fix
Record the buffer base in
disInstr_S390, sign-extend the displacement toLong, and prefetch only when all six target bytes lie inside the buffer.Otherwise
last_execute_targetstays zero ands390_irgen_EXemits therun-time lookup it already emits for an unknown target, so the same six bytes
now lift and the lookup reads the right address:
The buffer's extent is the client's to state, so
VexControlgainsguest_bytes_size; zero means unstated, and the bound falls back toguest_max_bytes, which is what every existing caller gets. A target that liesin the caller's buffer but before the address it stated is deliberately not
prefetched;
lookback_amountcould allow that later.Testing
Regressions live in angr/pyvex#564, which carries the submodule bump and
tests/test_s390x_exrl.py. All six pass on this branch, five runs of five. Atthe merge base two are killed by SIGSEGV, one fails its assertion in all five
runs, and one is unstable — it reads uninitialised heap 8 KiB past the buffer
and passed four of five.
Validation: #89 (comment)
Merge order: this lands first, then angr/pyvex#564, whose
vexsubmodule is pinned to this head (88aa12d). Merging the pyvex half on its own leaves pyvex master's submodule pointing at a commit that is on no vex branch. Nothing in this repository reads async:line —.github/workflows/build.ymlis the only workflow and it is three build jobs — so the line below is a note to the reader.sync: angr/pyvex#564
session: sharpen