From f074659520a767a37bebef3c2b1e3ab814f48bd3 Mon Sep 17 00:00:00 2001 From: Alexander Couzens Date: Tue, 20 Jul 2021 19:05:45 +0200 Subject: gprs_ns2: don't 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 --- src/gb/gprs_ns2.c | 42 ++++++++++++++++++++++++++---------------- src/gb/gprs_ns2_internal.h | 1 + src/gb/gprs_ns2_sns.c | 5 +---- 3 files changed, 28 insertions(+), 20 deletions(-) (limited to 'src') diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c index 326312c9..a895e3d0 100644 --- a/src/gb/gprs_ns2.c +++ b/src/gb/gprs_ns2.c @@ -664,22 +664,29 @@ void gprs_ns2_free_nsvc(struct gprs_ns2_vc *nsvc) talloc_free(nsvc); } +void ns2_free_nsvcs(struct gprs_ns2_nse *nse) +{ + struct gprs_ns2_vc *nsvc; + + /* prevent recursive free() when the user reacts on a down event and free() a second time */ + while (!llist_empty(&nse->nsvc)) { + nsvc = llist_first_entry(&nse->nsvc, struct gprs_ns2_vc, list); + gprs_ns2_free_nsvc(nsvc); + } +} + /*! Destroy/release all NS-VC of given NSE * \param[in] nse NSE */ void gprs_ns2_free_nsvcs(struct gprs_ns2_nse *nse) { - struct gprs_ns2_vc *nsvc, *tmp; - if (!nse || nse->freed) return; if (nse->bss_sns_fi) { osmo_fsm_inst_dispatch(nse->bss_sns_fi, NS2_SNS_EV_REQ_FREE_NSVCS, NULL); } else { - llist_for_each_entry_safe(nsvc, tmp, &nse->nsvc, list) { - gprs_ns2_free_nsvc(nsvc); - } + ns2_free_nsvcs(nse); } } @@ -893,7 +900,6 @@ uint16_t gprs_ns2_nse_nsei(struct gprs_ns2_nse *nse) * \param[in] nse NS Entity to destroy */ void gprs_ns2_free_nse(struct gprs_ns2_nse *nse) { - struct gprs_ns2_vc *nsvc, *nsvc2; if (!nse || nse->freed) return; @@ -907,9 +913,7 @@ void gprs_ns2_free_nse(struct gprs_ns2_nse *nse) 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) { - gprs_ns2_free_nsvc(nsvc); - } + ns2_free_nsvcs(nse); llist_del(&nse->list); talloc_free(nse); @@ -917,9 +921,11 @@ void gprs_ns2_free_nse(struct gprs_ns2_nse *nse) 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) { + /* prevent recursive free() when the user reacts on a down event and free() a second time */ + while (!llist_empty(&nsi->nse)) { + nse = llist_first_entry(&nsi->nse, struct gprs_ns2_nse, list); gprs_ns2_free_nse(nse); } } @@ -1473,19 +1479,21 @@ void gprs_ns2_start_alive_all_nsvcs(struct gprs_ns2_nse *nse) * \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; + if (gprs_ns2_is_ip_bind(bind)) { llist_for_each_entry(nse, &bind->nsi->nse, list) { gprs_ns2_sns_del_bind(nse, bind); } } - llist_for_each_entry_safe(nsvc, tmp, &bind->nsvc, blist) { + /* prevent recursive free() when the user reacts on a down event and free() a second time */ + while (!llist_empty(&bind->nsvc)) { + nsvc = llist_first_entry(&bind->nsvc, struct gprs_ns2_vc, blist); gprs_ns2_free_nsvc(nsvc); } @@ -1500,9 +1508,11 @@ void gprs_ns2_free_bind(struct gprs_ns2_vc_bind *bind) 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) { + /* prevent recursive free() when the user reacts on a down event and free() a second time */ + 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_internal.h b/src/gb/gprs_ns2_internal.h index d2407f66..ca6bfb7e 100644 --- a/src/gb/gprs_ns2_internal.h +++ b/src/gb/gprs_ns2_internal.h @@ -354,6 +354,7 @@ struct gprs_ns2_vc *ns2_vc_alloc(struct gprs_ns2_vc_bind *bind, enum gprs_ns2_vc_mode vc_mode, const char *id); +void ns2_free_nsvcs(struct gprs_ns2_nse *nse); int ns2_bind_alloc(struct gprs_ns2_inst *nsi, const char *name, struct gprs_ns2_vc_bind **result); diff --git a/src/gb/gprs_ns2_sns.c b/src/gb/gprs_ns2_sns.c index e96f6b3f..43e49208 100644 --- a/src/gb/gprs_ns2_sns.c +++ b/src/gb/gprs_ns2_sns.c @@ -1565,7 +1565,6 @@ static void ns2_sns_st_all_action_bss(struct osmo_fsm_inst *fi, uint32_t event, { 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; /* reset when receiving NS2_SNS_EV_REQ_NO_NSVC */ switch (event) { @@ -1581,9 +1580,7 @@ static void ns2_sns_st_all_action_bss(struct osmo_fsm_inst *fi, uint32_t event, /* 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) { - gprs_ns2_free_nsvc(nsvc); - } + ns2_free_nsvcs(nse); ns2_clear_elems(&gss->local); ns2_clear_elems(&gss->remote); -- cgit v1.2.3