Skip to content

Commit 0ed8355

Browse files
spencersevillaSpencer Sevilla
spencersevilla
authored and
Spencer Sevilla
committed
[sgwc][smf] check if session is active before dereferencing (#66)
1 parent 6aa43c3 commit 0ed8355

13 files changed

+143
-62
lines changed

‎src/sgwc/context.c

+8
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,11 @@ sgwc_sess_t *sgwc_sess_add(sgwc_ue_t *sgwc_ue, char *apn)
279279
(long long)ogs_app()->pool.sess);
280280
return NULL;
281281
}
282+
282283
memset(sess, 0, sizeof *sess);
283284

285+
sess->active = true;
286+
284287
ogs_pfcp_pool_init(&sess->pfcp);
285288

286289
sess->index = ogs_pool_index(&sgwc_sess_pool, sess);
@@ -402,6 +405,11 @@ int sgwc_sess_remove(sgwc_sess_t *sess)
402405
sgwc_ue = sess->sgwc_ue;
403406
ogs_assert(sgwc_ue);
404407

408+
if (!sess->active) {
409+
ogs_error("sgwc_sess_remove double-free");
410+
}
411+
sess->active = false;
412+
405413
ogs_list_remove(&sgwc_ue->sess_list, sess);
406414

407415
sgwc_bearer_remove_all(sess);

‎src/sgwc/context.h

+3
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ typedef struct sgwc_sess_s {
9292
ogs_pfcp_node_t *pfcp_node;
9393

9494
sgwc_ue_t *sgwc_ue;
95+
96+
bool active;
97+
9598
} sgwc_sess_t;
9699

