From d4f2a95b6c4f090e155d01113b0bed2130a98986 Mon Sep 17 00:00:00 2001 From: Louis Royer Date: Sat, 14 Dec 2024 10:38:22 +0100 Subject: [PATCH] fix: wrong GTP length when Sequence Number is enabled and QoS is disabled ## Bug description If you create a configuration that make a new GTP packet with Sequence Number but you don't provide a QER IE (which is an optional IE), the resulting GTP packet's length field is off by 4. This make the packet total payload is shorter than indicated in the header, resulting in interoperability issues (in particular, when attempting to use a custom SMF that will push a different set of PFCP rules than Free5GC's one, and where QoS management is not yet implemented). For exemple, when creating a gNB using [go-gtp](https://github.com/wmnsk/go-gtp), such packet is considered malformed, and is always dropped. ## Bug cause This bug results from the following: ```c ext_pdu_sess_ctrl_t *ext_pdu_sess; // ... if (opt_flag) { // ... payload_len += ((sizeof(*gtp1opt) + sizeof(*ext_pdu_sess)); } ``` When `ext_pdu_sess` is not initialized, `sizeof(*ext_pdu_sess)` returns `4` and not `0`. ## Bug mitigation The fix is to split the incrementation in 2 steps: 1. increment by `sizeof(*gtp1opt)` if an option (Sequence Number, or Extension Header) is provided; 2. ensure `ext_pdu_sess` is well initialized before incrementing `payload_len` by `sizeof(*ext_pdu_sess)`. --- src/gtpu/pktinfo.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gtpu/pktinfo.c b/src/gtpu/pktinfo.c index 65c0591..9207d5a 100644 --- a/src/gtpu/pktinfo.c +++ b/src/gtpu/pktinfo.c @@ -331,7 +331,11 @@ void gtp5g_push_header(struct sk_buff *skb, struct gtp5g_pktinfo *pktinfo) gtp1opt->NPDU = 0; gtp1opt->next_ehdr_type = next_ehdr_type; // Increment the GTP-U payload length by size of optional headers length - payload_len += (sizeof(*gtp1opt) + sizeof(*ext_pdu_sess)); + payload_len += sizeof(*gtp1opt); + } + // Increment the GTP-U payload length by size of extensions length + if (ext_flag) { + payload_len += sizeof(*ext_pdu_sess); } /* Bits 8 7 6 5 4 3 2 1