lynxis lazus has uploaded this change for review.

View Change

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

The auth fsm may destroy itself from a chain of fsm event resulting
in a use-after-free of the fsm instance.

Change-Id: Id5c77290f7181adb40d3429183b9696b4a40013b
---
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, 19 insertions(+), 6 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/74/43474/1

diff --git a/src/libvlr/vlr_access_req_fsm.c b/src/libvlr/vlr_access_req_fsm.c
index 6d4a084..e57cd50 100644
--- a/src/libvlr/vlr_access_req_fsm.c
+++ b/src/libvlr/vlr_access_req_fsm.c
@@ -358,12 +358,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 68c9a54..7557971 100644
--- a/src/libvlr/vlr_auth_fsm.c
+++ b/src/libvlr/vlr_auth_fsm.c
@@ -641,7 +641,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,
@@ -675,11 +675,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_try_reuse_tuple(struct vlr_subscr *vsub, uint8_t key_seq)
 {
 	int max_reuse_count = vsub->vlr->cfg.auth_tuple_max_reuse_count;
diff --git a/src/libvlr/vlr_auth_fsm.h b/src/libvlr/vlr_auth_fsm.h
index 1cb25b6..ca462a3 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 a3f553f..e2b714b 100644
--- a/src/libvlr/vlr_lu_fsm.c
+++ b/src/libvlr/vlr_lu_fsm.c
@@ -986,17 +986,21 @@
 
 	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,
+		/* if auth_fsm_start fails and the fsm already ran into a failure of auth_fsm,.
+		 * auth_fsm is assigned an already free'ed fsm */
+		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 43474. To unsubscribe, or for help writing mail filters, visit settings.