Change in osmo-ccid-firmware[master]: better ccid error handling, fix buffer leaks

[Hoernchen]

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16282 )


Change subject: better ccid error handling, fix buffer leaks
......................................................................

better ccid error handling, fix buffer leaks

Change-Id: Ib8b8524809e12608a7ade79ce7d7c3ced16eeb57
---
M ccid_common/ccid_device.c
M ccid_common/ccid_slot_fsm.c
M ccid_common/iso7816_fsm.c
3 files changed, 30 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware refs/changes/82/16282/1

diff --git a/ccid_common/ccid_device.c b/ccid_common/ccid_device.c
index 11edd3e..b14f194 100644
--- a/ccid_common/ccid_device.c
+++ b/ccid_common/ccid_device.c
@@ -475,14 +475,25 @@
 	const struct ccid_header *ch = (const struct ccid_header *) u;
 	uint8_t seq = u->reset_parameters.hdr.bSeq;
 	struct msgb *resp;
+	int rc;
 
 	/* copy default parameters from somewhere */
 	/* FIXME: T=1 */
-	cs->ci->slot_ops->set_params(cs, seq, CCID_PROTOCOL_NUM_T0, cs->default_pars);
-	cs->pars = *cs->default_pars;
 
-	resp = ccid_gen_parameters_t0(cs, seq, CCID_CMD_STATUS_OK, 0);
-	return ccid_slot_send_unbusy(cs, resp);
+	/* validate parameters; abort if they are not supported */
+	rc = cs->ci->slot_ops->set_params(cs, seq, CCID_PROTOCOL_NUM_T0, cs->default_pars);
+	if (rc < 0) {
+		resp = ccid_gen_parameters_t0(cs, seq, CCID_CMD_STATUS_FAILED, -rc);
+		goto out;
+	}
+
+	msgb_free(msg);
+	/* busy, tdpu like callback */
+	return 1;
+out:
+	msgb_free(msg);
+	ccid_slot_send_unbusy(cs, resp);
+	return 1;
 }
 
 /* Section 6.1.7 */
@@ -523,10 +534,14 @@
 		resp = ccid_gen_parameters_t0(cs, seq, CCID_CMD_STATUS_FAILED, -rc);
 		goto out;
 	}
+
+	msgb_free(msg);
 	/* busy, tdpu like callback */
 	return 1;
 out:
-	return ccid_slot_send_unbusy(cs, resp);
+	msgb_free(msg);
+	ccid_slot_send_unbusy(cs, resp);
+	return 1;
 }
 
 /* Section 6.1.8 */
@@ -682,6 +697,15 @@
 		return ccid_send(ci, resp);
 	}
 
+	if(!cs->icc_present) {
+		LOGPCS(cs, LOGL_ERROR, "No icc present, but another cmd received\n");
+		/* FIXME: ABORT logic as per section 5.3.1 of CCID Spec v1.1 */
+		resp = gen_err_resp(ch->bMessageType, ch->bSlot, get_icc_status(cs), ch->bSeq,
+				CCID_ERR_ICC_MUTE);
+		msgb_free(msg);
+		return ccid_send(ci, resp);
+	}
+
 	LOGPCS(cs, LOGL_DEBUG, "Rx CCID(OUT) %s %s\n",
 		get_value_string(ccid_msg_type_vals, ch->bMessageType), msgb_hexdump(msg));
 
diff --git a/ccid_common/ccid_slot_fsm.c b/ccid_common/ccid_slot_fsm.c
index 8d38f29..d2aec26 100644
--- a/ccid_common/ccid_slot_fsm.c
+++ b/ccid_common/ccid_slot_fsm.c
@@ -179,9 +179,6 @@
 	struct iso_fsm_slot *ss = ccid_slot2iso_fsm_slot(cs);
 	struct msgb *tpdu;
 
-	if(!cs->icc_present)
-		return -CCID_ERR_ICC_MUTE;
-
 	ss->seq = xfb->hdr.bSeq;
 
 	/* must be '0' for TPDU level exchanges or for short APDU */
diff --git a/ccid_common/iso7816_fsm.c b/ccid_common/iso7816_fsm.c
index 623193f..3778dcc 100644
--- a/ccid_common/iso7816_fsm.c
+++ b/ccid_common/iso7816_fsm.c
@@ -293,6 +293,7 @@
 		ip->user_cb(fi, event, 0, atr);
 		break;
 	case ISO7816_E_ATR_ERR_IND:
+		atr = data;
 		osmo_fsm_inst_state_chg(fi, ISO7816_S_RESET, 0, 0);
 		ip->user_cb(fi, event, 0, atr);
 		break;

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16282
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ccid-firmware
Gerrit-Branch: master
Gerrit-Change-Id: Ib8b8524809e12608a7ade79ce7d7c3ced16eeb57
Gerrit-Change-Number: 16282
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <ewild at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191127/262655c4/attachment.htm>