fix use-after-free in ipaccess_bts_keepalive_fsm_alloc()2023q1

In ipaccess_bts_keepalive_fsm_alloc() we allocate a keepalive FSM
instance as a child of the respective struct ipa_client_conn, and
store the pointer to the respective struct e1inp_ts.

  + struct e1inp_line
  |
  ---+ struct ipaccess_line (void *driver_data)
  |  |
  |  ---+ struct ipa_client_conn *ipa_cli[NUM_E1_TS]  // <-- parent
  |
  ---+ struct e1inp_ts ts[NUM_E1_TS]
  |  |
  |  ---+ .driver.ipaccess.ka_fsm  // <-- pointer

When an ipaccess connection (be it OML or RSL) goes down and then
up again, for instance if the BSC gets restarted, osmo-bts crashes.
The problem is that struct ipa_client_conn gets free()ed before the
associated FSM instance gets terminated:

* e1inp_ipa_bts_rsl_connect_n() is called
** calling e1inp_ipa_bts_rsl_close_n()
*** this function free()s struct ipa_client_conn
*** (!) as well as the struct osmo_fsm_inst (talloc child)
** calling ipaccess_bts_keepalive_fsm_alloc()
*** calling ipaccess_keepalive_fsm_cleanup()
**** accessing free()d e1i_ts->driver.ipaccess.ka_fsm
**** BOOOM!  segmentation fault

Fix this by calling ipaccess_keepalive_fsm_cleanup() before free()ing
the associated struct ipa_client_conn.

Note that ipaccess_bsc_keepalive_fsm_alloc() is not affected because
it's allocating keepalive FSMs using the global tall_ipa_ctx.

Change-Id: Ic56c4b5b7b24b63104908a0c24f2f645ba4c5c1b
Related: SYS#6438
(cherry picked from commit f6bde0f521155f1d2a073181cfca97df83de2684)

1 files changed, 5 insertions, 1 deletions

diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c
index 8a52591..418606e 100644
--- a/src/input/ipaccess.c
+++ b/src/input/ipaccess.c
@@ -192,7 +192,7 @@ static void ipaccess_bts_keepalive_fsm_alloc(struct e1inp_ts *e1i_ts, struct ipa
 	struct e1inp_line *line = e1i_ts->line;
 	struct osmo_fsm_inst *ka_fsm;
 
-	ipaccess_keepalive_fsm_cleanup(e1i_ts);
+	OSMO_ASSERT(e1i_ts->driver.ipaccess.ka_fsm == NULL);
 	if (!line->ipa_kap)
 		return;
 
@@ -1207,6 +1207,7 @@ int e1inp_ipa_bts_rsl_connect_n(struct e1inp_line *line,
 int e1inp_ipa_bts_rsl_close_n(struct e1inp_line *line, uint8_t trx_nr)
 {
 	struct ipaccess_line *il;
+	struct e1inp_ts *e1i_ts;
 
 	if (E1INP_SIGN_RSL+trx_nr-1 >= NUM_E1_TS) {
 		LOGP(DLINP, LOGL_ERROR,
@@ -1218,6 +1219,9 @@ int e1inp_ipa_bts_rsl_close_n(struct e1inp_line *line, uint8_t trx_nr)
 	if (!il)
 		return 0; /* Nothing to do, no lines created */
 
+	e1i_ts = e1inp_line_ipa_rsl_ts(line, trx_nr);
+	ipaccess_keepalive_fsm_cleanup(e1i_ts);
+
 	if (il->ipa_cli[1 + trx_nr]) {
 		ipa_client_conn_close(il->ipa_cli[1 + trx_nr]);
 		ipa_client_conn_destroy(il->ipa_cli[1 + trx_nr]);