From 456a62e98e5e0ac773244191cba092bf58a91988 Mon Sep 17 00:00:00 2001 From: Harald Welte Date: Sun, 3 May 2020 12:00:52 +0200 Subject: 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 --- src/osmo-bsc/bts_nokia_site.c | 55 ++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 24 deletions(-) (limited to 'src/osmo-bsc/bts_nokia_site.c') 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 +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 ? */ -- cgit v1.2.3