Change in libosmocore[master]: i460_mux: correctly reset subchannels

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/19381 )

Change subject: i460_mux: correctly reset subchannels
......................................................................

i460_mux: correctly reset subchannels

When a subchannel is deleted or created the initalization mainly
consists of a memset over the wohle subchannel struct a message buffer
initailization.

However, when we delete a subchannel we also must take of the resetting
of the related struct. Currently this is done with a memest.
Unfortunately this creates not only a memory leak (there might be still
items in the multiplexer tx queue) but also it makes the application
crash when the message buffer is used the next time since the llist_head
of the tx queue looses its initialization.

Lets fix the memory leak problem and the message buffer problem and put
the reset functionality in a single place.

Change-Id: I937a9d4db95f44a860cd2c5dbb660dc1970b9a49
---
M src/gsm/i460_mux.c
1 file changed, 23 insertions(+), 7 deletions(-)

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



diff --git a/src/gsm/i460_mux.c b/src/gsm/i460_mux.c
index 3fb63ec..50cb56e 100644
--- a/src/gsm/i460_mux.c
+++ b/src/gsm/i460_mux.c
@@ -306,6 +306,26 @@
 	return -1;
 }
 
+/* reset subchannel struct into a defined state */
+static void subchan_reset(struct osmo_i460_subchan *schan, bool first_time)
+{
+	/* Before we zero out the subchannel struct, we must be sure that the
+	 * tx_queue is cleared and all dynamically allocated memory is freed.
+	 * However, on an uninitalized subchannel struct we can not be sure
+	 * that the pointers are valid. If the subchannel is reset the first
+	 * time the caller must set first_time to true. */
+	if (!first_time) {
+		if (schan->demux.out_bitbuf)
+			talloc_free(schan->demux.out_bitbuf);
+		msgb_queue_free(&schan->mux.tx_queue);
+	}
+
+	/* Reset subchannel to a defined state */
+	memset(schan, 0, sizeof(*schan));
+	schan->rate = OSMO_I460_RATE_NONE;
+	INIT_LLIST_HEAD(&schan->mux.tx_queue);
+}
+
 /*! initialize an I.460 timeslot */
 void osmo_i460_ts_init(struct osmo_i460_timeslot *ts)
 {
@@ -313,10 +333,7 @@
 
 	for (i = 0; i < ARRAY_SIZE(ts->schan); i++) {
 		struct osmo_i460_subchan *schan = &ts->schan[i];
-
-		memset(schan, 0, sizeof(*schan));
-		schan->rate = OSMO_I460_RATE_NONE;
-		INIT_LLIST_HEAD(&schan->mux.tx_queue);
+		subchan_reset(schan, true);
 	}
 }
 
@@ -345,7 +362,7 @@
 	schan->demux.user_data = chd->demux.user_data;
 	rc = alloc_bitbuf(ctx, schan, chd->demux.num_bits);
 	if (rc < 0) {
-		memset(schan, 0, sizeof(*schan));
+		subchan_reset(schan, false);
 		return NULL;
 	}
 
@@ -356,8 +373,7 @@
 /* remove a su-channel from the multiplex */
 void osmo_i460_subchan_del(struct osmo_i460_subchan *schan)
 {
-	talloc_free(schan->demux.out_bitbuf);
-	memset(schan, 0, sizeof(*schan));
+	subchan_reset(schan, false);
 }
 
 /*! @} */

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I937a9d4db95f44a860cd2c5dbb660dc1970b9a49
Gerrit-Change-Number: 19381
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200730/87b255f4/attachment.htm>