smartcard-i2c-fixes

## armsrc: smartcard / I2C correctness + SAM compatibility fixes

A focused pass over `armsrc/i2c.{c,h}` and `armsrc/i2c_direct.c` addressing correctness bugs (NULL deref on alloc failure, unbounded recursion, write-failure silently falling through to read), and removing two long-standing SAM-compatibility blockers (`SmartCardSetBaud` was a stub; float in firmware hot path).

### Fixed

- **`SmartCardRaw`: BigBuf_calloc result was unchecked.** NULL deref on memory pressure crashed the firmware. Now bails with `PM3_EMALLOC` before touching `resp[]`.
- **`SmartCardRaw`: `I2C_BufferWrite` failure was only acted on at `dbglevel > 3`.** Lower debug levels silently fell through to `sc_rx_bytes()` reading from a card we never wrote to. The `reply_ng` / `goto OUT` path now runs on any write failure; the `Dbprintf` is the only thing gated on dbglevel.
- **`SmartCardRaw`: replaced float division** `(p->wait_delay * 1000) / 3.07` with integer math via `uint64_t`. Avoids pulling `__aeabi_fdiv` into the ARM image and removes overflow at large `wait_delay` values: `wait = (ms * 100000 + 153) / 307`.
- **`SmartCardUpgrade`: BigBuf_calloc result was unchecked.** Now bails with `PM3_EMALLOC`. Renamed local `verfiydata` typo to `verifydata`. Failure messages now include the offset where the upgrade aborted (`Writing failed at offset 0x%04X` instead of `Writing failed`).
- **`I2C_BufferRead`: rejected `len == 0` but accepted `len == 1`**, which cannot represent the SIM module's 2-byte BE length header (returned -1 cast through int16_t). Tightened to `len < 2`.
- **`WaitSCL_L_timeout`: `return (delay == 0)` was dead code** — post-decrement underflows `delay` to `UINT32_MAX` after the loop, so the expression was always false. Replaced with explicit `return false` and corrected the comment that claimed an 1800 ms cap (actual cap is 1200 ms).
- **`i2c.h`: SIM_WAIT_DELAY comment was wrong** — `150000 * 3.07us = 460 ms`, not 270 ms. Updated comment to match the actual value; constant unchanged.
- **`SmartCardDirectSend`: BigBuf_calloc result for `resp[]` was unchecked.** The next `resp[0] = prepend` would NULL-deref on alloc failure. Now bails.
- **`SmartCardDirectSend`: GET RESPONSE (61xx) chain recursed without bound**, allocating a fresh `smart_card_raw_t` from BigBuf each round. A misbehaving card returning 61xx every time would wedge the device. Added `depth` parameter capped at `SC_DIRECT_MAX_DEPTH` (8); both call sites updated. The GET RESPONSE inner allocation is also now NULL-checked.
- **`SmartCardDirectSend`: same write-failure-silent fix** as `SmartCardRaw`.
- **`SmartCardDirectSend`: same float → integer-math conversion** as `SmartCardRaw`.

### Added

- **`SmartCardSetBaud` is now functional.** Was an empty stub (wired through `pm3_cmd.h` / `appmain.c` dispatch but did nothing). Now sends `I2C_DEVICE_CMD_SETBAUD` (0x04) to the SIM module's main address.

### Notes for follow-up

