From d211ff0770e6796dbb1fe0e6aaa36e1b99f1900e Mon Sep 17 00:00:00 2001 From: Arti Zirk Date: Sat, 31 Oct 2026 22:20:58 +0200 Subject: [PATCH] fix(badapple): --no-load must not wait for ICMP; survive a slow readback Two faults the 60 fps work exposed, both in the delivery tooling: * `demo.sh --no-load` waited for the terminal to answer ping. A loaded payload owns the LANCE and stops the OS's stack with it, so that gate can never pass once the payload is running -- the recovery path documented for "already jumped" deadlocked for its full 180 s. The gate now runs only when this invocation is going to load the payload. * `rdebug_load.py` used a 5 s socket timeout and no retry, so a readback peek that timed out on a freshly booted OS failed a load whose pokes had already been acknowledged. Timeout is 15 s (--timeout) and the readback retries once before it is believed. Tests: the retry path is pinned with a fake remote that times out on the first peek (one timeout, then the successful pass over every chunk) and one that never answers (it must raise rather than loop). 9 tests in harness/badapple pass. --- harness/badapple/demo.sh | 26 +++++++++++------ harness/badapple/rdebug_load.py | 33 ++++++++++++++++++--- harness/badapple/tests/test_rdebug_load.py | 34 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/harness/badapple/demo.sh b/harness/badapple/demo.sh index bfe795c..a0aa469 100755 --- a/harness/badapple/demo.sh +++ b/harness/badapple/demo.sh @@ -62,15 +62,23 @@ if [ "$SEND" = 1 ]; then fi # --- readiness: an ICMP reply from the terminal's own stack ----------------- -start=$(date +%s) -while ! ping -c1 -W1 "$HOST" > /dev/null 2>&1; do - if [ $(( $(date +%s) - start )) -ge "$WAIT" ]; then - echo "demo.sh: $HOST did not answer within ${WAIT}s" >&2 - echo " is the terminal booted and on the tap? (harness/mame/run.sh net)" >&2 - exit 1 - fi -done -echo "demo.sh: $HOST answers ($(( $(date +%s) - start ))s)" +# Only when this run is going to load the payload. A loaded payload owns the +# LANCE (and stops the OS's stack with it), so it never answers ICMP: waiting for +# a ping after the load would deadlock -- which is exactly what --no-load used to +# do. --no-load therefore skips the gate and trusts the operator. +if [ "$LOAD" = 1 ]; then + start=$(date +%s) + while ! ping -c1 -W1 "$HOST" > /dev/null 2>&1; do + if [ $(( $(date +%s) - start )) -ge "$WAIT" ]; then + echo "demo.sh: $HOST did not answer within ${WAIT}s" >&2 + echo " is the terminal booted and on the tap? (harness/mame/run.sh net)" >&2 + exit 1 + fi + done + echo "demo.sh: $HOST answers ($(( $(date +%s) - start ))s)" +else + echo "demo.sh: --no-load: not waiting for ICMP (a loaded payload owns the LANCE)" +fi # --- the payload, over the firmware's own remote-debug service -------------- if [ "$LOAD" = 1 ]; then diff --git a/harness/badapple/rdebug_load.py b/harness/badapple/rdebug_load.py index 918b185..9b20dd9 100755 --- a/harness/badapple/rdebug_load.py +++ b/harness/badapple/rdebug_load.py @@ -46,6 +46,11 @@ CHUNK = 512 #: only when the environment file is absent, so a hand-built image still loads. DEFAULT_LOAD = 0x018C0000 +#: Socket timeout. Generous on purpose: a freshly booted OS answers rdebug +#: slowly (measured: a readback peek timed out at 5 s while the machine was still +#: settling), and a slow peek must not fail a load that already succeeded. +DEFAULT_TIMEOUT = 15.0 + def read_env(path) -> dict: """Parse ``vidstream.env`` (``KEY=VALUE`` lines) into ints where possible.""" @@ -85,14 +90,31 @@ def plan(image: bytes, load: int, size: int = CHUNK): for n, part in enumerate(chunks(image, size))] +def readback(rd, transfers, retries=1): + """Peek every chunk back, retrying a stalled read once. + + A timeout here is the machine being busy, not a bad image -- the poke has + already been acknowledged -- so it is retried before it is believed. + """ + back = b"" + for attempt in range(retries + 1): + try: + return b"".join(rd.peek(addr, len(part)) for addr, part in transfers) + except (OSError, TimeoutError): + if attempt == retries: + raise + return back + + def load_and_jump(image_path, entry, load, do_jump=True, chunk=CHUNK, - host="192.168.99.2", port=771, verbose=False): + host="192.168.99.2", port=771, verbose=False, + timeout=DEFAULT_TIMEOUT): """Poke ``image_path`` at ``load``, verify it, then jump to ``entry``.""" import exploit # dwt/exploit.py: the protocol the firmware speaks data = open(image_path, "rb").read() transfers = plan(data, load, chunk) - rd = exploit.RDebug(host, port, 5.0, verbose) + rd = exploit.RDebug(host, port, timeout, verbose) rd.connect() try: for addr, part in transfers: @@ -100,7 +122,7 @@ def load_and_jump(image_path, entry, load, do_jump=True, chunk=CHUNK, print("poked %d bytes in %d chunk(s) at %#x" % (len(data), len(transfers), load)) - back = b"".join(rd.peek(addr, len(part)) for addr, part in transfers) + back = readback(rd, transfers) if back != data: bad = next(i for i in range(min(len(back), len(data))) if back[i] != data[i]) @@ -140,6 +162,8 @@ def main(argv=None) -> int: ap.add_argument("--chunk", type=int, default=CHUNK, help="poke size in bytes (max %d; default %%(default)s)" % CHUNK) + ap.add_argument("--timeout", type=float, default=DEFAULT_TIMEOUT, + help="socket timeout in seconds (default %(default)s)") ap.add_argument("--no-jump", action="store_true", help="load and verify, do not start it") ap.add_argument("--dry-run", action="store_true", @@ -182,7 +206,8 @@ def main(argv=None) -> int: try: return load_and_jump(args.image, entry, load, do_jump=not args.no_jump, chunk=args.chunk, - host=args.host, port=args.port) + host=args.host, port=args.port, + timeout=args.timeout) except Exception as exc: # noqa: BLE001 - one line, not a traceback print("error: %s: %s" % (type(exc).__name__, exc), file=sys.stderr) return 1 diff --git a/harness/badapple/tests/test_rdebug_load.py b/harness/badapple/tests/test_rdebug_load.py index 1d0384c..f69ee4f 100644 --- a/harness/badapple/tests/test_rdebug_load.py +++ b/harness/badapple/tests/test_rdebug_load.py @@ -54,6 +54,40 @@ def test_missing_env_file_says_so_instead_of_guessing(tmp_path, capsys): assert "no build environment" in err and "build-stream.sh" in err +# --- readback ------------------------------------------------------------ + +class _Flaky: + """A remote end whose first peek times out, as a busy OS does.""" + + def __init__(self, transfers, fail_times=1): + self.mem = dict(transfers) + self.fail = fail_times + self.calls = 0 + + def peek(self, addr, length): + self.calls += 1 + if self.fail: + self.fail -= 1 + raise TimeoutError("timed out") + return self.mem[addr][:length] + + +def test_readback_retries_a_stalled_peek_instead_of_failing_the_load(): + transfers = rdebug_load.plan(b"0123456789", 0x018C0000, 4) + rd = _Flaky(transfers, fail_times=1) + assert rdebug_load.readback(rd, transfers) == b"0123456789" + # The retry re-reads the whole image: one timed-out chunk plus the three + # chunks of the successful pass. + assert rd.calls == len(transfers) + 1 + + +def test_readback_gives_up_after_the_retry(): + transfers = rdebug_load.plan(b"0123", 0x018C0000, 4) + rd = _Flaky(transfers, fail_times=9) + with pytest.raises(TimeoutError): + rdebug_load.readback(rd, transfers) + + # --- the clip format ------------------------------------------------------ def test_packed_rows_put_pixel_zero_on_bit_zero_of_byte_zero():