[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
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);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0
Gerrit-Change-Number: 38968
Gerrit-PatchSet: 2
Gerrit-Owner: pespin
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: laforge
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
Attention is currently required from: daniel, fixeria, pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email ) Change subject: abis: Fix reusing bts->*_link while it is being destroyed .. Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 Gerrit-Change-Number: 38968 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: osmith Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-Comment-Date: Fri, 29 Nov 2024 11:49:12 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes
[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
Attention is currently required from: pespin. osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email ) Change subject: abis: Fix reusing bts->*_link while it is being destroyed .. Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 Gerrit-Change-Number: 38968 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 28 Nov 2024 08:34:48 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes
[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder 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(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/68/38968/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0 Gerrit-Change-Number: 38968 Gerrit-PatchSet: 2 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder
[S] Change in osmo-bsc[master]: abis: Fix reusing bts->*_link while it is being destroyed
pespin has uploaded this change for review. (
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(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/68/38968/1
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c
b/src/osmo-bsc/bts_ipaccess_nanobts.c
index b0532e5..b823dfc 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_primar 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);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38968?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ice71b3143f167482e4a23759012b58e9ee13bfc0
Gerrit-Change-Number: 38968
Gerrit-PatchSet: 1
Gerrit-Owner: pespin