- `SmartCardSetBaud` is now functional on the firmware side, but no client command exposes it — `client/src/cmdsmartcard.c` has no `setbaud` subcommand. A separate change can wire `CMD_SMART_SETBAUD` into the client if needed; leaving it out of this patch keeps it firmware-only and safe to backport.
- The proxmark3 SIM module's stock asm firmware (`sim013`) silently drops `CMD_SETBAUD`. gentilkiwi's `sim_c` v4.50+ also currently dispatches it to a no-op. Full PPS follow-through requires a SIM-firmware-side patch in addition to this commit.
- Mid-byte SCL stretch timeouts in `I2C_SendByte` / `I2C_ReadByte` are still swallowed (return void / -1, both ignored at most callers). Fixing this requires changing those signatures and verifying every caller — out of scope here, merits its own PR.
This commit is contained in:
Antiklesys
2026-05-10 14:35:16 +08:00
parent b2689aad92
commit d5d00bce0a
3 changed files with 82 additions and 26 deletions
+43 -16
View File
@@ -187,9 +187,8 @@ static bool WaitSCL_L(void) {
return WaitSCL_L_delay(5000);
}
// Wait max 1800ms or until SCL goes LOW.
// It timeout reading response from card
// Which ever comes first
// Wait up to 1200ms or until SCL goes LOW, whichever comes first.
// Used to bound the timeout while reading a response from the card.
static bool WaitSCL_L_timeout(void) {
volatile uint32_t delay = 1200;
while (delay--) {
@@ -199,7 +198,7 @@ static bool WaitSCL_L_timeout(void) {
WaitMS(1);
}
return (delay == 0);
return false;
}
static bool I2C_Start(void) {
@@ -489,8 +488,9 @@ bool I2C_BufferWrite(const uint8_t *data, uint16_t len, uint8_t device_cmd, uint
// len = uint16 because we need to read up to 256bytes
int16_t I2C_BufferRead(uint8_t *data, uint16_t len, uint8_t device_cmd, uint8_t device_address) {
// sanity check
if (data == NULL || len == 0) {
// sanity check - need at least 2 bytes for the SIM-module length header
// (the response format prepends a 2-byte BE length); fewer cannot be parsed.
if (data == NULL || len < 2) {
return 0;
}
@@ -858,7 +858,11 @@ void SmartCardRaw(const smart_card_raw_t *p) {
uint16_t len = 0;
uint8_t *resp = BigBuf_calloc(ISO7816_MAX_FRAME);
// check if alloacted...
if (resp == NULL) {
reply_ng(CMD_SMART_RAW, PM3_EMALLOC, NULL, 0);
LEDsoff();
return;
}
smartcard_command_t flags = p->flags;
if ((flags & SC_CLEARLOG) == SC_CLEARLOG)
@@ -888,7 +892,10 @@ void SmartCardRaw(const smart_card_raw_t *p) {
uint32_t wait = SIM_WAIT_DELAY;
if ((flags & SC_WAIT) == SC_WAIT) {
wait = (uint32_t)((p->wait_delay * 1000) / 3.07);
// wait_delay is in ms; one WaitSCL_H_delay iteration is ~3.07us.
// Integer-only conversion via uint64_t to avoid soft-float and avoid
// overflow at large wait_delay values: (ms * 100000 + 153) / 307.
wait = (uint32_t)(((uint64_t)p->wait_delay * 100000U + 153U) / 307U);
}
LogTrace(p->data, p->len, 0, 0, NULL, true);
@@ -900,8 +907,10 @@ void SmartCardRaw(const smart_card_raw_t *p) {
I2C_DEVICE_ADDRESS_MAIN
);
if (res == false && g_dbglevel > 3) {
DbpString(I2C_ERROR);
if (res == false) {
if (g_dbglevel > 3) {
DbpString(I2C_ERROR);
}
reply_ng(CMD_SMART_RAW, PM3_ESOFT, NULL, 0);
goto OUT;
}
@@ -937,7 +946,12 @@ void SmartCardUpgrade(uint64_t arg0) {
bool isOK = true;
uint16_t length = arg0, pos = 0;
const uint8_t *fwdata = BigBuf_get_addr();
uint8_t *verfiydata = BigBuf_calloc(I2C_BLOCK_SIZE);
uint8_t *verifydata = BigBuf_calloc(I2C_BLOCK_SIZE);
if (verifydata == NULL) {
reply_ng(CMD_SMART_UPGRADE, PM3_EMALLOC, NULL, 0);
LED_C_OFF();
return;
}
while (length) {
@@ -951,7 +965,7 @@ void SmartCardUpgrade(uint64_t arg0) {
// write
int16_t res = I2C_WriteFW(fwdata + pos, size, msb, lsb, I2C_DEVICE_ADDRESS_BOOT);
if (!res) {
DbpString("Writing failed");
Dbprintf("Writing failed at offset 0x%04X", pos);
isOK = false;
break;
}
@@ -960,16 +974,16 @@ void SmartCardUpgrade(uint64_t arg0) {
WaitMS(50);
// read
res = I2C_ReadFW(verfiydata, size, msb, lsb, I2C_DEVICE_ADDRESS_BOOT);
res = I2C_ReadFW(verifydata, size, msb, lsb, I2C_DEVICE_ADDRESS_BOOT);
if (res <= 0) {
DbpString("Reading back failed");
Dbprintf("Reading back failed at offset 0x%04X", pos);
isOK = false;
break;
}
// cmp
if (0 != memcmp(fwdata + pos, verfiydata, size)) {
DbpString("not equal data");
if (0 != memcmp(fwdata + pos, verifydata, size)) {
Dbprintf("Verify mismatch at offset 0x%04X", pos);
isOK = false;
break;
}
@@ -983,7 +997,20 @@ void SmartCardUpgrade(uint64_t arg0) {
BigBuf_free();
}
// Send a single byte to the SIM module's CMD_SETBAUD opcode (0x04).
// The 8051 firmware uses this to reload Timer1 (UART0 baud generator).
// Until 2026 the implementation was an empty stub; the SIM module silently
// ignored any host-driven baud renegotiation. Some smart cards (notably the
// HID Artemis SLE88 SAM family) advertise non-default Fi/Di in TA1 and need
// PPS to switch the bridge baud post-ATR.
void SmartCardSetBaud(uint64_t arg0) {
LED_D_ON();
I2C_Reset_EnterMainProgram();
bool ok = I2C_WriteByte((uint8_t)(arg0 & 0xFF),
I2C_DEVICE_CMD_SETBAUD,
I2C_DEVICE_ADDRESS_MAIN);
reply_ng(CMD_SMART_SETBAUD, ok ? PM3_SUCCESS : PM3_ESOFT, NULL, 0);
LEDsoff();
}
void SmartCardSetClock(uint64_t arg0) {
+5 -3
View File
@@ -34,9 +34,11 @@
#define ISO7816_MAX_FRAME 270
// 8051 speaks with smart card.
// 1000*50*3.07 = 153.5ms
// 1 byte transfer == 1ms with max frame being 256 bytes
#define SIM_WAIT_DELAY 150000 // about 270ms delay // 109773 -- about 337.7ms delay
// 1 byte transfer == 1ms with max frame being 256 bytes.
// SIM_WAIT_DELAY is the iteration count passed to WaitSCL_H_delay(); each iter
// is ~3.07us, so 150000 * 3.07us = ~460ms - the upper bound we wait for the
// SIM module to assert SCL after a SIM-side operation.
#define SIM_WAIT_DELAY 150000 // ~460ms total wait via WaitSCL_H_delay
void I2C_recovery(void);
+34 -7
View File
@@ -36,13 +36,25 @@
#include "i2c.h"
#include "i2c_direct.h"
static void SmartCardDirectSend(uint8_t prepend, const smart_card_raw_t *p, uint8_t *output, uint16_t *olen) {
// Maximum chained ISO 7816-4 GET RESPONSE (61xx) follow-ups before bailing.
// A misbehaving card that always returns 61xx would otherwise recurse without
// bound, allocating fresh smart_card_raw_t payloads from BigBuf each time
// until the device wedges. Eight rounds is more than any legitimate APDU
// chain would need.
#define SC_DIRECT_MAX_DEPTH 8
static void SmartCardDirectSend(uint8_t prepend, const smart_card_raw_t *p, uint8_t *output, uint16_t *olen, uint8_t depth) {
LED_D_ON();
uint16_t len = 0;
uint8_t *resp = BigBuf_calloc(ISO7816_MAX_FRAME);
if (resp == NULL) {
Dbprintf("SmartCardDirectSend: BigBuf_calloc failed");
if (olen) *olen = 0;
LEDsoff();
return;
}
resp[0] = prepend;
// check if alloacted...
smartcard_command_t flags = p->flags;
if ((flags & SC_LOG) == SC_LOG)
@@ -70,7 +82,10 @@ static void SmartCardDirectSend(uint8_t prepend, const smart_card_raw_t *p, uint
if (((flags & SC_RAW) == SC_RAW) || ((flags & SC_RAW_T0) == SC_RAW_T0)) {
if ((flags & SC_WAIT) == SC_WAIT) {
wait = (uint32_t)((p->wait_delay * 1000) / 3.07);
// wait_delay is in ms; one WaitSCL_H_delay iteration is ~3.07us.
// Integer-only conversion via uint64_t to avoid soft-float and
// avoid overflow at large wait_delay values.
wait = (uint32_t)(((uint64_t)p->wait_delay * 100000U + 153U) / 307U);
}
LogTrace(p->data, p->len, 0, 0, NULL, true);
@@ -82,8 +97,10 @@ static void SmartCardDirectSend(uint8_t prepend, const smart_card_raw_t *p, uint
I2C_DEVICE_ADDRESS_MAIN
);
if (res == false && g_dbglevel > 3) {
Dbprintf("SmartCardDirectSend: I2C_BufferWrite failed\n");
if (res == false) {
if (g_dbglevel > 3) {
Dbprintf("SmartCardDirectSend: I2C_BufferWrite failed");
}
goto OUT;
}
@@ -98,15 +115,25 @@ static void SmartCardDirectSend(uint8_t prepend, const smart_card_raw_t *p, uint
}
if (len == 2 && resp[1] == 0x61) {
if (depth >= SC_DIRECT_MAX_DEPTH) {
Dbprintf("SmartCardDirectSend: GET RESPONSE chain depth (%u) exceeded; aborting", depth);
goto OUT;
}
uint8_t cmd_getresp[] = {0x00, ISO7816_GET_RESPONSE, 0x00, 0x00, resp[2]};
smart_card_raw_t *payload = (smart_card_raw_t *)BigBuf_calloc(sizeof(smart_card_raw_t) + sizeof(cmd_getresp));
if (payload == NULL) {
Dbprintf("SmartCardDirectSend: GET RESPONSE alloc failed");
goto OUT;
}
payload->flags = SC_RAW | SC_LOG;
payload->len = sizeof(cmd_getresp);
payload->wait_delay = 0;
memcpy(payload->data, cmd_getresp, sizeof(cmd_getresp));
SmartCardDirectSend(prepend, payload, output, olen);
SmartCardDirectSend(prepend, payload, output, olen, depth + 1);
} else if (len == 2) {
Dbprintf("***** BAD response from card (response unsupported)...");
Dbhexdump(3, &resp[0], false);
@@ -199,7 +226,7 @@ int CmdSmartRaw(const uint8_t prepend, const uint8_t *data, int dlen, uint8_t *o
payload->flags |= SC_RAW;
}
SmartCardDirectSend(prepend, payload, output, olen);
SmartCardDirectSend(prepend, payload, output, olen, 0);
return PM3_SUCCESS;
}