lynxis lazus submitted this change.

View Change

libvlr: auth_fsm: split off fsm creation and start of it

The auth fsm may destroy itself from a chain of fsm events by
VLR_AUTH_E_START. Resulting in an use-after-free of the fsm instance.

Change-Id: Ibad920676d9d4a8b7de4a820196385738764ffe9
---
M src/libvlr/vlr_access_req_fsm.c
M src/libvlr/vlr_auth_fsm.c
M src/libvlr/vlr_auth_fsm.h
M src/libvlr/vlr_lu_fsm.c
4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 5d2b7f9..f68e5fc 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -363,12 +363,13 @@
 	if (is_auth_to_be_attempted(par)) {
 		osmo_fsm_inst_state_chg(fi, PR_ARQ_S_WAIT_AUTH,
 					0, 0);
-		vsub->auth_fsm = auth_fsm_start(vsub, fi,
+		vsub->auth_fsm = auth_fsm_create(vsub, fi,
 						PR_ARQ_E_AUTH_RES,
 						PR_ARQ_E_AUTH_NO_INFO,
 						PR_ARQ_E_AUTH_FAILURE,
 						par->is_r99,
 						par->is_utran);
+		auth_fsm_start(vsub->auth_fsm);
 	} else {
 		_proc_arq_vlr_node2(fi);
 	}
diff --git a/src/libvlr/vlr_auth_fsm.c b/src/libvlr/vlr_auth_fsm.c
index 192e390..7345037 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -642,7 +642,7 @@
  ***********************************************************************/
 
 /* MSC->VLR: Start Procedure Authenticate_VLR (TS 23.012 Ch. 4.1.2.2) */
-struct osmo_fsm_inst *auth_fsm_start(struct vlr_subscr *vsub,
+struct osmo_fsm_inst *auth_fsm_create(struct vlr_subscr *vsub,
 				     struct osmo_fsm_inst *parent,
 				     uint32_t parent_event_success,
 				     uint32_t parent_event_no_auth_info,
@@ -676,11 +676,17 @@
 	fi->priv = afp;
 	vsub->auth_fsm = fi;
 
-	osmo_fsm_inst_dispatch(fi, VLR_AUTH_E_START, NULL);
-
 	return fi;
 }
 
+int auth_fsm_start(struct osmo_fsm_inst *fi)
+{
+	if (fi)
+		return osmo_fsm_inst_dispatch(fi, VLR_AUTH_E_START, NULL);
+
+	return -ENOENT;
+}
+
 bool auth_ciph_sec_ctx_is_usable(const struct vlr_subscr *vsub, uint8_t key_seq)
 {
 	int max_reuse_count = vsub->vlr->cfg.ciph_sec_ctx_max_reuse;
diff --git a/src/libvlr/vlr_auth_fsm.h b/src/libvlr/vlr_auth_fsm.h
index 7a45a1a..078960b 100644
--- a/src/libvlr/vlr_auth_fsm.h
+++ b/src/libvlr/vlr_auth_fsm.h
@@ -29,7 +29,7 @@
 	VLR_AUTH_E_MS_ID_IMSI,
 };
 
-struct osmo_fsm_inst *auth_fsm_start(struct vlr_subscr *vsub,
+struct osmo_fsm_inst *auth_fsm_create(struct vlr_subscr *vsub,
 				     struct osmo_fsm_inst *parent,
 				     uint32_t parent_event_success,
 				     uint32_t parent_event_no_auth_info,
@@ -37,6 +37,8 @@
 				     bool is_r99,
 				     bool is_utran);
 
+int auth_fsm_start(struct osmo_fsm_inst *fi);
+
 void vlr_auth_fsm_init(bool is_ps);
 void vlr_auth_fsm_set_log_subsys(int log_level);
 bool auth_try_reuse_tuple(struct vlr_subscr *vsub, uint8_t key_seq);
diff --git a/src/libvlr/vlr_lu_fsm.c b/src/libvlr/vlr_lu_fsm.c
index d3aaf3a..e97f19d 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -991,17 +991,22 @@
 
 	OSMO_ASSERT(vsub);
 
+	/* check if we need to do an authentication, if only ciphering, check if ciphering key is available */
 	if (try_auth(lfp)) {
 		/* Authenticate_VLR */
 		osmo_fsm_inst_state_chg(fi, VLR_ULA_S_WAIT_AUTH,
 					LU_TIMEOUT_LONG, 0);
-		vsub->auth_fsm = auth_fsm_start(lfp->vsub,
+		/* First only create and assign vsub->auth_fsm, because an auth_fsm_start() might run into a failure
+		 * and the fsm will be free'ed.
+		 */
+		vsub->auth_fsm = auth_fsm_create(lfp->vsub,
 						fi,
 						VLR_ULA_E_AUTH_SUCCESS,
 						VLR_ULA_E_AUTH_NO_INFO,
 						VLR_ULA_E_AUTH_FAILURE,
 						lfp->is_r99,
 						lfp->is_utran);
+		auth_fsm_start(vsub->auth_fsm);
 	} else {
 		/* no need for authentication */
 		vlr_loc_upd_post_auth(fi);

To view, visit change 43510. To unsubscribe, or for help writing mail filters, visit settings.