Skip to content

Commit 02341a8

Browse files
tsayin4claude
authored andcommitted
io_legacy: send the status word in the buffer that is transmitted
`apdu_send` picks the response buffer from `tx`: non-zero means the caller filled `apdu_buffer`, zero means it filled `io_buffer` and set `tx_length`. `Comm::reply` ignored that split and always wrote the status word into `io_buffer`, so a caller using the `apdu_buffer`/`tx` pair transmitted its data with no status word at all - the two bytes went into a buffer that was never sent. The host does not see a short response, it sees a wrong one: it reads the last two data bytes as the status word. A reply of `de ad` with `9000` appended arrives as data `()` and status `0xdead`. `io.rs` appended to `apdu_buffer` at `tx` until 063cf2b ("IO revamp adaptation") moved the write to `io_buffer` without moving the switch in `apdu_send` with it. Reported in #431. The write now follows the same test `apdu_send` makes, extracted into `append_status_word` so it can be exercised without a transport. Both paths are covered: a response built in `apdu_buffer` keeps `io_buffer` untouched, and the `io_buffer` path is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01AqD13QkB8DhzkDSdKbJScv
1 parent 1fe624b commit 02341a8

2 files changed

Lines changed: 68 additions & 4 deletions

File tree

‎ledger_device_sdk/CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Fixed
11+
- `io_legacy`: `Comm::reply` appends the status word to the buffer that is actually
12+
transmitted. `apdu_send` sends `apdu_buffer` when `tx` is set and `io_buffer`
13+
otherwise, but the status word was always written to `io_buffer`, so a caller
14+
using the `apdu_buffer`/`tx` pair sent its response data with no status word at
15+
all and the host read the last two data bytes as one.
16+
817
## [1.42.1] - 2026-10-23
918

1019
### Changed

‎ledger_device_sdk/src/io_legacy.rs‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,26 @@ impl Comm {
823823
}
824824
}
825825

826+
/// Appends the status word to whichever response buffer `apdu_send` will
827+
/// transmit.
828+
///
829+
/// The two response paths of this module are selected by `tx`: a caller that
830+
/// filled `apdu_buffer` sets `tx`, and one that filled `io_buffer` sets
831+
/// `tx_length`. `apdu_send` honours that split, so the status word has to be
832+
/// written into the buffer the same test will pick - writing it into
833+
/// `io_buffer` unconditionally left it in a buffer that was never sent.
834+
fn append_status_word(&mut self, sw: u16) {
835+
if self.tx != 0 {
836+
self.apdu_buffer[self.tx] = (sw >> 8) as u8;
837+
self.apdu_buffer[self.tx + 1] = sw as u8;
838+
self.tx += 2;
839+
} else {
840+
self.io_buffer[self.tx_length] = (sw >> 8) as u8;
841+
self.io_buffer[self.tx_length + 1] = sw as u8;
842+
self.tx_length += 2;
843+
}
844+
}
845+
826846
/// Set the Status Word of the response to the previous Command event, and
827847
/// transmit the response.
828848
///
@@ -832,11 +852,8 @@ impl Comm {
832852
/// StatusWords, a SyscallError, or any type which can be converted to a
833853
/// Reply.
834854
pub fn reply<T: Into<Reply>>(&mut self, reply: T) {
835-
let sw = reply.into().0;
836855
// Append status word
837-
self.io_buffer[self.tx_length] = (sw >> 8) as u8;
838-
self.io_buffer[self.tx_length + 1] = sw as u8;
839-
self.tx_length += 2;
856+
self.append_status_word(reply.into().0);
840857
// Transmit the response
841858
self.apdu_send();
842859
}
@@ -1054,4 +1071,42 @@ mod test {
10541071
assert_eq!(m.p1, 0);
10551072
assert_eq!(m.p2, 0);
10561073
}
1074+
1075+
/// A caller that filled `apdu_buffer` and set `tx` must get its status word
1076+
/// in `apdu_buffer`, because that is the buffer `apdu_send` transmits when
1077+
/// `tx` is non-zero. Writing it to `io_buffer` instead dropped the status
1078+
/// word from every such response that carried data.
1079+
#[test]
1080+
fn status_word_follows_apdu_buffer() {
1081+
let mut c = Comm::new();
1082+
c.apdu_buffer[0] = 0xde;
1083+
c.apdu_buffer[1] = 0xad;
1084+
c.tx = 2;
1085+
1086+
c.append_status_word(StatusWords::Ok as u16);
1087+
1088+
assert_eq!(c.tx, 4);
1089+
assert_eq!(c.apdu_buffer[2], 0x90);
1090+
assert_eq!(c.apdu_buffer[3], 0x00);
1091+
// The unused buffer must be left alone.
1092+
assert_eq!(c.tx_length, 0);
1093+
assert_eq!(c.io_buffer[0], 0);
1094+
assert_eq!(c.io_buffer[1], 0);
1095+
}
1096+
1097+
/// The `io_buffer` path is selected when `tx` is zero, and keeps working.
1098+
#[test]
1099+
fn status_word_follows_io_buffer() {
1100+
let mut c = Comm::new();
1101+
c.io_buffer[0] = 0xde;
1102+
c.io_buffer[1] = 0xad;
1103+
c.tx_length = 2;
1104+
1105+
c.append_status_word(StatusWords::Ok as u16);
1106+
1107+
assert_eq!(c.tx_length, 4);
1108+
assert_eq!(c.io_buffer[2], 0x90);
1109+
assert_eq!(c.io_buffer[3], 0x00);
1110+
assert_eq!(c.tx, 0);
1111+
}
10571112
}

0 commit comments

Comments
 (0)