Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrong GTP length when Sequence Number is enabled and QoS is disabled #130

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

louisroyer
Copy link
Contributor

@louisroyer louisroyer commented Dec 14, 2024

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 makes the packet total payload 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, such packet is considered malformed, and is always dropped.

Bug cause

This bug results from the following:

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

…bled

## 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 makes the packet total payload 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)`.
@tim-ywliu tim-ywliu merged commit 5c04e67 into free5gc:master Dec 18, 2024
@tim-ywliu
Copy link
Collaborator

LGTM

@louisroyer louisroyer deleted the fix-gtp-len branch December 19, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants