<p>Stefan Sperling has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/9252">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fix double-free/use-after-free of pointers in struct e1inp_line<br><br>Ensure that pointers in cloned e1inp_lines point to valid memory.<br>Some members of struct e1inp_line can simply be deep-copied.<br>Use talloc reference counting for pointers to objects which may<br>be shared between clones (driver-private state and counters).<br>Prevents double-free bugs, e.g. when multiple links referring<br>to the same line are closed.<br><br>Also, do not forget to unlink struct e1inp_line's counter group from<br>the counter list. Fixes use-after-free in rate_ctr_timer_cb() during<br>osmo-bts shutdown.<br><br>Change-Id: I9f4724b4a5a064801591e9acf4f2fd1db006d082<br>Related: OS#3011<br>Related: OS#3137<br>Related: OS#3282<br>---<br>M src/e1_input.c<br>1 file changed, 27 insertions(+), 1 deletion(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/52/9252/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/e1_input.c b/src/e1_input.c</span><br><span>index 29ba440..f5c167a 100644</span><br><span>--- a/src/e1_input.c</span><br><span>+++ b/src/e1_input.c</span><br><span>@@ -394,6 +394,25 @@</span><br><span>              return NULL;</span><br><span> </span><br><span>     memcpy(clone, line, sizeof(struct e1inp_line));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     if (line->name) {</span><br><span style="color: hsl(120, 100%, 40%);">+          clone->name = talloc_strdup(clone, line->name);</span><br><span style="color: hsl(120, 100%, 40%);">+         OSMO_ASSERT(clone->name);</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (line->sock_path) {</span><br><span style="color: hsl(120, 100%, 40%);">+             clone->sock_path = talloc_strdup(clone, line->sock_path);</span><br><span style="color: hsl(120, 100%, 40%);">+               OSMO_ASSERT(clone->sock_path);</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /*</span><br><span style="color: hsl(120, 100%, 40%);">+     * Rate counters and driver data are shared between clones. These are pointers</span><br><span style="color: hsl(120, 100%, 40%);">+         * to dynamic memory so we must use reference counting to avoid a double-free.</span><br><span style="color: hsl(120, 100%, 40%);">+         */</span><br><span style="color: hsl(120, 100%, 40%);">+   OSMO_ASSERT(line->rate_ctr);</span><br><span style="color: hsl(120, 100%, 40%);">+       clone->rate_ctr = talloc_reference(clone, line->rate_ctr);</span><br><span style="color: hsl(120, 100%, 40%);">+      if (line->driver_data)</span><br><span style="color: hsl(120, 100%, 40%);">+             clone->driver_data = talloc_reference(clone, line->driver_data);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>     clone->refcnt = 1;</span><br><span>        return clone;</span><br><span> }</span><br><span>@@ -406,8 +425,15 @@</span><br><span> void e1inp_line_put(struct e1inp_line *line)</span><br><span> {</span><br><span>       line->refcnt--;</span><br><span style="color: hsl(0, 100%, 40%);">-      if (line->refcnt == 0)</span><br><span style="color: hsl(120, 100%, 40%);">+     if (line->refcnt == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+           /* Remove our counter group from the global counter list</span><br><span style="color: hsl(120, 100%, 40%);">+               * if we were holding the last remaining reference. */</span><br><span style="color: hsl(120, 100%, 40%);">+                if (talloc_reference_count(line->rate_ctr) == 0)</span><br><span style="color: hsl(120, 100%, 40%);">+                   rate_ctr_group_free(line->rate_ctr);</span><br><span style="color: hsl(120, 100%, 40%);">+               else</span><br><span style="color: hsl(120, 100%, 40%);">+                  talloc_unlink(line, line->rate_ctr);</span><br><span>              talloc_free(line);</span><br><span style="color: hsl(120, 100%, 40%);">+    }</span><br><span> }</span><br><span> </span><br><span> void</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/9252">change 9252</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/9252"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-abis </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I9f4724b4a5a064801591e9acf4f2fd1db006d082 </div>
<div style="display:none"> Gerrit-Change-Number: 9252 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Stefan Sperling <ssperling@sysmocom.de> </div>