lynxis lazus has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/43474?usp=email )

Change subject: libvlr: auth_fsm: split off fsm creation and start of it ......................................................................

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);