Change in osmocom-bb[master]: layer23/sap_interface.c: fix: properly call sap_msg_free()

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/12437 )

Change subject: layer23/sap_interface.c: fix: properly call sap_msg_free()
......................................................................

layer23/sap_interface.c: fix: properly call sap_msg_free()

Passing NULL to sap_msg_free() is not only meaningless, but also
would result in NULL pointer dereference. We should call it in
successful case only, so let's fix this.

Change-Id: Icf868c4299e292a17c4b7aad1f9e728ea3653494
---
M src/host/layer23/src/common/sap_interface.c
1 file changed, 8 insertions(+), 6 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/host/layer23/src/common/sap_interface.c b/src/host/layer23/src/common/sap_interface.c
index dd7737a..b39a57f 100644
--- a/src/host/layer23/src/common/sap_interface.c
+++ b/src/host/layer23/src/common/sap_interface.c
@@ -296,15 +296,15 @@
 
 static void sap_parse_resp(struct osmocom_ms *ms, uint8_t *data, uint16_t len)
 {
-	struct sap_msg *msg = NULL;
+	struct sap_msg *msg;
 	if(len > ms->sap_entity.max_msg_size){
 		LOGP(DSAP, LOGL_ERROR, "Read more data than allowed by max_msg_size, ignoring.\n");
 		return;
 	}
 
 	msg = sap_parse_msg(data);
-	if(!msg){
-		sap_msg_free(msg);
+	if (!msg) {
+		LOGP(DSAP, LOGL_ERROR, "Failed to parse SAP message\n");
 		return;
 	}
 
@@ -335,7 +335,7 @@
 	case SAP_TRANSFER_ATR_RESP:
 		if(ms->sap_entity.sap_state != SAP_PROCESSING_ATR_REQUEST){
 			LOGP(DSAP, LOGL_ERROR, "got ATR resp in state: %u\n", ms->sap_entity.sap_state);
-			return;
+			break;
 		}
 		if(msg->num_params >= 2){
 			LOGP(DSAP, LOGL_INFO, "ATR: %s\n", osmo_hexdump(msg->params[1].value, msg->params[1].len));
@@ -345,11 +345,11 @@
 	case SAP_TRANSFER_APDU_RESP:
 		if(ms->sap_entity.sap_state != SAP_PROCESSING_APDU_REQUEST){
 			LOGP(DSAP, LOGL_ERROR, "got APDU resp in state: %u\n", ms->sap_entity.sap_state);
-			return;
+			break;
 		}
 		if(msg->num_params != 2){
 			LOGP(DSAP, LOGL_ERROR, "wrong number of parameters %u in APDU response\n", msg->num_params);
-			return;
+			break;
 		}
 		ms->sap_entity.sap_state = SAP_IDLE;
 		if(sap_parse_result(&msg->params[0]) == 0){
@@ -369,6 +369,8 @@
 		LOGP(DSAP, LOGL_ERROR, "got unknown or not implemented SAP msgid: %u\n", msg->id);
 		break;
 	}
+
+	sap_msg_free(msg);
 }
 
 static int sap_read(struct osmo_fd *fd)

-- 
To view, visit https://gerrit.osmocom.org/12437
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icf868c4299e292a17c4b7aad1f9e728ea3653494
Gerrit-Change-Number: 12437
Gerrit-PatchSet: 3
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190107/e0122e53/attachment.htm>