<p>lynxis lazus has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/25147">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">gprs_ns2: dont use llist_for_each when freeing an element<br><br>The problem are recursive execution because a free generates an event which could<br>allow the use to free a nsvcs while the llist_for_each() is still running<br><br>Change-Id: I902557fb6e56e6588728a46e43a9cbe3215d5c68<br>---<br>M src/gb/gprs_ns2.c<br>M src/gb/gprs_ns2_sns.c<br>2 files changed, 21 insertions(+), 15 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/47/25147/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c</span><br><span>index ca8de19..9c126ac 100644</span><br><span>--- a/src/gb/gprs_ns2.c</span><br><span>+++ b/src/gb/gprs_ns2.c</span><br><span>@@ -669,7 +669,7 @@</span><br><span>  */</span><br><span> void gprs_ns2_free_nsvcs(struct gprs_ns2_nse *nse)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct gprs_ns2_vc *nsvc, *tmp;</span><br><span style="color: hsl(120, 100%, 40%);">+       struct gprs_ns2_vc *nsvc;</span><br><span> </span><br><span>        if (!nse || nse->freed)</span><br><span>           return;</span><br><span>@@ -677,7 +677,8 @@</span><br><span>        if (nse->bss_sns_fi) {</span><br><span>            osmo_fsm_inst_dispatch(nse->bss_sns_fi, GPRS_SNS_EV_REQ_FREE_NSVCS, NULL);</span><br><span>        } else {</span><br><span style="color: hsl(0, 100%, 40%);">-                llist_for_each_entry_safe(nsvc, tmp, &nse->nsvc, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+               while (!llist_empty(&nse->nsvc)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                     nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list);</span><br><span>                       gprs_ns2_free_nsvc(nsvc);</span><br><span>            }</span><br><span>    }</span><br><span>@@ -893,7 +894,7 @@</span><br><span>  *  \param[in] nse NS Entity to destroy */</span><br><span> void gprs_ns2_free_nse(struct gprs_ns2_nse *nse)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-  struct gprs_ns2_vc *nsvc, *nsvc2;</span><br><span style="color: hsl(120, 100%, 40%);">+     struct gprs_ns2_vc *nsvc;</span><br><span>    if (!nse || nse->freed)</span><br><span>           return;</span><br><span> </span><br><span>@@ -907,7 +908,8 @@</span><br><span>    gprs_ns2_free_nsvcs(nse);</span><br><span>    ns2_prim_status_ind(nse, NULL, 0, GPRS_NS2_AFF_CAUSE_FAILURE);</span><br><span>       rate_ctr_group_free(nse->ctrg);</span><br><span style="color: hsl(0, 100%, 40%);">-      llist_for_each_entry_safe(nsvc, nsvc2, &nse->nsvc, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+     while (!llist_empty(&nse->nsvc)) {</span><br><span style="color: hsl(120, 100%, 40%);">+             nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list);</span><br><span>               gprs_ns2_free_nsvc(nsvc);</span><br><span>    }</span><br><span> </span><br><span>@@ -917,9 +919,10 @@</span><br><span> </span><br><span> void gprs_ns2_free_nses(struct gprs_ns2_inst *nsi)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-     struct gprs_ns2_nse *nse, *ntmp;</span><br><span style="color: hsl(120, 100%, 40%);">+      struct gprs_ns2_nse *nse;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   llist_for_each_entry_safe(nse, ntmp, &nsi->nse, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+        while (!llist_empty(&nsi->nse)) {</span><br><span style="color: hsl(120, 100%, 40%);">+              nse = llist_first_entry(&nsi->nse, struct gprs_ns2_nse, list);</span><br><span>                gprs_ns2_free_nse(nse);</span><br><span>      }</span><br><span> }</span><br><span>@@ -1473,22 +1476,23 @@</span><br><span>  *  \param[in] bind the bind we want to destroy */</span><br><span> void gprs_ns2_free_bind(struct gprs_ns2_vc_bind *bind)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   struct gprs_ns2_vc *nsvc, *tmp;</span><br><span style="color: hsl(120, 100%, 40%);">+       struct gprs_ns2_vc *nsvc;</span><br><span>    struct gprs_ns2_nse *nse;</span><br><span>    if (!bind || bind->freed)</span><br><span>                 return;</span><br><span> </span><br><span>  bind->freed = true;</span><br><span style="color: hsl(0, 100%, 40%);">-  llist_for_each_entry_safe(nsvc, tmp, &bind->nsvc, blist) {</span><br><span style="color: hsl(0, 100%, 40%);">-               gprs_ns2_free_nsvc(nsvc);</span><br><span style="color: hsl(0, 100%, 40%);">-       }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>    if (gprs_ns2_is_ip_bind(bind)) {</span><br><span>             llist_for_each_entry(nse, &bind->nsi->nse, list) {</span><br><span>                         gprs_ns2_sns_del_bind(nse, bind);</span><br><span>            }</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ while (!llist_empty(&bind->nsvc)) {</span><br><span style="color: hsl(120, 100%, 40%);">+            nsvc = llist_first_entry(&bind->nsvc, struct gprs_ns2_vc, blist);</span><br><span style="color: hsl(120, 100%, 40%);">+              gprs_ns2_free_nsvc(nsvc);</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  if (bind->driver->free_bind)</span><br><span>           bind->driver->free_bind(bind);</span><br><span> </span><br><span>@@ -1500,9 +1504,10 @@</span><br><span> </span><br><span> void gprs_ns2_free_binds(struct gprs_ns2_inst *nsi)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-       struct gprs_ns2_vc_bind *bind, *tbind;</span><br><span style="color: hsl(120, 100%, 40%);">+        struct gprs_ns2_vc_bind *bind;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      llist_for_each_entry_safe(bind, tbind, &nsi->binding, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+  while (!llist_empty(&nsi->binding)) {</span><br><span style="color: hsl(120, 100%, 40%);">+          bind = llist_first_entry(&nsi->binding, struct gprs_ns2_vc_bind, list);</span><br><span>               gprs_ns2_free_bind(bind);</span><br><span>    }</span><br><span> }</span><br><span>diff --git a/src/gb/gprs_ns2_sns.c b/src/gb/gprs_ns2_sns.c</span><br><span>index f38a60d..a9964b4 100644</span><br><span>--- a/src/gb/gprs_ns2_sns.c</span><br><span>+++ b/src/gb/gprs_ns2_sns.c</span><br><span>@@ -1565,7 +1565,7 @@</span><br><span> {</span><br><span>         struct ns2_sns_state *gss = (struct ns2_sns_state *) fi->priv;</span><br><span>    struct gprs_ns2_nse *nse = nse_inst_from_fi(fi);</span><br><span style="color: hsl(0, 100%, 40%);">-        struct gprs_ns2_vc *nsvc, *nsvc2;</span><br><span style="color: hsl(120, 100%, 40%);">+     struct gprs_ns2_vc *nsvc;</span><br><span> </span><br><span>        /* reset when receiving GPRS_SNS_EV_REQ_NO_NSVC */</span><br><span>   switch (event) {</span><br><span>@@ -1581,7 +1581,8 @@</span><br><span>             /* tear down previous state</span><br><span>           * gprs_ns2_free_nsvcs() will trigger NO_NSVC, prevent this from triggering a reselection */</span><br><span>                 gss->reselection_running = true;</span><br><span style="color: hsl(0, 100%, 40%);">-             llist_for_each_entry_safe(nsvc, nsvc2, &nse->nsvc, list) {</span><br><span style="color: hsl(120, 100%, 40%);">+             while (!llist_empty(&nse->nsvc)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                     nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list);</span><br><span>                       gprs_ns2_free_nsvc(nsvc);</span><br><span>            }</span><br><span>            ns2_clear_elems(&gss->local);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/25147">change 25147</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/c/libosmocore/+/25147"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I902557fb6e56e6588728a46e43a9cbe3215d5c68 </div>
<div style="display:none"> Gerrit-Change-Number: 25147 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>