August 2026 - gerrit-log - lists.osmocom.org

[XS] Change in simtrace2[master]: firmware: sniffer: fix ~INS procedure byte comparison
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43129?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: firmware: sniffer: fix ~INS procedure byte comparison ...................................................................... firmware: sniffer: fix ~INS procedure byte comparison (~g_tpdu.packet[1]) == byte can never be true. Unary ~ applies the integer promotions first, so for tpdu INS = 0xA4 lhs should be 0x5B but as int it gets zero extended to at least 16 bits and then flipped, so it is 0xFFFFFF5B = -165, byte promotes to 0..255. The ack was therefore dead code -> fallthrough to SW1 branch, fails 0x6x/0x9x test, TPDU gets flagged SNIFF_DATA_FLAG_ERROR_MALFORMED from what I can tell. But I am losing track of all these arcane issues to be honest. Narrow the complement back to 8 bits. Fyi this is unrelated to signedness and not specific to ARM. Change-Id: I800f50ef35356429d07aa685ea919e70ec34946e --- M firmware/libcommon/source/sniffer.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 948eee8..419e8fb 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -775,7 +775,7 @@ } else if (g_tpdu.packet[1] == byte) { /* get all remaining data bytes */ change_tpdu_state(TPDU_S_DATA_REMAINING); break; - } else if ((~g_tpdu.packet[1]) == byte) { /* get single data byte */ + } else if ((uint8_t)(~g_tpdu.packet[1]) == byte) { /* get single data byte */ change_tpdu_state(TPDU_S_DATA_SINGLE); break; } -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43129?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I800f50ef35356429d07aa685ea919e70ec34946e Gerrit-Change-Number: 43129 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: firmware: sniffer: do not drop maximum-length ATRs
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43130?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: firmware: sniffer: do not drop maximum-length ATRs ...................................................................... firmware: sniffer: do not drop maximum-length ATRs 7816-3 8.1/8.2.1 allow TS plus 32 bytes. atr_i is a byte count, not an index, process_byte_atr() guards its own store with the same condition before incrementing, so atr_i reaches 33. Change-Id: Ic8398cbefc0b522946b6470fd0268fa70662dab1 --- M firmware/libcommon/source/sniffer.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified lynxis lazus: Looks good to me, approved diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 419e8fb..98cc6cd 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -406,7 +406,7 @@ TRACE_WARNING("Can't print ATR in ISO 7816-3 state %u\n\r", iso_state); return; } - if (g_atr.atr_i >= ARRAY_SIZE(g_atr.atr)) { + if (g_atr.atr_i > ARRAY_SIZE(g_atr.atr)) { TRACE_ERROR("ATR buffer overflow\n\r"); return; } -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43130?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Ic8398cbefc0b522946b6470fd0268fa70662dab1 Gerrit-Change-Number: 43130 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: firmware: sniffer: honor the ep argument of usb_msg_alloc_hdr()
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43131?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: firmware: sniffer: honor the ep argument of usb_msg_alloc_hdr() ...................................................................... firmware: sniffer: honor the ep argument of usb_msg_alloc_hdr() Even though all callers pass the same endpoint anyway the arg should be used and not discarded. Change-Id: I1fa0097b9eef531900b359c7293a6c60040254e4 --- M firmware/libcommon/source/sniffer.c 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved Jenkins Builder: Verified diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 98cc6cd..93a66b2 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -239,14 +239,14 @@ static struct msgb *usb_msg_alloc_hdr(uint8_t ep, uint8_t msg_class, uint8_t msg_type) { /* Only allocate message if not too many are already in the queue */ - struct llist_head *head = usb_get_queue(SIMTRACE_USB_EP_CARD_DATAIN); + struct llist_head *head = usb_get_queue(ep); if (!head) { return NULL; } if (llist_count(head) > 5) { return NULL; } - struct msgb *usb_msg = usb_buf_alloc(SIMTRACE_USB_EP_CARD_DATAIN); + struct msgb *usb_msg = usb_buf_alloc(ep); if (!usb_msg) { return NULL; } -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43131?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I1fa0097b9eef531900b359c7293a6c60040254e4 Gerrit-Change-Number: 43131 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: firmware: qmod: fix '@' debug command switching the wrong channel
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43132?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: firmware: qmod: fix '@' debug command switching the wrong channel ...................................................................... firmware: qmod: fix '@' debug command switching the wrong channel Apparently copy paste mistake. Change-Id: I9383adcfffab07ffa5f1fd3505e3aa244fb393ff --- M firmware/libboard/qmod/source/board_qmod.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved diff --git a/firmware/libboard/qmod/source/board_qmod.c b/firmware/libboard/qmod/source/board_qmod.c index 189fb58..078570d 100644 --- a/firmware/libboard/qmod/source/board_qmod.c +++ b/firmware/libboard/qmod/source/board_qmod.c @@ -287,7 +287,7 @@ sim_switch_use_physical(0, 0); break; case '@': - sim_switch_use_physical(0, 0); + sim_switch_use_physical(1, 0); break; case 't': talloc_report(NULL, stdout); -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43132?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I9383adcfffab07ffa5f1fd3505e3aa244fb393ff Gerrit-Change-Number: 43132 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: host: simtrace2_api: do not log random memory
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43133?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: host: simtrace2_api: do not log random memory ...................................................................... host: simtrace2_api: do not log random memory tx_cfg->features is logged before the memcpy() that fills it. Looks like no one is using config.ac --enable-sanitize? Change-Id: Id8369d312c8600ba9eea80c8f7782f196d7e20d1 --- M host/lib/simtrace2_api.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: lynxis lazus: Looks good to me, approved Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve diff --git a/host/lib/simtrace2_api.c b/host/lib/simtrace2_api.c index 37be08a..e2bad31 100644 --- a/host/lib/simtrace2_api.c +++ b/host/lib/simtrace2_api.c @@ -280,7 +280,7 @@ tx_cfg = (struct cardemu_usb_msg_config *) msgb_put(msg, sizeof(*tx_cfg)); - LOGSLOT(ci->slot, LOGL_NOTICE, "<= %s(features=%08x)\n", __func__, tx_cfg->features); + LOGSLOT(ci->slot, LOGL_NOTICE, "<= %s(features=%08x)\n", __func__, user_cfg->features); memcpy(tx_cfg, user_cfg, sizeof(*tx_cfg)); osmo_store32le(user_cfg->features, &tx_cfg->features); -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43133?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Id8369d312c8600ba9eea80c8f7782f196d7e20d1 Gerrit-Change-Number: 43133 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: host: simtrace2-sniff: init exit code
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43134?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: host: simtrace2-sniff: init exit code ...................................................................... host: simtrace2-sniff: init exit code Early failure i.e. of libusb return random values as exit code. Change-Id: If920824d35c1f37efe16440b5fb190ca0fb19148 --- M host/src/simtrace2-sniff.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved Jenkins Builder: Verified diff --git a/host/src/simtrace2-sniff.c b/host/src/simtrace2-sniff.c index 8504e8b..1f2c69d 100644 --- a/host/src/simtrace2-sniff.c +++ b/host/src/simtrace2-sniff.c @@ -328,7 +328,7 @@ int main(int argc, char **argv) { - int i, rc, ret; + int i, rc, ret = 1; print_welcome(); /* Parse arguments */ -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43134?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: If920824d35c1f37efe16440b5fb190ca0fb19148 Gerrit-Change-Number: 43134 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: contrib/flash.py: fix typos
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43135?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: contrib/flash.py: fix typos ...................................................................... contrib/flash.py: fix typos Python has no versoin nor printf. Change-Id: I4fd4d2592b44b3ae827e00ae73d8c451aa2ec18f --- M contrib/flash.py 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved diff --git a/contrib/flash.py b/contrib/flash.py index 01fe7c9..393bdf6 100755 --- a/contrib/flash.py +++ b/contrib/flash.py @@ -110,12 +110,12 @@ if serial: version = "< 0.5.1.45-ac7e" else: - versoin = "< 0.5.1.45-ac7e" + version = "< 0.5.1.45-ac7e" else: if serial: version = "< 0.5.1.37-ede8" else: - versoin = "< 0.5.1.34-e026" + version = "< 0.5.1.34-e026" print("device firmware version: " + version) # flash latest firmware if to_flash == "list": # we just want to list the devices, not flash them @@ -156,7 +156,7 @@ dfu_result = subprocess.run(["dfu-util", "--device", hex(definition.usb_vendor_id) + ":" + hex(definition.usb_product_id), "--path", usb_path, "--cfg", "1", "--alt", "1", "--reset", "--download", dl_path]) os.remove(dl_path) if 0 != dfu_result.returncode: - printf("flashing firmware using dfu-util failed. ensure dfu-util is installed and you have the permissions to access this USB device") + print("flashing firmware using dfu-util failed. ensure dfu-util is installed and you have the permissions to access this USB device") continue updated_nb += 1 -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43135?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I4fd4d2592b44b3ae827e00ae73d8c451aa2ec18f Gerrit-Change-Number: 43135 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[S] Change in simtrace2[master]: firmware: protect uart_tx_queue against ISR
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43127?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: firmware: protect uart_tx_queue against ISR ...................................................................... firmware: protect uart_tx_queue against ISR cardem: - dispatch_usb_command_cardem() appends to uart_tx_queue from the main loop - tx_byte_tpdu() dequeues from the USART IRQ handler @ NVIC prio 0 card_handle_reset() has the same issue, drains queue and frees uart_tx_msg from main loop while the ISR may own them. All of this needs protection against the irq. Needs a fixed llist_add_tail_irqsafe(), which called __enable_irq() instead of restoring the saved PRIMASK for some unknown reason?!?!?!? Change-Id: I7d9cdcc56263b27dfd4649dfb1da1d67761ee923 --- M firmware/libcommon/include/llist_irqsafe.h M firmware/libcommon/source/card_emu.c M firmware/libcommon/source/mode_cardemu.c 3 files changed, 21 insertions(+), 8 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve lynxis lazus: Looks good to me, approved diff --git a/firmware/libcommon/include/llist_irqsafe.h b/firmware/libcommon/include/llist_irqsafe.h index 8aafcfb..e6c893a 100644 --- a/firmware/libcommon/include/llist_irqsafe.h +++ b/firmware/libcommon/include/llist_irqsafe.h @@ -34,7 +34,7 @@ local_irq_save(x); llist_add_tail(_new, head); - __enable_irq(); + local_irq_restore(x); } static inline struct llist_head *llist_head_dequeue_irqsafe(struct llist_head *head) diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c index 9c90850..a1d4483 100644 --- a/firmware/libcommon/source/card_emu.c +++ b/firmware/libcommon/source/card_emu.c @@ -219,19 +219,31 @@ static void card_handle_reset(struct card_handle *ch) { struct msgb *msg; + unsigned long x; card_emu_uart_update_wt(ch->uart_chan, 0); - /* release any buffers we may still own */ - if (ch->uart_tx_msg) { - usb_buf_free(ch->uart_tx_msg); - ch->uart_tx_msg = NULL; - } + /* Release any buffers we may still own. + * uart_tx_msg + uart_tx_queue are shared with the UART IRQ handler, + * that preempts us here -> needs atomic detach and free */ + local_irq_save(x); + msg = ch->uart_tx_msg; + ch->uart_tx_msg = NULL; + local_irq_restore(x); + if (msg) + usb_buf_free(msg); + if (ch->uart_rx_msg) { usb_buf_free(ch->uart_rx_msg); ch->uart_rx_msg = NULL; } - while ((msg = msgb_dequeue(&ch->uart_tx_queue))) { + + while (1) { + local_irq_save(x); + msg = msgb_dequeue(&ch->uart_tx_queue); + local_irq_restore(x); + if (!msg) + break; usb_buf_free(msg); } } diff --git a/firmware/libcommon/source/mode_cardemu.c b/firmware/libcommon/source/mode_cardemu.c index 3a0808d..6c1ac8d 100644 --- a/firmware/libcommon/source/mode_cardemu.c +++ b/firmware/libcommon/source/mode_cardemu.c @@ -737,7 +737,8 @@ switch (hdr->msg_type) { case SIMTRACE_MSGT_DT_CEMU_TX_DATA: queue = card_emu_get_uart_tx_queue(ci->ch); - llist_add_tail(&msg->list, queue); + /* drained from the USART IRQ handler at highest NVIC prio */ + llist_add_tail_irqsafe(&msg->list, queue); card_emu_have_new_uart_tx(ci->ch); break; case SIMTRACE_MSGT_DT_CEMU_SET_ATR: -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43127?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I7d9cdcc56263b27dfd4649dfb1da1d67761ee923 Gerrit-Change-Number: 43127 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[XS] Change in simtrace2[master]: firmware: use the full 11 bit US_FIDI.FI_DI_RATIO
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43125?usp=email ) Change subject: firmware: use the full 11 bit US_FIDI.FI_DI_RATIO ...................................................................... firmware: use the full 11 bit US_FIDI.FI_DI_RATIO US_FIDI_FI_DI_RATIO_Msk is 0x7ff, cemu rejected >= 0x400 in emu_update_fidi() and masked with 0x3ff in card_emu_uart_update_fidi(), but update_fidi() used by the sniffer already used 0x7ff. -> ratios 1024..2047 are unusable in cemu, which is the entire upper half of ISO 7816-3 Table 7 at Di=1. A reader trying one of those in a PPS gets the proposal echoed and accepted, after which the card keeps transmitting at the old rate. FI_DI_RATIO is clock periods per bit -> larger ratio is a SLOWER link. The old check rejected slow values but accepted Fi=372/Di=64, ratio 5 !? Unify and use the register mask (= shifed by 0 so usable as value) and reject ratios that do not fit rather than truncating to garbage dividers. Change-Id: I6211dd5be7c5c5d2150af2aa37a403b33e6d340d --- M firmware/libcommon/source/card_emu.c M firmware/libcommon/source/mode_cardemu.c M firmware/libcommon/source/simtrace_iso7816.c 3 files changed, 4 insertions(+), 4 deletions(-) Approvals: Jenkins Builder: Verified lynxis lazus: Looks good to me, approved diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c index ab54fc5..bee0383 100644 --- a/firmware/libcommon/source/card_emu.c +++ b/firmware/libcommon/source/card_emu.c @@ -376,7 +376,7 @@ int rc; rc = iso7816_3_compute_fd_ratio(ch->F_index, ch->D_index); - if (rc > 0 && rc < 0x400) { + if (rc > 0 && rc <= (US_FIDI_FI_DI_RATIO_Msk >> US_FIDI_FI_DI_RATIO_Pos)) { TRACE_INFO("%u: computed F(%u)/D(%u) ratio: %d\r\n", ch->num, ch->F_index, ch->D_index, rc); /* make sure UART uses new F/D ratio */ diff --git a/firmware/libcommon/source/mode_cardemu.c b/firmware/libcommon/source/mode_cardemu.c index a5485e4..3a0808d 100644 --- a/firmware/libcommon/source/mode_cardemu.c +++ b/firmware/libcommon/source/mode_cardemu.c @@ -387,7 +387,7 @@ Usart *usart = get_usart_by_chan(uart_chan); usart->US_CR |= US_CR_RXDIS | US_CR_RSTRX; - usart->US_FIDI = fidi & 0x3ff; + usart->US_FIDI = US_FIDI_FI_DI_RATIO(fidi); usart->US_CR |= US_CR_RXEN | US_CR_STTTO; return 0; } diff --git a/firmware/libcommon/source/simtrace_iso7816.c b/firmware/libcommon/source/simtrace_iso7816.c index 27677a6..f231faa 100644 --- a/firmware/libcommon/source/simtrace_iso7816.c +++ b/firmware/libcommon/source/simtrace_iso7816.c @@ -127,14 +127,14 @@ uint8_t di = fidi & 0xf; int ratio = iso7816_3_compute_fd_ratio(fi, di); - if (ratio > 0 && ratio < 0x8000) { + if (ratio > 0 && ratio <= (US_FIDI_FI_DI_RATIO_Msk >> US_FIDI_FI_DI_RATIO_Pos)) { /* make sure USART uses new F/D ratio */ usart->base->US_CR |= US_CR_RXDIS | US_CR_RSTRX; /* disable write protection */ if (usart->base->US_WPMR) { usart->base->US_WPMR = US_WPMR_WPKEY(0x555341); } - usart->base->US_FIDI = (ratio & 0x7ff); + usart->base->US_FIDI = US_FIDI_FI_DI_RATIO(ratio); usart->base->US_CR |= US_CR_RXEN | US_CR_STTTO; //TRACE_INFO("updated USART(%u) Fi(%u)/Di(%u) ratio(%d): %u\n\r", usart->id, fi, di, ratio, usart->base->US_FIDI); } else { -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43125?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I6211dd5be7c5c5d2150af2aa37a403b33e6d340d Gerrit-Change-Number: 43125 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

[M] Change in simtrace2[master]: firmware: card_emu: use Di in the waiting time
by lynxis lazus 11 Aug '26

11 Aug '26

lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/43126?usp=email ) Change subject: firmware: card_emu: use Di in the waiting time ...................................................................... firmware: card_emu: use Di in the waiting time ISO 7816-3 section 10.2 defines WT = WI x 960 x Fi/f seconds, store as etu, etu = Fi / (D x f) seconds, so the Fi cancels, but the D does not: WT [etu] = WI x 960 x D cemu dropped both (?!) -> WI x 960. The old comment explains why Fi can be dropped, which is right, but what about Di ?! sniffer gets it right (wt_wi * 960UL * wt_d), so the two state machines disagreed here again, by up to a factor of 64???!!?!!? This was fixed in osmo-ccid-firmware in 066489d in 2020 but not ported to st2. Additionally the waiting time was only recalculated at the end of the ATR, where D is still 1 by definition, so a PPS increasing D reprogrammed the baud rate but left the waiting time untouched??! etu duration shrinks with D by the same factor, wall clock WT is independent of D, which is the whole point. The old code decreased the waiting time by a factor of D: after a PPS to D=8 the card emitted its NULL procedure byte at ~0.09s instead of ~0.71s with a reader deadline of ~1.43s, and the inactivity timeout fires 8x too early, which probably led to unexplained wtime_exp errors. Update wt when WI becomes known (end of ATR) and where D changes (after the PPS response) + tests. Change-Id: I4263176d6073029d01f9ff5b11a6311617956af6 --- M firmware/libcommon/source/card_emu.c M firmware/test/card_emu_tests.c 2 files changed, 97 insertions(+), 11 deletions(-) Approvals: lynxis lazus: Looks good to me, approved Jenkins Builder: Verified diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c index bee0383..9c90850 100644 --- a/firmware/libcommon/source/card_emu.c +++ b/firmware/libcommon/source/card_emu.c @@ -386,6 +386,26 @@ ch->num, rc); } +/*! Calculate the WT from current WI and D. + * + * ISO 7816-3 10.2 defines WT = WI x 960 x Fi / f [seconds]. + * Our waiting time is stored in units of etu = Fi / (D x f) seconds + * -> the Fi cancels out, but D does not. + * + * WT [etu] = WI x 960 x D + * + * D is the value from ISO 7816-3 Table 8. Only 1..9 are defined, + * 0 and RFU range 10..15 have no D -> use D = 1 */ +static void emu_update_wt(struct card_handle *ch) +{ + uint8_t d = 1; + + if (ch->D_index >= 1 && ch->D_index <= 9) + d = iso7816_3_di_table[ch->D_index]; + + ch->waiting_time = ch->wi * 960 * d; +} + /* Update the ISO 7816-3 TPDU receiver state */ static void card_set_state(struct card_handle *ch, enum iso7816_3_card_state new_state) @@ -505,11 +525,8 @@ } } } - /* update waiting time (see ISO 7816-3 10.2). We can drop the Fi - * multiplier as we store the waiting time in units of 'etu', and - * don't really care what the number of clock cycles or the absolute - * wall clock time is */ - ch->waiting_time = ch->wi * 960; + /* update the waiting time now that WI is known (see emu_update_wt) */ + emu_update_wt(ch); /* go to next state */ card_set_state(ch, ISO_S_WAIT_TPDU); return 0; @@ -675,6 +692,9 @@ card_emu_uart_wait_tx_idle(ch->uart_chan); /* update baud rate generator with F/D */ emu_update_fidi(ch); + /* the waiting time is expressed in etu and scales with D, so it has + * to be recomputed whenever D changes */ + emu_update_wt(ch); /* Wait for the next TPDU */ card_set_state(ch, ISO_S_WAIT_TPDU); set_pts_state(ch, PTS_S_WAIT_REQ_PTSS); diff --git a/firmware/test/card_emu_tests.c b/firmware/test/card_emu_tests.c index 1fa4a88..82939c7 100644 --- a/firmware/test/card_emu_tests.c +++ b/firmware/test/card_emu_tests.c @@ -396,12 +396,73 @@ 0xFF ^ 0b00010000// PCK }; -static void -test_ppss(struct card_handle *ch) +/* Fi/Di that is actually valid: Fi idx 9 (Fi=512) and Di idx 4 (Di=8) + * This tests calculating F/D ratio (512/8 = 64) and the calculation of the + * waiting time which scales with Di. */ +const uint8_t pps_fidi[] = { + 0xFF, // PPSS + 0b00010000, // PPS0: PPS1 present + 0x94, // PPS1: Fi index 9, Di index 4 + 0xFF ^ 0b00010000 ^ 0x94// PCK +}; + +/* Di 8 idx Di=12 (ISO 7816-3:2006 Table 8),: + * the ratio be 372/12 = 31, not 372*12. */ +const uint8_t pps_di12[] = { + 0xFF, // PPSS + 0b00010000, // PPS0: PPS1 present + 0x18, // PPS1: Fi index 1, Di index 8 + 0xFF ^ 0b00010000 ^ 0x18// PCK +}; + +/* Fi idx 5 Fi=1488 andDi=1 -> ratio 1488, needs all 11 bits of + * the US_FIDI.FI_DI_RATIO field. */ +const uint8_t pps_hi_ratio[] = { + 0xFF, // PPSS + 0b00010000, // PPS0: PPS1 present + 0x51, // PPS1: Fi index 5, Di index 1 + 0xFF ^ 0b00010000 ^ 0x51// PCK +}; + +/* Get a cemu status report to check the negotiated parameters + * This is the only way to get the waiting time from struct card_handle. */ +static void verify_status(struct card_handle *ch, uint8_t exp_f_index, uint8_t exp_d_index, + uint32_t exp_waiting_time) { - reader_send_bytes(ch, pps, sizeof(pps)); - get_and_verify_rctx_pps(pps, sizeof(pps)); - card_tx_verify_chars(ch, pps, sizeof(pps)); + struct usb_buffered_ep *bep = usb_get_buf_ep(PHONE_DATAIN); + struct cardemu_usb_msg_status *sts; + struct simtrace_msg_hdr *mh; + struct msgb *msg; + + card_emu_report_status(ch, false); + + assert(bep); + msg = msgb_dequeue_count(&bep->queue, &bep->queue_len); + assert(msg); + mh = (struct simtrace_msg_hdr *) msg->l1h; + assert(mh->msg_type == SIMTRACE_MSGT_BD_CEMU_STATUS); + sts = (struct cardemu_usb_msg_status *) msg->l2h; + + printf("status: F_index=%u D_index=%u wi=%u waiting_time=%u\n", + sts->F_index, sts->D_index, sts->wi, sts->waiting_time); + + assert(sts->F_index == exp_f_index); + assert(sts->D_index == exp_d_index); + /* WT = WI x 960 x D in etu, see ISO 7816-3 Section 10.2 */ + assert(sts->waiting_time == exp_waiting_time); + + usb_buf_free(msg); +} + +static void +test_ppss(struct card_handle *ch, const uint8_t *req, unsigned int req_len, + uint8_t exp_f_index, uint8_t exp_d_index, uint32_t exp_waiting_time) +{ + printf("\n==> PPS exchange\n"); + reader_send_bytes(ch, req, req_len); + get_and_verify_rctx_pps(req, req_len); + card_tx_verify_chars(ch, req, req_len); + verify_status(ch, exp_f_index, exp_d_index, exp_waiting_time); } /* READ RECORD (offset 0, 10 bytes) */ @@ -426,7 +487,12 @@ io_start_card(ch); card_tx_verify_chars(ch, NULL, 0); - test_ppss(ch); + /* WI is 10 so WT = 10 x 960 x D */ + /* Fi/Di index 0/0 is invalid: the F/D ratio is rejected, D falls back to 1 */ + test_ppss(ch, pps, sizeof(pps), 0, 0, 10 * 960 * 1); + test_ppss(ch, pps_fidi, sizeof(pps_fidi), 9, 4, 10 * 960 * 8); + test_ppss(ch, pps_di12, sizeof(pps_di12), 1, 8, 10 * 960 * 12); + test_ppss(ch, pps_hi_ratio, sizeof(pps_hi_ratio), 5, 1, 10 * 960 * 1); for (i = 0; i < 2; i++) { test_tpdu_reader2card(ch, tpdu_hdr_write_rec, tpdu_body_write_rec, sizeof(tpdu_body_write_rec)); -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/43126?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: I4263176d6073029d01f9ff5b11a6311617956af6 Gerrit-Change-Number: 43126 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>

1 0

2026

2025

2024

2023

2022

gerrit-log August 2026