Change in ...osmo-remsim[master]: rspro_client_fsm/remsim_client: Fix double-free

[laforge]

laforge has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-remsim/+/14907 )

Change subject: rspro_client_fsm/remsim_client: Fix double-free
......................................................................

rspro_client_fsm/remsim_client: Fix double-free

respro_dec_msg() takes ownership of the input msgb in both
successful and unsuccessful cases, so we must not call talloc_free
on the resulting msgb.

Change-Id: Id54d1b73395da1329a998d213c190da49eb90a93
---
M src/remsim_client.c
M src/rspro_client_fsm.c
M src/simtrace2-remsim_client.c
3 files changed, 5 insertions(+), 2 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/src/remsim_client.c b/src/remsim_client.c
index 88ac4cd..e73fcc4 100644
--- a/src/remsim_client.c
+++ b/src/remsim_client.c
@@ -90,6 +90,7 @@
 		default:
 			break;
 		}
+		msgb_free(msg);
 		break;
 	case IPAC_PROTO_OSMO:
 		if (!he || msgb_l2len(msg) < sizeof(*he))
diff --git a/src/rspro_client_fsm.c b/src/rspro_client_fsm.c
index 768c15f..06364da 100644
--- a/src/rspro_client_fsm.c
+++ b/src/rspro_client_fsm.c
@@ -130,6 +130,7 @@
 			break;
 		default:
 			break;
+		msgb_free(msg);
 		}
 		break;
 	case IPAC_PROTO_OSMO:
@@ -139,6 +140,8 @@
 		switch (he->proto) {
 		case IPAC_PROTO_EXT_RSPRO:
 			LOGPFSM(srvc->fi, "Received RSPRO %s\n", msgb_hexdump(msg));
+			/* respro_dec_msg() takes ownership of the input message buffer in successful
+			 * and unsuccessful cases */
 			pdu = rspro_dec_msg(msg);
 			if (!pdu)
 				goto invalid;
@@ -152,7 +155,6 @@
 	default:
 		goto invalid;
 	}
-	msgb_free(msg);
 	return rc;
 
 invalid:
diff --git a/src/simtrace2-remsim_client.c b/src/simtrace2-remsim_client.c
index 04aa0c6..094773f 100644
--- a/src/simtrace2-remsim_client.c
+++ b/src/simtrace2-remsim_client.c
@@ -578,6 +578,7 @@
 
 static int bankd_handle_msg(struct bankd_client *bc, struct msgb *msg)
 {
+	/* rspro_dec_msg takes ownership of msgb and talloc_free()s it in successful and unsuccessful case */
 	RsproPDU_t *pdu = rspro_dec_msg(msg);
 	if (!pdu) {
 		LOGPFSML(bc->bankd_fi, LOGL_ERROR, "Error decoding PDU\n");
@@ -627,7 +628,6 @@
 	LOGPFSML(bc->bankd_fi, LOGL_DEBUG, "Received RSPRO %s\n", msgb_hexdump(msg));
 
 	rc = bankd_handle_msg(bc, msg);
-	msgb_free(msg);
 	return rc;
 
 invalid:

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

Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: Id54d1b73395da1329a998d213c190da49eb90a93
Gerrit-Change-Number: 14907
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190723/bd9a81d9/attachment.htm>