pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email )
Change subject: abis: Fix reusing bts->*_link while it is being destroyed ......................................................................
abis: Fix reusing bts->*_link while it is being destroyed
Call to e1inp_sign_link_destroy() may trigger a sign_down() callback, which if happening synchronously, could end up reentring the same code path we are in before bts->*_link was set to NULL. Avoid it by marking the pointer as NULL immediatelly before calling e1inp_sign_link_destroy().
Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 --- M src/osmo-bsc/bts_ipaccess_nanobts.c 1 file changed, 13 insertions(+), 3 deletions(-)
Approvals: osmith: Looks good to me, but someone else must approve laforge: Looks good to me, approved Jenkins Builder: Verified
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c index b0532e5..9382fa4 100644 --- a/src/osmo-bsc/bts_ipaccess_nanobts.c +++ b/src/osmo-bsc/bts_ipaccess_nanobts.c @@ -508,12 +508,17 @@ /* These are exported because they are used by the VTY interface. */ void ipaccess_drop_rsl(struct gsm_bts_trx *trx, const char *reason) { + struct e1inp_sign_link *link; + if (!trx->rsl_link_primary) return;
LOG_TRX(trx, DLINP, LOGL_NOTICE, "Dropping RSL link: %s\n", reason); - e1inp_sign_link_destroy(trx->rsl_link_primary); + /* Mark bts->rsl_link_primary ptr null before calling sign_link_destroy, + * to avoid a callback triggering this same code path. */ + link = trx->rsl_link_primary; trx->rsl_link_primary = NULL; + e1inp_sign_link_destroy(link); osmo_stat_item_dec(osmo_stat_item_group_get_item(trx->bts->bts_statg, BTS_STAT_RSL_CONNECTED), 1);
if (trx->bts->c0 == trx) @@ -529,6 +534,7 @@ uint8_t i; struct timespec tp; int rc; + struct e1inp_sign_link *link;
/* First of all, remove deferred drop if enabled */ osmo_timer_del(&bts->oml_drop_link_timer); @@ -537,8 +543,11 @@ return;
LOG_BTS(bts, DLINP, LOGL_NOTICE, "Dropping OML link: %s\n", reason); - e1inp_sign_link_destroy(bts->oml_link); + /* Mark bts->oml_link ptr null before calling sign_link_destroy, + * to avoid a callback triggering this same code path. */ + link = bts->oml_link; bts->oml_link = NULL; + e1inp_sign_link_destroy(link); rc = osmo_clock_gettime(CLOCK_MONOTONIC, &tp); bts->updowntime = (rc < 0) ? 0 : tp.tv_sec; /* we don't need sub-second precision for downtime */ osmo_stat_item_dec(osmo_stat_item_group_get_item(bts->bts_statg, BTS_STAT_OML_CONNECTED), 1); @@ -546,8 +555,9 @@
/* Also drop the associated OSMO link */ OSMO_ASSERT(bts->osmo_link); - e1inp_sign_link_destroy(bts->osmo_link); + link = bts->osmo_link; bts->osmo_link = NULL; + e1inp_sign_link_destroy(link);
bts_setup_ramp_remove(bts);