From c5900898662ffb3285cf1ce492d8599fb7baa7df Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 7 Sep 2026 12:10:03 +0000 Subject: [PATCH] Properly handle out of order read fragments --- docs/nfc_read.md | 19 ++++++++++-------- docs/reverse_engineering_procedure.md | 3 ++- openvixdisklib/nfc_open.py | 29 ++++++++++++++++++--------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/docs/nfc_read.md b/docs/nfc_read.md index d61205c..f2158bf 100644 --- a/docs/nfc_read.md +++ b/docs/nfc_read.md @@ -67,27 +67,30 @@ Reply payload (handle is zeroed; lengths describe this fragment): | 8 | `uint64` | `1` (read) | | 16 | `uint64` | Byte offset of the **request** | | 24 | `uint32` | Total request length | -| 28 | `uint32` | Fragment index (`0`, `1`, …) | +| 28 | `uint32` | Byte offset of this fragment (`0`, `65536`, …) | | 32 | `uint32` | This fragment’s byte length | | 36 | `uint32` | Same as offset 32 | | 40 | `uint32` | `0` | When there is a single fragment, offsets 24–31 look like a `uint64` -length (index is 0). The 129-sector capture shows why they are two -`uint32`s: fragment 0 has `(66048, 0)` then chunk 65536; fragment 1 -has `(66048, 1)` then chunk 512. +length (the fragment offset is 0). The 129-sector capture shows why +they are two `uint32`s: fragment 0 has `(66048, 0)` then chunk 65536; +fragment 1 has `(66048, 65536)` then chunk 512. `0x00010000` at offset +28 is the byte offset, not a 0-based index. Read loop: receive fragments with that `opId` until the concatenated data length equals the request. Use the `uint32` at payload offset 32 -as the extra-data size for that fragment. Do not treat extra data as -part of AIO `size` (that field stays 44). +as the extra-data size for that fragment, and copy it to the byte +offset at payload offset 28 — fragments are not always delivered in +order. Do not treat extra data as part of AIO `size` (that field stays +44). 129-sector example (one client request, two server fragments): ``` C: type=7 opId=18 size=44 offset=0 length=66048 -S: type=7 opId=18 size=44 index=0 chunk=65536 + 65536 data -S: type=7 opId=18 size=44 index=1 chunk=512 + 512 data +S: type=7 opId=18 size=44 dest=0 chunk=65536 + 65536 data +S: type=7 opId=18 size=44 dest=65536 chunk=512 + 512 data ``` ## Lab check diff --git a/docs/reverse_engineering_procedure.md b/docs/reverse_engineering_procedure.md index 256b399..9299cb0 100644 --- a/docs/reverse_engineering_procedure.md +++ b/docs/reverse_engineering_procedure.md @@ -227,7 +227,8 @@ What that comparison showed: VDDK repeats the byte length there. - Zeros on the wire are real transferred zeros, not a sparse skip. -Replay: `NfcDisk.read` loops on fragments until `length` bytes arrive. +Replay: `NfcDisk.read` places fragments at the byte offset in the +reply (they may arrive out of order) until `length` bytes are filled. Proof: `tests/integration/test_nfc_read_write.py` writes a known pattern (including a 129-sector read that must assemble two fragments) and checks the bytes that came back. diff --git a/openvixdisklib/nfc_open.py b/openvixdisklib/nfc_open.py index b291913..7b7f0e1 100644 --- a/openvixdisklib/nfc_open.py +++ b/openvixdisklib/nfc_open.py @@ -198,7 +198,9 @@ class NfcDisk: Matches ``VixDiskLib_Read``: one ``NFC_AIO_MSG_IO`` request in byte units. If the length exceeds the AIO buffer (64 KiB) the - server replies with several same-``opId`` fragments. + server replies with several same-``opId`` fragments, which are + placed by the fragment byte offset in the reply (they may arrive + out of order). Args: start_sector: Sector offset from the start of the disk. @@ -220,8 +222,10 @@ class NfcDisk: op_id = self._next_op_id() self._sock.sendall( _pack_aio_hdr(NFC_AIO_MSG_IO, len(payload), op_id) + payload) - data = bytearray() - while len(data) < length: + data = bytearray(length) + filled = 0 + seen: set[int] = set() + while filled < length: rhdr = _recvn(self._sock, NFC_AIO_HDR_SIZE) rtype, rsize, rop = _unpack_aio_hdr(rhdr) if rtype != NFC_AIO_MSG_IO or rop != op_id: @@ -232,13 +236,20 @@ class NfcDisk: if rsize < 36: raise NfcProtocolError( f"AIO IO reply payload too short: {rsize}") - chunk_len = struct.unpack_from(" remaining: + # Fragments may arrive out of order. Offset 28 is the byte + # offset of this chunk within the request (0, 65536, …), + # not a 0-based index. + dest, chunk_len = struct.unpack_from(" length): raise NfcProtocolError( - f"AIO IO chunk length {chunk_len} invalid, " - f"remaining {remaining}") - data.extend(_recvn(self._sock, chunk_len)) + f"AIO IO chunk offset={dest} length={chunk_len} invalid, " + f"request {length}") + seen.add(dest) + data[dest:dest + chunk_len] = _recvn(self._sock, chunk_len) + filled += chunk_len return bytes(data) def write(