97100
typedef struct sgwc_bearer_s {

‎src/sgwc/s11-handler.c

+17-10
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ void sgwc_s11_handle_modify_bearer_request(
494494
}
495495

496496
sess = bearer->sess;
497-
ogs_assert(sess);
497+
ogs_assert(sess && sess->active);
498498

499499
ogs_list_for_each_entry(&pfcp_xact_list, pfcp_xact, tmpnode) {
500500
if (sess == pfcp_xact->data) {
@@ -631,7 +631,7 @@ void sgwc_s11_handle_delete_session_request(
631631

632632
if (cause_value == OGS_GTP2_CAUSE_REQUEST_ACCEPTED) {
633633
sess = sgwc_sess_find_by_ebi(sgwc_ue, req->linked_eps_bearer_id.u8);
634-
if (!sess) {
634+
if (!sess || !sess->active) {
635635
ogs_error("Unknown EPS Bearer [IMSI:%s, EBI:%d]",
636636
sgwc_ue->imsi_bcd, req->linked_eps_bearer_id.u8);
637637
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
@@ -671,7 +671,7 @@ void sgwc_s11_handle_delete_session_request(
671671
* Check ALL Context
672672
********************/
673673
ogs_assert(sgwc_ue);
674-
ogs_assert(sess);
674+
ogs_assert(sess && sess->active);
675675
ogs_assert(sess->gnode);
676676
ogs_debug(" MME_S11_TEID[%d] SGW_S11_TEID[%d]",
677677
sgwc_ue->mme_s11_teid, sgwc_ue->sgw_s11_teid);
@@ -744,7 +744,7 @@ void sgwc_s11_handle_create_bearer_response(
744744

745745
ogs_assert(bearer);
746746
sess = bearer->sess;
747-
ogs_assert(sess);
747+
ogs_assert(sess && sess->active);
748748

749749
rv = ogs_gtp_xact_commit(s11_xact);
750750
ogs_expect(rv == OGS_OK);
@@ -833,7 +833,7 @@ void sgwc_s11_handle_create_bearer_response(
833833
* Check ALL Context
834834
********************/
835835
ogs_assert(sgwc_ue);
836-
ogs_assert(sess);
836+
ogs_assert(sess && sess->active);
837837
ogs_assert(bearer);
838838

839839
/* Correlate with SGW-S1U-TEID */
@@ -928,7 +928,6 @@ void sgwc_s11_handle_update_bearer_response(
928928

929929
ogs_assert(bearer);
930930
sess = bearer->sess;
931-
ogs_assert(sess);
932931

933932
rv = ogs_gtp_xact_commit(s11_xact);
934933
ogs_expect(rv == OGS_OK);
@@ -943,6 +942,11 @@ void sgwc_s11_handle_update_bearer_response(
943942
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
944943
}
945944

945+
if (!sess || !sess->active) {
946+
ogs_error("No Sess Context");
947+
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
948+
}
949+
946950
if (cause_value != OGS_GTP2_CAUSE_REQUEST_ACCEPTED) {
947951
ogs_gtp_send_error_message(s5c_xact, sess ? sess->pgw_s5c_teid : 0,
948952
OGS_GTP2_UPDATE_BEARER_RESPONSE_TYPE, cause_value);
@@ -1007,7 +1011,7 @@ void sgwc_s11_handle_update_bearer_response(
10071011
* Check ALL Context
10081012
********************/
10091013
ogs_assert(sgwc_ue);
1010-
ogs_assert(sess);
1014+
ogs_assert(sess && sess->active);
10111015

10121016
ogs_debug(" MME_S11_TEID[%d] SGW_S11_TEID[%d]",
10131017
sgwc_ue->mme_s11_teid, sgwc_ue->sgw_s11_teid);
@@ -1060,7 +1064,7 @@ void sgwc_s11_handle_delete_bearer_response(
10601064

10611065
ogs_assert(bearer);
10621066
sess = bearer->sess;
1063-
ogs_assert(sess);
1067+
ogs_assert(sess && sess->active);
10641068

10651069
rv = ogs_gtp_xact_commit(s11_xact);
10661070
ogs_expect(rv == OGS_OK);
@@ -1232,14 +1236,17 @@ void sgwc_s11_handle_downlink_data_notification_ack(
12321236
bearer = s11_xact->data;
12331237
ogs_assert(bearer);
12341238
sess = bearer->sess;
1235-
ogs_assert(sess);
12361239

12371240
rv = ogs_gtp_xact_commit(s11_xact);
12381241
ogs_expect(rv == OGS_OK);
12391242

12401243
/************************
12411244
* Check SGWC-UE Context
12421245
************************/
1246+
if (!sess || !sess->active) {
1247+
ogs_error("No Sess Context");
1248+
}
1249+
12431250
if (ack->cause.presence) {
12441251
ogs_gtp2_cause_t *cause = ack->cause.data;
12451252
ogs_assert(cause);
@@ -1531,7 +1538,7 @@ void sgwc_s11_handle_bearer_resource_command(
15311538
********************/
15321539
ogs_assert(bearer);
15331540
sess = bearer->sess;
1534-
ogs_assert(sess);
1541+
ogs_assert(sess && sess->active);
15351542
ogs_assert(sess->gnode);
15361543
ogs_assert(sgwc_ue);
15371544

‎src/sgwc/s5c-handler.c

+7-7
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ void sgwc_s5c_handle_create_session_response(
100100
************************/
101101
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
102102

103-
if (!sess) {
103+
if (!sess || !sess->active) {
104104
ogs_error("No Context in TEID");
105105
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
106106
} else {
@@ -320,7 +320,7 @@ void sgwc_s5c_handle_delete_session_response(
320320
************************/
321321
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
322322

323-
if (!sess) {
323+
if (!sess || !sess->active) {
324324
ogs_error("No Context in TEID");
325325
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
326326
} else {
@@ -433,7 +433,7 @@ void sgwc_s5c_handle_modify_bearer_response(
433433
************************/
434434
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
435435

436-
if (!sess) {
436+
if (!sess || !sess->active) {
437437
ogs_error("No Context in TEID");
438438
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
439439
} else {
@@ -558,7 +558,7 @@ void sgwc_s5c_handle_create_bearer_request(
558558
************************/
559559
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
560560

561-
if (!sess) {
561+
if (!sess || !sess->active) {
562562
ogs_error("No Context in TEID");
563563
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
564564
} else {
@@ -677,7 +677,7 @@ void sgwc_s5c_handle_update_bearer_request(
677677
************************/
678678
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
679679

680-
if (!sess) {
680+
if (!sess || !sess->active) {
681681
ogs_error("No Context in TEID");
682682
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
683683
} else {
@@ -778,7 +778,7 @@ void sgwc_s5c_handle_delete_bearer_request(
778778
************************/
779779
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
780780

781-
if (!sess) {
781+
if (!sess || !sess->active) {
782782
ogs_error("No Context in TEID");
783783
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
784784
} else {
@@ -925,7 +925,7 @@ void sgwc_s5c_handle_bearer_resource_failure_indication(
925925
/************************
926926
* Check Session Context
927927
************************/
928-
if (!sess) {
928+
if (!sess || !sess->active) {
929929
ogs_error("No Context in TEID");
930930
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
931931
} else {

‎src/sgwc/sxa-handler.c

+30-19
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ static void sgwc_sxa_handle_session_reestablishment(
128128
sgwc_sess_t *sess, ogs_pfcp_xact_t *pfcp_xact,
129129
ogs_pfcp_session_establishment_response_t *pfcp_rsp)
130130
{
131-
ogs_assert(sess);
131+
ogs_assert(sess && sess->active);
132132
ogs_assert(pfcp_xact);
133133
ogs_assert(pfcp_rsp);
134134

@@ -195,7 +195,7 @@ void sgwc_sxa_handle_session_establishment_response(
195195

196196
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
197197

198-
if (!sess) {
198+
if (!sess || !sess->active) {
199199
ogs_warn("No Context");
200200
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
201201
}
@@ -275,7 +275,7 @@ void sgwc_sxa_handle_session_establishment_response(
275275
return;
276276
}
277277

278-
ogs_assert(sess);
278+
ogs_assert(sess && sess->active);
279279

280280
ogs_debug(" SGW_S5C_TEID[0x%x] PGW_S5C_TEID[0x%x]",
281281
sess->sgw_s5c_teid, sess->pgw_s5c_teid);
@@ -485,13 +485,19 @@ void sgwc_sxa_handle_session_modification_response(
485485
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
486486

487487
if (flags & OGS_PFCP_MODIFY_SESSION) {
488-
if (!sess) {
489-
ogs_warn("No Context");
490-
488+
if (!sess || !sess->active) {
489+
// sess pointer was expired; can we recover it from xact?
491490
sess = pfcp_xact->data;
492-
ogs_assert(sess);
493491

494-
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
492+
if (!sess || !sess->active) {
493+
ogs_error("No Context");
494+
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
495+
// send gtp error?
496+
ogs_pfcp_xact_commit(pfcp_xact);
497+
return;
498+
} else {
499+
ogs_warn("sess expired but pfcp_xact->sess active");
500+
}
495501
}
496502
sgwc_ue = sess->sgwc_ue;
497503
ogs_assert(sgwc_ue);
@@ -500,15 +506,20 @@ void sgwc_sxa_handle_session_modification_response(
500506
bearer = pfcp_xact->data;
501507
ogs_assert(bearer);
502508

503-
if (!sess) {
504-
ogs_warn("No Context");
505-
509+
if (!sess || !sess->active) {
510+
// sess pointer was expired; can we recover it from bearer?
506511
sess = bearer->sess;
507-
ogs_assert(sess);
508512

509-
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
513+
if (!sess || !sess->active) {
514+
ogs_error("No Context");
515+
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
516+
// send gtp error?
517+
ogs_pfcp_xact_commit(pfcp_xact);
518+
return;
519+
} else {
520+
ogs_warn("sess expired but bearer->sess active");
521+
}
510522
}
511-
512523
sgwc_ue = bearer->sgwc_ue;
513524
ogs_assert(sgwc_ue);
514525
}
@@ -533,7 +544,7 @@ void sgwc_sxa_handle_session_modification_response(
533544

534545
OGS_LIST(pdr_to_create_list);
535546

536-
ogs_assert(sess);
547+
ogs_assert(sess && sess->active);
537548

538549
ogs_list_copy(&pdr_to_create_list, &pfcp_xact->pdr_to_create_list);
539550

@@ -1232,7 +1243,7 @@ void sgwc_sxa_handle_session_deletion_response(
12321243

12331244
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
12341245

1235-
if (!sess) {
1246+
if (!sess || !sess->active) {
12361247
ogs_warn("No Context");
12371248
cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
12381249
}
@@ -1307,7 +1318,7 @@ void sgwc_sxa_handle_session_deletion_response(
13071318
return;
13081319
}
13091320

1310-
ogs_assert(sess);
1321+
ogs_assert(sess && sess->active);
13111322
sgwc_ue = sess->sgwc_ue;
13121323
ogs_assert(sgwc_ue);
13131324

@@ -1368,7 +1379,7 @@ void sgwc_sxa_handle_session_report_request(
13681379

13691380
cause_value = OGS_GTP2_CAUSE_REQUEST_ACCEPTED;
13701381

1371-
if (!sess) {
1382+
if (!sess || !sess->active) {
13721383
ogs_warn("No Context");
13731384
cause_value = OGS_PFCP_CAUSE_SESSION_CONTEXT_NOT_FOUND;
13741385
}
@@ -1385,7 +1396,7 @@ void sgwc_sxa_handle_session_report_request(
13851396
return;
13861397
}
13871398

1388-
ogs_assert(sess);
1399+
ogs_assert(sess && sess->active);
13891400
sgwc_ue = sess->sgwc_ue;
13901401
ogs_assert(sgwc_ue);
13911402

‎src/smf/context.c

+7
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,7 @@ smf_sess_t *smf_sess_add_by_apn(smf_ue_t *smf_ue, char *apn, uint8_t rat_type)
10681068
ogs_fsm_create(&sess->sm, smf_gsm_state_initial, smf_gsm_state_final);
10691069
ogs_fsm_init(&sess->sm, &e);
10701070

1071+
sess->active = true;
10711072
sess->smf_ue = smf_ue;
10721073

10731074
ogs_list_add(&smf_ue->sess_list, sess);
@@ -1276,6 +1277,7 @@ smf_sess_t *smf_sess_add_by_psi(smf_ue_t *smf_ue, uint8_t psi)
12761277
ogs_fsm_create(&sess->sm, smf_gsm_state_initial, smf_gsm_state_final);
12771278
ogs_fsm_init(&sess->sm, &e);
12781279

1280+
sess->active = true;
12791281
sess->smf_ue = smf_ue;
12801282

12811283
ogs_list_add(&smf_ue->sess_list, sess);
@@ -1553,6 +1555,11 @@ void smf_sess_remove(smf_sess_t *sess)
15531555

15541556
ogs_list_remove(&smf_ue->sess_list, sess);
15551557

1558+
if (!sess->active) {
1559+
ogs_error("smf_sess_remove double-free");
1560+
}
1561+
sess->active = false;
1562+
15561563
memset(&e, 0, sizeof(e));
15571564
e.sess = sess;
15581565
ogs_fsm_fini(&sess->sm, &e);

‎src/smf/context.h

+1
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ typedef struct smf_sess_s {
416416
bool teardown_gtp;
417417
bool pfcp_established;
418418
ogs_pfcp_xact_t *timeout_xact;
419+
bool active;
419420

420421
} smf_sess_t;
421422

‎src/smf/gn-handler.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ void smf_gn_handle_update_pdp_context_request(
330330
cause_value = OGS_GTP1_CAUSE_MANDATORY_IE_MISSING;
331331
}
332332

333-
if (!sess) {
333+
if (!sess || !sess->active) {
334334
ogs_warn("No Context");
335335
cause_value = OGS_GTP1_CAUSE_NON_EXISTENT;
336336
}

0 commit comments

Comments
 (0)