Change in libosmo-abis[master]: fix double-free/use-after-free of pointers in struct e1inp_line

[Stefan Sperling]

Stefan Sperling has uploaded this change for review. ( https://gerrit.osmocom.org/9252


Change subject: fix double-free/use-after-free of pointers in struct e1inp_line
......................................................................

fix double-free/use-after-free of pointers in struct e1inp_line

Ensure that pointers in cloned e1inp_lines point to valid memory.
Some members of struct e1inp_line can simply be deep-copied.
Use talloc reference counting for pointers to objects which may
be shared between clones (driver-private state and counters).
Prevents double-free bugs, e.g. when multiple links referring
to the same line are closed.

Also, do not forget to unlink struct e1inp_line's counter group from
the counter list. Fixes use-after-free in rate_ctr_timer_cb() during
osmo-bts shutdown.

Change-Id: I9f4724b4a5a064801591e9acf4f2fd1db006d082
Related: OS#3011
Related: OS#3137
Related: OS#3282
---
M src/e1_input.c
1 file changed, 27 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/52/9252/1

diff --git a/src/e1_input.c b/src/e1_input.c
index 29ba440..f5c167a 100644
--- a/src/e1_input.c
+++ b/src/e1_input.c
@@ -394,6 +394,25 @@
 	        return NULL;
 
 	memcpy(clone, line, sizeof(struct e1inp_line));
+
+	if (line->name) {
+		clone->name = talloc_strdup(clone, line->name);
+		OSMO_ASSERT(clone->name);
+	}
+	if (line->sock_path) {
+		clone->sock_path = talloc_strdup(clone, line->sock_path);
+		OSMO_ASSERT(clone->sock_path);
+	}
+
+	/*
+	 * Rate counters and driver data are shared between clones. These are pointers
+	 * to dynamic memory so we must use reference counting to avoid a double-free.
+	 */
+	OSMO_ASSERT(line->rate_ctr);
+	clone->rate_ctr = talloc_reference(clone, line->rate_ctr);
+	if (line->driver_data)
+		clone->driver_data = talloc_reference(clone, line->driver_data);
+
 	clone->refcnt = 1;
 	return clone;
 }
@@ -406,8 +425,15 @@
 void e1inp_line_put(struct e1inp_line *line)
 {
 	line->refcnt--;
-	if (line->refcnt == 0)
+	if (line->refcnt == 0) {
+		/* Remove our counter group from the global counter list
+		 * if we were holding the last remaining reference. */
+		if (talloc_reference_count(line->rate_ctr) == 0)
+			rate_ctr_group_free(line->rate_ctr);
+		else
+			talloc_unlink(line, line->rate_ctr);
 		talloc_free(line);
+	}
 }
 
 void

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

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f4724b4a5a064801591e9acf4f2fd1db006d082
Gerrit-Change-Number: 9252
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180522/8c4ac0bf/attachment.htm>