Change in libosmocore[master]: gprs_ns2: dont use llist_for_each when freeing an element

[lynxis lazus]

lynxis lazus has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/25147 )


Change subject: gprs_ns2: dont use llist_for_each when freeing an element
......................................................................

gprs_ns2: dont use llist_for_each when freeing an element

The problem are recursive execution because a free generates an event which could
allow the use to free a nsvcs while the llist_for_each() is still running

Change-Id: I902557fb6e56e6588728a46e43a9cbe3215d5c68
---
M src/gb/gprs_ns2.c
M src/gb/gprs_ns2_sns.c
2 files changed, 21 insertions(+), 15 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/47/25147/1

diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index ca8de19..9c126ac 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -669,7 +669,7 @@
  */
 void gprs_ns2_free_nsvcs(struct gprs_ns2_nse *nse)
 {
-	struct gprs_ns2_vc *nsvc, *tmp;
+	struct gprs_ns2_vc *nsvc;
 
 	if (!nse || nse->freed)
 		return;
@@ -677,7 +677,8 @@
 	if (nse->bss_sns_fi) {
 		osmo_fsm_inst_dispatch(nse->bss_sns_fi, GPRS_SNS_EV_REQ_FREE_NSVCS, NULL);
 	} else {
-		llist_for_each_entry_safe(nsvc, tmp, &nse->nsvc, list) {
+		while (!llist_empty(&nse->nsvc)) {
+			nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list);
 			gprs_ns2_free_nsvc(nsvc);
 		}
 	}
@@ -893,7 +894,7 @@
  *  \param[in] nse NS Entity to destroy */
 void gprs_ns2_free_nse(struct gprs_ns2_nse *nse)
 {
-	struct gprs_ns2_vc *nsvc, *nsvc2;
+	struct gprs_ns2_vc *nsvc;
 	if (!nse || nse->freed)
 		return;
 
@@ -907,7 +908,8 @@
 	gprs_ns2_free_nsvcs(nse);
 	ns2_prim_status_ind(nse, NULL, 0, GPRS_NS2_AFF_CAUSE_FAILURE);
 	rate_ctr_group_free(nse->ctrg);
-	llist_for_each_entry_safe(nsvc, nsvc2, &nse->nsvc, list) {
+	while (!llist_empty(&nse->nsvc)) {
+		nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list);
 		gprs_ns2_free_nsvc(nsvc);
 	}
 
@@ -917,9 +919,10 @@
 
 void gprs_ns2_free_nses(struct gprs_ns2_inst *nsi)
 {
-	struct gprs_ns2_nse *nse, *ntmp;
+	struct gprs_ns2_nse *nse;
 
-	llist_for_each_entry_safe(nse, ntmp, &nsi->nse, list) {
+	while (!llist_empty(&nsi->nse)) {
+		nse = llist_first_entry(&nsi->nse, struct gprs_ns2_nse, list);
 		gprs_ns2_free_nse(nse);
 	}
 }
@@ -1473,22 +1476,23 @@
  *  \param[in] bind the bind we want to destroy */
 void gprs_ns2_free_bind(struct gprs_ns2_vc_bind *bind)
 {
-	struct gprs_ns2_vc *nsvc, *tmp;
+	struct gprs_ns2_vc *nsvc;
 	struct gprs_ns2_nse *nse;
 	if (!bind || bind->freed)
 		return;
 
 	bind->freed = true;
-	llist_for_each_entry_safe(nsvc, tmp, &bind->nsvc, blist) {
-		gprs_ns2_free_nsvc(nsvc);
-	}
-
 	if (gprs_ns2_is_ip_bind(bind)) {
 		llist_for_each_entry(nse, &bind->nsi->nse, list) {
 			gprs_ns2_sns_del_bind(nse, bind);
 		}
 	}
 
+	while (!llist_empty(&bind->nsvc)) {
+		nsvc = llist_first_entry(&bind->nsvc, struct gprs_ns2_vc, blist);
+		gprs_ns2_free_nsvc(nsvc);
+	}
+
 	if (bind->driver->free_bind)
 		bind->driver->free_bind(bind);
 
@@ -1500,9 +1504,10 @@
 
 void gprs_ns2_free_binds(struct gprs_ns2_inst *nsi)
 {
-	struct gprs_ns2_vc_bind *bind, *tbind;
+	struct gprs_ns2_vc_bind *bind;
 
-	llist_for_each_entry_safe(bind, tbind, &nsi->binding, list) {
+	while (!llist_empty(&nsi->binding)) {
+		bind = llist_first_entry(&nsi->binding, struct gprs_ns2_vc_bind, list);
 		gprs_ns2_free_bind(bind);
 	}
 }
diff --git a/src/gb/gprs_ns2_sns.c b/src/gb/gprs_ns2_sns.c
index f38a60d..a9964b4 100644
--- a/src/gb/gprs_ns2_sns.c
+++ b/src/gb/gprs_ns2_sns.c
@@ -1565,7 +1565,7 @@
 {
 	struct ns2_sns_state *gss = (struct ns2_sns_state *) fi->priv;
 	struct gprs_ns2_nse *nse = nse_inst_from_fi(fi);
-	struct gprs_ns2_vc *nsvc, *nsvc2;
+	struct gprs_ns2_vc *nsvc;
 
 	/* reset when receiving GPRS_SNS_EV_REQ_NO_NSVC */
 	switch (event) {
@@ -1581,7 +1581,8 @@
 		/* tear down previous state
 		 * gprs_ns2_free_nsvcs() will trigger NO_NSVC, prevent this from triggering a reselection */
 		gss->reselection_running = true;
-		llist_for_each_entry_safe(nsvc, nsvc2, &nse->nsvc, list) {
+		while (!llist_empty(&nse->nsvc)) {
+			nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list);
 			gprs_ns2_free_nsvc(nsvc);
 		}
 		ns2_clear_elems(&gss->local);

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I902557fb6e56e6588728a46e43a9cbe3215d5c68
Gerrit-Change-Number: 25147
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210806/b3df63bf/attachment.htm>