From 2b6369e9d997eb8eb158e3803e3ae4f4d207cd4d Mon Sep 17 00:00:00 2001 From: Sukchan Lee Date: Sat, 13 Apr 2024 15:01:32 +0900 Subject: [PATCH] [SMF] crash when malformed NAS message (#3132) A malformed PDU Session Modification Request is sent from UE after Registration Complete. ``` Crash 1: 04/12 15:00:44.031: [amf] INFO: [imsi-999700000000001:1:11][0:0:NULL] /nsmf-pdusession/v1/sm-contexts/{smContextRef}/modify (../src/amf/nsmf-handler.c:837) 04/12 15:00:46.569: [nas] FATAL: ogs_nas_parse_qos_flow_descriptions: Assertion `descriptions->length' failed. (../lib/nas/5gs/types.c:486) 04/12 15:00:46.569: [core] FATAL: backtrace() returned 11 addresses (../lib/core/ogs-abort.c:37) ../src/smf/../../lib/nas/5gs/libogsnas-5gs.so.2(ogs_nas_parse_qos_flow_descriptions+0x162) [0x7e6e7a5a4e5d] ../src/smf/open5gs-smfd(+0x8c6ec) [0x5dd6c333d6ec] ../src/smf/open5gs-smfd(+0x2d69b) [0x5dd6c32de69b] ../src/smf/../../lib/core/libogscore.so.2(ogs_fsm_dispatch+0x119) [0x7e6e7b216c0c] ../src/smf/open5gs-smfd(+0x288b3) [0x5dd6c32d98b3] ../src/smf/../../lib/core/libogscore.so.2(ogs_fsm_dispatch+0x119) [0x7e6e7b216c0c] ../src/smf/open5gs-smfd(+0xf2d8) [0x5dd6c32c02d8] ../src/smf/../../lib/core/libogscore.so.2(+0x1197a) [0x7e6e7b20797a] /lib/x86_64-linux-gnu/libc.so.6(+0x94ac3) [0x7e6e7a094ac3] /lib/x86_64-linux-gnu/libc.so.6(+0x126850) [0x7e6e7a126850] 04/12 15:00:46.613: [app] ERROR: Signal-NUM[17] received (Child status change) (../src/main.c:81) 04/12 15:00:46.613: [sbi] WARNING: [92] HTTP/2 stream 19 was not closed cleanly before end of the underlying stream (../lib/sbi/client.c:626) 04/12 15:00:46.613: [scp] WARNING: response_handler() failed [-1] (../src/scp/sbi-path.c:539) 04/12 15:00:46.613: [amf] ERROR: [1:0] No SmContextUpdateError [500] (../src/amf/nsmf-handler.c:866) 04/12 15:00:46.613: [amf] ERROR: AMF_SESS_CLEAR (../src/amf/amf-sm.c:484) 04/12 15:00:46.613: [amf] INFO: [Removed] Number of AMF-Sessions is now 0 (../src/amf/context.c:2551) 04/12 15:00:50.596: [nrf] WARNING: [c466ec64-f8fe-41ee-a888-194dc4363612] No heartbeat (../src/nrf/nrf-sm.c:260) 04/12 15:00:50.596: [nrf] INFO: [c466ec64-f8fe-41ee-a888-194dc4363612] NF de-registered (../src/nrf/nf-sm.c:205) 04/12 15:00:50.596: [sbi] INFO: [c466ec64-f8fe-41ee-a888-194dc4363612:1] NF removed (../lib/sbi/nnrf-handler.c:750) 04/12 15:00:50.596: [sbi] INFO: [c466ec64-f8fe-41ee-a888-194dc4363612:1] NF removed (../lib/sbi/nnrf-handler.c:750) 04/12 15:00:55.094: [pfcp] WARNING: [10] LOCAL No Reponse. Give up! for step 1 type 1 peer [127.0.0.4]:8805 (../lib/pfcp/xact.c:599) 04/12 15:00:55.094: [upf] WARNING: No Heartbeat from SMF [127.0.0.4]:8805 (../src/upf/pfcp-sm.c:329) 04/12 15:00:55.094: [upf] INFO: PFCP de-associated [127.0.0.4]:8805 (../src/upf/pfcp-sm.c:199) 04/12 15:01:02.599: [pfcp] WARNING: [11] LOCAL No Reponse. Give up! for step 1 type 5 peer [127.0.0.4]:8805 (../lib/pfcp/xact.c:599) 04/12 15:01:06.098: [upf] WARNING: Retry to association with peer [127.0.0.4]:8805 failed (../src/upf/pfcp-sm.c:107) Crash 2: 04/12 15:16:39.748: [amf] INFO: [imsi-999700000000001:1:11][0:0:NULL] /nsmf-pdusession/v1/sm-contexts/{smContextRef}/modify (../src/amf/nsmf-handler.c:837) 04/12 15:16:42.155: [nas] FATAL: ogs_nas_parse_qos_rules: Assertion `size+sizeof(rule->flow.flags) <= length' failed. (../lib/nas/5gs/types.c:961) 04/12 15:16:42.155: [core] FATAL: backtrace() returned 11 addresses (../lib/core/ogs-abort.c:37) ../src/smf/../../lib/nas/5gs/libogsnas-5gs.so.2(ogs_nas_parse_qos_rules+0x12d1) [0x7d1affbd2d72] ../src/smf/open5gs-smfd(+0x8b446) [0x629a57861446] ../src/smf/open5gs-smfd(+0x2d69b) [0x629a5780369b] ../src/smf/../../lib/core/libogscore.so.2(ogs_fsm_dispatch+0x119) [0x7d1affd05c0c] ../src/smf/open5gs-smfd(+0x288b3) [0x629a577fe8b3] ../src/smf/../../lib/core/libogscore.so.2(ogs_fsm_dispatch+0x119) [0x7d1affd05c0c] ../src/smf/open5gs-smfd(+0xf2d8) [0x629a577e52d8] ../src/smf/../../lib/core/libogscore.so.2(+0x1197a) [0x7d1affcf697a] /lib/x86_64-linux-gnu/libc.so.6(+0x94ac3) [0x7d1afea94ac3] /lib/x86_64-linux-gnu/libc.so.6(+0x126850) [0x7d1afeb26850] 04/12 15:16:42.199: [sbi] WARNING: [92] HTTP/2 stream 13 was not closed cleanly before end of the underlying stream (../lib/sbi/client.c:626) 04/12 15:16:42.199: [scp] WARNING: response_handler() failed [-1] (../src/scp/sbi-path.c:539) 04/12 15:16:42.199: [app] ERROR: Signal-NUM[17] received (Child status change) (../src/main.c:81) 04/12 15:16:42.200: [amf] ERROR: [1:0] No SmContextUpdateError [500] (../src/amf/nsmf-handler.c:866) 04/12 15:16:42.200: [amf] ERROR: AMF_SESS_CLEAR (../src/amf/amf-sm.c:484) 04/12 15:16:42.200: [amf] INFO: [Removed] Number of AMF-Sessions is now 0 (../src/amf/context.c:2551) 04/12 15:16:49.858: [nrf] WARNING: [23f1aee2-f901-41ee-a488-85a58e1e3420] No heartbeat (../src/nrf/nrf-sm.c:260) 04/12 15:16:49.858: [nrf] INFO: [23f1aee2-f901-41ee-a488-85a58e1e3420] NF de-registered (../src/nrf/nf-sm.c:205) 04/12 15:16:49.859: [sbi] INFO: [23f1aee2-f901-41ee-a488-85a58e1e3420:1] NF removed (../lib/sbi/nnrf-handler.c:750) 04/12 15:16:49.859: [sbi] INFO: [23f1aee2-f901-41ee-a488-85a58e1e3420:1] NF removed (../lib/sbi/nnrf-handler.c:750) 04/12 15:16:59.364: [pfcp] WARNING: [5] LOCAL No Reponse. Give up! for step 1 type 1 peer [127.0.0.4]:8805 (../lib/pfcp/xact.c:599) 04/12 15:16:59.364: [upf] WARNING: No Heartbeat from SMF [127.0.0.4]:8805 (../src/upf/pfcp-sm.c:329) 04/12 15:16:59.364: [upf] INFO: PFCP de-associated [127.0.0.4]:8805 (../src/upf/pfcp-sm.c:199) ``` So, I've fixed it. --- lib/nas/5gs/types.c | 220 ++++++++++++++++++++++++++++++++---------- src/smf/gsm-handler.c | 34 ++++--- 2 files changed, 189 insertions(+), 65 deletions(-) diff --git a/lib/nas/5gs/types.c b/lib/nas/5gs/types.c index ce0d65f512..cf576a64c9 100644 --- a/lib/nas/5gs/types.c +++ b/lib/nas/5gs/types.c @@ -483,35 +483,59 @@ int ogs_nas_parse_qos_flow_descriptions( ogs_assert(description); ogs_assert(descriptions); - ogs_assert(descriptions->length); + + if (descriptions->length == 0) { + ogs_error("Length is 0"); + goto cleanup; + } + if (descriptions->buffer == NULL) { + ogs_error("Buffer is NULL"); + goto cleanup; + } + length = descriptions->length; - ogs_assert(descriptions->buffer); buffer = descriptions->buffer; size = 0; while (size < length) { memset(description, 0, sizeof(*description)); - ogs_assert(size+3 <= length); + if (size+3 > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(description, buffer+size, 3); size += 3; for (i = 0; i < description->num_of_parameter && i < OGS_NAS_MAX_NUM_OF_QOS_FLOW_PARAMETER; i++) { - ogs_assert(size+sizeof(description->param[i].identifier) <= length); + if (size+sizeof(description->param[i].identifier) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&description->param[i].identifier, buffer+size, sizeof(description->param[i].identifier)); size += sizeof(description->param[i].identifier); - ogs_assert(size+sizeof(description->param[i].len) <= length); + if (size+sizeof(description->param[i].len) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&description->param[i].len, buffer+size, sizeof(description->param[i].len)); size += sizeof(description->param[i].len); switch(description->param[i].identifier) { case OGS_NAX_QOS_FLOW_PARAMETER_ID_5QI: - ogs_assert(description->param[i].len == 1); - ogs_assert(size+description->param[i].len <= length); + if (description->param[i].len != 1) { + ogs_error("Invalid len[%d]", description->param[i].len); + goto cleanup; + } + if (size+description->param[i].len > length) { + ogs_error("Overflow: len[%d] length[%d]", + description->param[i].len, length); + goto cleanup; + } memcpy(&description->param[i].qos_index, buffer+size, description->param[i].len); size += description->param[i].len; @@ -521,8 +545,15 @@ int ogs_nas_parse_qos_flow_descriptions( case OGS_NAX_QOS_FLOW_PARAMETER_ID_GFBR_DOWNLINK: case OGS_NAX_QOS_FLOW_PARAMETER_ID_MFBR_UPLINK: case OGS_NAX_QOS_FLOW_PARAMETER_ID_MFBR_DOWNLINK: - ogs_assert(description->param[i].len == 3); - ogs_assert(size+description->param[i].len <= length); + if (description->param[i].len != 3) { + ogs_error("Invalid len[%d]", description->param[i].len); + goto cleanup; + } + if (size+description->param[i].len > length) { + ogs_error("Overflow: len[%d] length[%d]", + description->param[i].len, length); + goto cleanup; + } memcpy(&description->param[i].br, buffer+size, description->param[i].len); description->param[i].br.value = @@ -530,15 +561,17 @@ int ogs_nas_parse_qos_flow_descriptions( size += description->param[i].len; break; default: - ogs_fatal("Unknown qos_flow parameter identifier [%d]", + ogs_error("Unknown qos_flow parameter identifier [%d]", description->param[i].identifier); - ogs_assert_if_reached(); + goto cleanup; } } description++; } +cleanup: + return (int)(description-first); } @@ -777,28 +810,50 @@ int ogs_nas_parse_qos_rules( ogs_assert(rule); ogs_assert(rules); - ogs_assert(rules->length); + + if (rules->length == 0) { + ogs_error("Length is 0"); + goto cleanup; + } + if (rules->buffer == NULL) { + ogs_error("Buffer is NULL"); + goto cleanup; + } + length = rules->length; - ogs_assert(rules->buffer); buffer = rules->buffer; size = 0; while (size < length) { memset(rule, 0, sizeof(*rule)); - ogs_assert(size+sizeof(rule->identifier) <= length); + if (size+sizeof(rule->identifier) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->identifier, buffer+size, sizeof(rule->identifier)); size += sizeof(rule->identifier); - ogs_assert(size+sizeof(rule->length) <= length); + if (size+sizeof(rule->length) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->length, buffer+size, sizeof(rule->length)); rule->length = be16toh(rule->length); size += sizeof(rule->length); - ogs_assert(size+sizeof(rule->flags) <= length); + if (size+sizeof(rule->flags) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->flags, buffer+size, sizeof(rule->flags)); size += sizeof(rule->flags); + if (rule->code == 0 || rule->code == 7) { /* Reserved */ + ogs_error("Reserved Rule Code [%d]", rule->code); + goto cleanup; + } + if (rule->code == OGS_NAS_QOS_CODE_DELETE_EXISTING_QOS_RULE || rule->code == OGS_NAS_QOS_CODE_MODIFY_EXISTING_QOS_RULE_WITHOUT_MODIFYING_PACKET_FILTERS) { if (rule->num_of_packet_filter != 0) { @@ -806,12 +861,16 @@ int ogs_nas_parse_qos_rules( "and number of packet filter[%d]", rule->code, rule->num_of_packet_filter); rule->num_of_packet_filter = 0; + goto cleanup; } } for (i = 0; i < rule->num_of_packet_filter && i < OGS_MAX_NUM_OF_FLOW_IN_GTP; i++) { - ogs_assert(size+sizeof(rule->pf[i].flags) <= length); + if (size+sizeof(rule->pf[i].flags) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->pf[i].flags, buffer+size, sizeof(rule->pf[i].flags)); size += sizeof(rule->pf[i].flags); @@ -819,24 +878,35 @@ int ogs_nas_parse_qos_rules( OGS_NAS_QOS_CODE_MODIFY_EXISTING_QOS_RULE_AND_DELETE_PACKET_FILTERS) continue; - ogs_assert(size+sizeof(rule->pf[i].content.length) <= length); + if (size+sizeof(rule->pf[i].content.length) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->pf[i].content.length, buffer+size, sizeof(rule->pf[i].content.length)); size += sizeof(rule->pf[i].content.length); j = 0; len = 0; while(len < rule->pf[i].content.length) { - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].type) <= length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].type) > length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].type, buffer+size+len, sizeof(rule->pf[i].content.component[j].type)); len += sizeof(rule->pf[i].content.component[j].type); switch(rule->pf[i].content.component[j].type) { case OGS_PACKET_FILTER_PROTOCOL_IDENTIFIER_NEXT_HEADER_TYPE: - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].proto) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].proto) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].proto, buffer+size+len, sizeof(rule->pf[i].content.component[j].proto)); @@ -844,17 +914,25 @@ int ogs_nas_parse_qos_rules( break; case OGS_PACKET_FILTER_IPV4_REMOTE_ADDRESS_TYPE: case OGS_PACKET_FILTER_IPV4_LOCAL_ADDRESS_TYPE: - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].ipv4.addr) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].ipv4.addr) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].ipv4.addr, buffer+size+len, sizeof(rule->pf[i].content.component[j].ipv4.addr)); len += sizeof(rule->pf[i].content.component[j].ipv4.addr); - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].ipv4.mask) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].ipv4.mask) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].ipv4.mask, buffer+size+len, sizeof(rule->pf[i].content.component[j].ipv4.mask)); @@ -862,18 +940,26 @@ int ogs_nas_parse_qos_rules( break; case OGS_PACKET_FILTER_IPV6_LOCAL_ADDRESS_PREFIX_LENGTH_TYPE: case OGS_PACKET_FILTER_IPV6_REMOTE_ADDRESS_PREFIX_LENGTH_TYPE: - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].ipv6.addr) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].ipv6.addr) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].ipv6.addr, buffer+size+len, sizeof(rule->pf[i].content.component[j].ipv6.addr)); len += sizeof(rule->pf[i].content.component[j].ipv6.addr); - ogs_assert(size+len+ + if (size+len+ sizeof( - rule->pf[i].content.component[j].ipv6.prefixlen) <= - length); + rule->pf[i].content.component[j].ipv6.prefixlen) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].ipv6.prefixlen, buffer+size+len, sizeof( @@ -883,10 +969,14 @@ int ogs_nas_parse_qos_rules( break; case OGS_PACKET_FILTER_IPV6_LOCAL_ADDRESS_TYPE: case OGS_PACKET_FILTER_IPV6_REMOTE_ADDRESS_TYPE: - ogs_assert(size+len+ + if (size+len+ sizeof( - rule->pf[i].content.component[j].ipv6_mask.addr) <= - length); + rule->pf[i].content.component[j].ipv6_mask.addr) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].ipv6_mask.addr, buffer+size+len, sizeof( @@ -894,10 +984,14 @@ int ogs_nas_parse_qos_rules( len += sizeof( rule->pf[i].content.component[j].ipv6_mask.addr); - ogs_assert(size+len+ + if (size+len+ sizeof( - rule->pf[i].content.component[j].ipv6_mask.mask) <= - length); + rule->pf[i].content.component[j].ipv6_mask.mask) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].ipv6_mask.mask, buffer+size+len, sizeof( @@ -907,9 +1001,13 @@ int ogs_nas_parse_qos_rules( break; case OGS_PACKET_FILTER_SINGLE_LOCAL_PORT_TYPE: case OGS_PACKET_FILTER_SINGLE_REMOTE_PORT_TYPE: - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].port.low) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].port.low) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].port.low, buffer+size+len, sizeof(rule->pf[i].content.component[j].port.low)); @@ -919,9 +1017,13 @@ int ogs_nas_parse_qos_rules( break; case OGS_PACKET_FILTER_LOCAL_PORT_RANGE_TYPE: case OGS_PACKET_FILTER_REMOTE_PORT_RANGE_TYPE: - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].port.low) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].port.low) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].port.low, buffer+size+len, sizeof(rule->pf[i].content.component[j].port.low)); @@ -929,9 +1031,13 @@ int ogs_nas_parse_qos_rules( be16toh(rule->pf[i].content.component[j].port.low); len += sizeof(rule->pf[i].content.component[j].port.low); - ogs_assert(size+len+ - sizeof(rule->pf[i].content.component[j].port.high) <= - length); + if (size+len+ + sizeof(rule->pf[i].content.component[j].port.high) > + length) { + ogs_error("Overflow : size[%d] len[%d] length[%d]", + size, len, length); + goto cleanup; + } memcpy(&rule->pf[i].content.component[j].port.high, buffer+size+len, sizeof(rule->pf[i].content.component[j].port.high)); @@ -942,7 +1048,7 @@ int ogs_nas_parse_qos_rules( default: ogs_error("Unknown Packet Filter Type(%d)", rule->pf[i].content.component[j].type); - return -1; + goto cleanup; } j++; } @@ -954,11 +1060,17 @@ int ogs_nas_parse_qos_rules( rule->code != OGS_NAS_QOS_CODE_MODIFY_EXISTING_QOS_RULE_AND_DELETE_PACKET_FILTERS && rule->code != OGS_NAS_QOS_CODE_MODIFY_EXISTING_QOS_RULE_WITHOUT_MODIFYING_PACKET_FILTERS) { - ogs_assert(size+sizeof(rule->precedence) <= length); + if (size+sizeof(rule->precedence) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->precedence, buffer+size, sizeof(rule->precedence)); size += sizeof(rule->precedence); - ogs_assert(size+sizeof(rule->flow.flags) <= length); + if (size+sizeof(rule->flow.flags) > length) { + ogs_error("Overflow : size[%d] length[%d]", size, length); + goto cleanup; + } memcpy(&rule->flow.flags, buffer+size, sizeof(rule->flow.flags)); size += sizeof(rule->flow.flags); } @@ -966,6 +1078,8 @@ int ogs_nas_parse_qos_rules( rule++; } +cleanup: + return (int)(rule-first); } diff --git a/src/smf/gsm-handler.c b/src/smf/gsm-handler.c index d01252611e..48bd5ce834 100644 --- a/src/smf/gsm-handler.c +++ b/src/smf/gsm-handler.c @@ -229,7 +229,11 @@ int gsm_handle_pdu_session_modification_request( int num_of_rule = 0; num_of_rule = ogs_nas_parse_qos_rules(qos_rule, requested_qos_rules); - ogs_assert(num_of_rule > 0); + if (!num_of_rule) { + ogs_error("[%s:%d] Invalid modification request", + smf_ue->supi, sess->psi); + goto cleanup; + } for (i = 0; i < num_of_rule; i++) { qos_flow = smf_qos_flow_find_by_qfi( @@ -430,7 +434,11 @@ int gsm_handle_pdu_session_modification_request( num_of_description = ogs_nas_parse_qos_flow_descriptions( qos_flow_description, requested_qos_flow_descriptions); - ogs_assert(num_of_description > 0); + if (!num_of_description) { + ogs_error("[%s:%d] Invalid modification request", + smf_ue->supi, sess->psi); + goto cleanup; + } for (i = 0; i < num_of_description; i++) { qos_flow = smf_qos_flow_find_by_qfi( @@ -478,16 +486,7 @@ int gsm_handle_pdu_session_modification_request( ogs_error("[%s:%d] Invalid modification request [modify:%d]", smf_ue->supi, sess->psi, ogs_list_count(&sess->qos_flow_to_modify_list)); - - n1smbuf = gsm_build_pdu_session_modification_reject(sess, - OGS_5GSM_CAUSE_INVALID_MANDATORY_INFORMATION); - ogs_assert(n1smbuf); - - smf_sbi_send_sm_context_update_error_n1_n2_message( - stream, OGS_SBI_HTTP_STATUS_BAD_REQUEST, - n1smbuf, OpenAPI_n2_sm_info_type_NULL, NULL); - - return OGS_ERROR; + goto cleanup; } if (pfcp_flags & OGS_PFCP_MODIFY_REMOVE) { @@ -522,4 +521,15 @@ int gsm_handle_pdu_session_modification_request( OGS_PFCP_MODIFY_UE_REQUESTED|pfcp_flags, 0)); return OGS_OK; + +cleanup: + n1smbuf = gsm_build_pdu_session_modification_reject(sess, + OGS_5GSM_CAUSE_INVALID_MANDATORY_INFORMATION); + ogs_assert(n1smbuf); + + smf_sbi_send_sm_context_update_error_n1_n2_message( + stream, OGS_SBI_HTTP_STATUS_BAD_REQUEST, + n1smbuf, OpenAPI_n2_sm_info_type_NULL, NULL); + + return OGS_ERROR; }