diff options
author | Harald Welte <laforge@osmocom.org> | 2020-05-03 12:00:52 +0200 |
---|---|---|
committer | Sylvain Munaut <tnt@246tNt.com> | 2020-05-08 13:27:40 +0200 |
commit | 456a62e98e5e0ac773244191cba092bf58a91988 (patch) | |
tree | 59cdfe452dd4f1c204c3f883ed5274ca1f7a85ac | |
parent | 7ab23b75afe9d095f30776c6778451d6fc107e79 (diff) |
bts_nokia_site: Fix LAPD segfault during reset procedure
The existing Nokia *Site code destroyed the LAPD SAP instance for OML
while processing an OML message. Once the stack frame returned back
to the LAPD code, the LAPD SAP was gone -> segfault.
Let's work around this by moving deletion of the LAPD SAP out-of-line
by starting a timer 0ms in the future. Not particularly nice, but
effective.
Change-Id: I6270c7210f600e53f845561898245d2fd30a368d
Closes: OS#1761
-rw-r--r-- | include/osmocom/bsc/gsm_data.h | 2 | ||||
-rw-r--r-- | src/osmo-bsc/bts_nokia_site.c | 55 |
2 files changed, 32 insertions, 25 deletions
diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 6996905a4..1d02e3943 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -1143,7 +1143,7 @@ struct gsm_bts { no_loc_rel_cnf:1, /* don't wait for RSL REL CONF */ bts_reset_timer_cnf, /* timer for BTS RESET */ did_reset:1, /* we received a RESET ACK */ - wait_reset:1; /* we are waiting for reset to complete */ + wait_reset:2; /* we are waiting for reset to complete */ struct osmo_timer_list reset_timer; } nokia; }; diff --git a/src/osmo-bsc/bts_nokia_site.c b/src/osmo-bsc/bts_nokia_site.c index 66972c2b5..cde0afa97 100644 --- a/src/osmo-bsc/bts_nokia_site.c +++ b/src/osmo-bsc/bts_nokia_site.c @@ -41,6 +41,12 @@ #include <osmocom/abis/lapd.h> +enum reset_timer_state { + RESET_T_NONE = 0, + RESET_T_STOP_LAPD = 1, /* first timer expiration: stop LAPD SAP */ + RESET_T_RESTART_LAPD = 2, /* second timer expiration: restart LAPD SAP */ +}; + /* TODO: put in a separate file ? */ extern int abis_nm_sendmsg(struct gsm_bts *bts, struct msgb *msg); @@ -1461,18 +1467,28 @@ static void reset_timer_cb(void *_bts) struct gsm_e1_subslot *e1_link = &bts->oml_e1_link; struct e1inp_line *line; - bts->nokia.wait_reset = 0; - /* OML link */ line = e1inp_line_find(e1_link->e1_nr); if (!line) { - LOGP(DLINP, LOGL_ERROR, "BTS %u OML link referring to " - "non-existing E1 line %u\n", bts->nr, e1_link->e1_nr); + LOGP(DLINP, LOGL_ERROR, "BTS %u OML link referring to non-existing E1 line %u\n", + bts->nr, e1_link->e1_nr); return; } - start_sabm_in_line(line, 0, -1); /* stop all first */ - start_sabm_in_line(line, 1, SAPI_OML); /* start only OML */ + switch (bts->nokia.wait_reset) { + case RESET_T_NONE: /* shouldn't happen */ + break; + case RESET_T_STOP_LAPD: + start_sabm_in_line(line, 0, -1); /* stop all first */ + bts->nokia.wait_reset = RESET_T_RESTART_LAPD; + osmo_timer_schedule(&bts->nokia.reset_timer, bts->nokia.bts_reset_timer_cnf, 0); + break; + case RESET_T_RESTART_LAPD: + bts->nokia.wait_reset = 0; + start_sabm_in_line(line, 0, -1); /* stop all first */ + start_sabm_in_line(line, 1, SAPI_OML); /* start only OML */ + break; + } } /* TODO: put in a separate file ? */ @@ -1574,25 +1590,16 @@ static int abis_nm_rcvmsg_fom(struct msgb *mb) (function handle_ts1_read()) and ignoring the received data. It seems to be necessary for the MetroSite too. */ - bts->nokia.wait_reset = 1; - - osmo_timer_setup(&bts->nokia.reset_timer, - reset_timer_cb, bts); - osmo_timer_schedule(&bts->nokia.reset_timer, bts->nokia.bts_reset_timer_cnf, 0); - - struct gsm_e1_subslot *e1_link = &bts->oml_e1_link; - struct e1inp_line *line; - /* OML link */ - line = e1inp_line_find(e1_link->e1_nr); - if (!line) { - LOGP(DLINP, LOGL_ERROR, - "BTS %u OML link referring to " - "non-existing E1 line %u\n", bts->nr, - e1_link->e1_nr); - return -ENOMEM; - } - start_sabm_in_line(line, 0, -1); /* stop all first */ + /* we cannot delete / stop the OML LAPD SAP right here, as we are in + * the middle of processing an LAPD I frame and are subsequently returning + * back to the LAPD I frame processing code that assumes the SAP is still + * active. So we first schedule the timer at 0ms in the future, where we + * kill all LAPD SAP and re-arm the timer for the reset duration, after which + * we re-create them */ + bts->nokia.wait_reset = RESET_T_STOP_LAPD; + osmo_timer_setup(&bts->nokia.reset_timer, reset_timer_cb, bts); + osmo_timer_schedule(&bts->nokia.reset_timer, 0, 0); } /* ACK for CONF DATA message ? */ |