<p>laforge <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/15746">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">iso7816_fsm: Handle WTIME expiry during ATR reception<br><br>There are some cards that state a wrong length of the historical<br>bytes in their ATR header, resulting in WTIME expiry. Let's dispatch<br>ISO7816_E_WTIME_EXP into the ATR FSM and treat it as normal ATR_DONE if<br>it happens during rx of historical bytes or TCK.<br><br>Also introdcue an ISO7816_E_ATR_ERR_IND for those situations where<br>waiting time expiration occurs during reception of TS/T0/TA/TB/TC/TD<br>bytes.<br><br>Change-Id: I62d47cb5e06b480941c67122f3c7d7a462ea2099<br>---<br>M ccid_common/iso7816_fsm.c<br>M ccid_common/iso7816_fsm.h<br>2 files changed, 58 insertions(+), 15 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/ccid_common/iso7816_fsm.c b/ccid_common/iso7816_fsm.c</span><br><span>index 8705ad9..af7892c 100644</span><br><span>--- a/ccid_common/iso7816_fsm.c</span><br><span>+++ b/ccid_common/iso7816_fsm.c</span><br><span>@@ -141,6 +141,7 @@</span><br><span> { ISO7816_E_RX_ERR_IND, "RX_ERR_IND" },</span><br><span> { ISO7816_E_TX_ERR_IND, "TX_ERR_IND" },</span><br><span> { ISO7816_E_ATR_DONE_IND, "ATR_DONE_IND" },</span><br><span style="color: hsl(120, 100%, 40%);">+ { ISO7816_E_ATR_ERR_IND, "ATR_ERR_IND" },</span><br><span> { ISO7816_E_TPDU_DONE_IND, "TPDU_DONE_IND" },</span><br><span> { ISO7816_E_XCEIVE_TPDU_CMD, "XCEIVE_TPDU_CMD" },</span><br><span> /* allstate events */</span><br><span>@@ -251,6 +252,10 @@</span><br><span> osmo_fsm_inst_state_chg(fi, ISO7816_S_IN_ATR, 0, 0);</span><br><span> osmo_fsm_inst_dispatch(ip->atr_fi, event, data);</span><br><span> break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_WTIME_EXP:</span><br><span style="color: hsl(120, 100%, 40%);">+ ip->user_cb(fi, event, 0, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_state_chg(fi, ISO7816_S_RESET, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> default:</span><br><span> OSMO_ASSERT(0);</span><br><span> }</span><br><span>@@ -265,6 +270,7 @@</span><br><span> switch (event) {</span><br><span> case ISO7816_E_RX_SINGLE:</span><br><span> case ISO7816_E_RX_ERR_IND:</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_WTIME_EXP:</span><br><span> /* simply pass this through to the child FSM for the ATR */</span><br><span> osmo_fsm_inst_dispatch(ip->atr_fi, event, data);</span><br><span> break;</span><br><span>@@ -275,6 +281,10 @@</span><br><span> /* notify user about ATR */</span><br><span> ip->user_cb(fi, event, 0, atr);</span><br><span> break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_ATR_ERR_IND:</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_state_chg(fi, ISO7816_S_RESET, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ ip->user_cb(fi, event, 0, atr);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> default:</span><br><span> OSMO_ASSERT(0);</span><br><span> }</span><br><span>@@ -326,6 +336,10 @@</span><br><span> /* hand finished TPDU to user */</span><br><span> ip->user_cb(fi, event, 0, apdu);</span><br><span> break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_WTIME_EXP:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FIXME: power off? */</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_state_chg(fi, ISO7816_S_RESET, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> default:</span><br><span> OSMO_ASSERT(0);</span><br><span> }</span><br><span>@@ -336,7 +350,6 @@</span><br><span> OSMO_ASSERT(fi->fsm == &iso7816_3_fsm);</span><br><span> </span><br><span> switch (event) {</span><br><span style="color: hsl(0, 100%, 40%);">- case ISO7816_E_WTIME_EXP:</span><br><span> case ISO7816_E_HW_ERR_IND:</span><br><span> case ISO7816_E_CARD_REMOVAL:</span><br><span> /* FIXME: power off? */</span><br><span>@@ -366,7 +379,8 @@</span><br><span> },</span><br><span> [ISO7816_S_WAIT_ATR] = {</span><br><span> .name = "WAIT_ATR",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ISO7816_S_RESET) |</span><br><span> S(ISO7816_S_IN_ATR),</span><br><span> .action = iso7816_3_wait_atr_action,</span><br><span>@@ -375,7 +389,9 @@</span><br><span> .name = "IN_ATR",</span><br><span> .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span> S(ISO7816_E_RX_ERR_IND) |</span><br><span style="color: hsl(0, 100%, 40%);">- S(ISO7816_E_ATR_DONE_IND),</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_ATR_DONE_IND) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_ATR_ERR_IND) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ISO7816_S_RESET) |</span><br><span> S(ISO7816_S_IN_ATR) |</span><br><span> S(ISO7816_S_WAIT_TPDU),</span><br><span>@@ -398,7 +414,8 @@</span><br><span> S(ISO7816_E_TX_COMPL) |</span><br><span> S(ISO7816_E_RX_ERR_IND) |</span><br><span> S(ISO7816_E_TX_ERR_IND) |</span><br><span style="color: hsl(0, 100%, 40%);">- S(ISO7816_E_TPDU_DONE_IND),</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_TPDU_DONE_IND) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ISO7816_S_RESET) |</span><br><span> S(ISO7816_S_WAIT_TPDU) |</span><br><span> S(ISO7816_S_IN_TPDU),</span><br><span>@@ -435,8 +452,7 @@</span><br><span> .log_subsys = DISO7816,</span><br><span> .event_names = iso7816_3_event_names,</span><br><span> .allstate_action = iso7816_3_allstate_action,</span><br><span style="color: hsl(0, 100%, 40%);">- .allstate_event_mask = S(ISO7816_E_WTIME_EXP) |</span><br><span style="color: hsl(0, 100%, 40%);">- S(ISO7816_E_CARD_REMOVAL) |</span><br><span style="color: hsl(120, 100%, 40%);">+ .allstate_event_mask = S(ISO7816_E_CARD_REMOVAL) |</span><br><span> S(ISO7816_E_POWER_DN_IND) |</span><br><span> S(ISO7816_E_RESET_ACT_IND) |</span><br><span> S(ISO7816_E_HW_ERR_IND) |</span><br><span>@@ -550,6 +566,9 @@</span><br><span> }</span><br><span> atp->i = 0; /* first interface byte sub-group is coming (T0 is kind of TD0) */</span><br><span> break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_WTIME_EXP:</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(fi->proc.parent, ISO7816_E_ATR_ERR_IND, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> default: </span><br><span> OSMO_ASSERT(0);</span><br><span> }</span><br><span>@@ -642,6 +661,20 @@</span><br><span> OSMO_ASSERT(0);</span><br><span> }</span><br><span> break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_WTIME_EXP:</span><br><span style="color: hsl(120, 100%, 40%);">+ switch (fi->state) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case ATR_S_WAIT_HIST:</span><br><span style="color: hsl(120, 100%, 40%);">+ case ATR_S_WAIT_TCK:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Some cards have an ATR with long indication of historical bytes */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* FIXME: should we check the checksum? */</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_state_chg(fi, ATR_S_DONE, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(fi->proc.parent, ISO7816_E_ATR_DONE_IND, atp->atr);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ default:</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(fi->proc.parent, ISO7816_E_ATR_ERR_IND, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> default:</span><br><span> OSMO_ASSERT(0);</span><br><span> }</span><br><span>@@ -650,7 +683,8 @@</span><br><span> static const struct osmo_fsm_state atr_states[] = {</span><br><span> [ATR_S_WAIT_TS] = {</span><br><span> .name = "WAIT_TS",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_T0),</span><br><span> .action = atr_wait_ts_action,</span><br><span>@@ -658,7 +692,8 @@</span><br><span> },</span><br><span> [ATR_S_WAIT_T0] = {</span><br><span> .name = "WAIT_T0",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_TA) |</span><br><span> S(ATR_S_WAIT_TB) |</span><br><span>@@ -671,7 +706,8 @@</span><br><span> },</span><br><span> [ATR_S_WAIT_TA] = {</span><br><span> .name = "WAIT_TA",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_TB) |</span><br><span> S(ATR_S_WAIT_TC) |</span><br><span>@@ -683,7 +719,8 @@</span><br><span> },</span><br><span> [ATR_S_WAIT_TB] = {</span><br><span> .name = "WAIT_TB",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_TC) |</span><br><span> S(ATR_S_WAIT_TD) |</span><br><span>@@ -694,7 +731,8 @@</span><br><span> },</span><br><span> [ATR_S_WAIT_TC] = {</span><br><span> .name = "WAIT_TC",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_TD) |</span><br><span> S(ATR_S_WAIT_HIST) |</span><br><span>@@ -704,7 +742,8 @@</span><br><span> },</span><br><span> [ATR_S_WAIT_TD] = {</span><br><span> .name = "WAIT_TD",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_TA) |</span><br><span> S(ATR_S_WAIT_TB) |</span><br><span>@@ -717,15 +756,18 @@</span><br><span> },</span><br><span> [ATR_S_WAIT_HIST] = {</span><br><span> .name = "WAIT_HIST",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_WAIT_TCK) |</span><br><span style="color: hsl(0, 100%, 40%);">- S(ATR_S_WAIT_T0),</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ATR_S_WAIT_T0) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ATR_S_DONE),</span><br><span> .action = atr_wait_tX_action,</span><br><span> },</span><br><span> [ATR_S_WAIT_TCK] = {</span><br><span> .name = "WAIT_TCK",</span><br><span style="color: hsl(0, 100%, 40%);">- .in_event_mask = S(ISO7816_E_RX_SINGLE),</span><br><span style="color: hsl(120, 100%, 40%);">+ .in_event_mask = S(ISO7816_E_RX_SINGLE) |</span><br><span style="color: hsl(120, 100%, 40%);">+ S(ISO7816_E_WTIME_EXP),</span><br><span> .out_state_mask = S(ATR_S_WAIT_TS) |</span><br><span> S(ATR_S_DONE),</span><br><span> .action = atr_wait_tX_action,</span><br><span>diff --git a/ccid_common/iso7816_fsm.h b/ccid_common/iso7816_fsm.h</span><br><span>index 9a6b24e..f2c7483 100644</span><br><span>--- a/ccid_common/iso7816_fsm.h</span><br><span>+++ b/ccid_common/iso7816_fsm.h</span><br><span>@@ -27,6 +27,7 @@</span><br><span> </span><br><span> /* internal events between FSMs in this file */</span><br><span> ISO7816_E_ATR_DONE_IND, /*!< ATR Done indication from ATR child FSM */</span><br><span style="color: hsl(120, 100%, 40%);">+ ISO7816_E_ATR_ERR_IND, /*!< ATR Error indication from ATR child FSM */</span><br><span> ISO7816_E_TPDU_DONE_IND, /*!< TPDU Done indication from TPDU child FSM */</span><br><span> ISO7816_E_TPDU_CLEAR_REQ, /*!< Return TPDU FSM to TPDU_S_INIT */</span><br><span> };</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/15746">change 15746</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/15746"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-ccid-firmware </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I62d47cb5e06b480941c67122f3c7d7a462ea2099 </div>
<div style="display:none"> Gerrit-Change-Number: 15746 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>