Fixing L4 offset for IP packets with Options field #54
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: evershalik <[email protected]>
evershalik
requested review from
jniesz,
sanfern,
jaysheth2 and
aka320
as code owners
aka320
reviewed
Jul 29, 2023
| __u8 l4_offset = iph->ihl * 4; // ipv4 header length | ||
|
|
||
| /* Check if it is a valid IPV4 packet */ | ||
| if (iph->ihl < 5 || ((unsigned char *)iph + l4_offset) > data_end) |
There was a problem hiding this comment.
Range of IHL is 5 to 15, with boundary values included. Please add check explicitly for this.
aka320
reviewed
Jul 29, 2023
| __u16 data_offset = tcph->doff * 4; // tcp header length | ||
|
|
||
| /* Check if it is a valid TCP packet */ | ||
| if (tcph->doff < 5 || ((unsigned char *)tcph + data_offset) > data_end) |
aka320
requested changes
Jul 29, 2023
|
@evershalik Can you address this above comments. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixed #30
Problem Statement:
The current implementation of the codebase does not handle IP and TCP packets with a header length greater than 20 bytes correctly. This issue is particularly problematic for packets containing options, which increase the header length beyond 20 bytes.
Approach:
To address this issue, we have taken the following approaches:
Firstly, I have modified the code to use the
l4_offsetvariable(IP Header Length) instead ofsizeof(iphdr struct)when adding to the iph pointer. This ensures that the code correctly traverses to the end of the IP header( or the start of the transport layer header), even in cases where the header length is greater than 20 bytes.Secondly, I have modified the code to use the
data_offsetvariable(TCP Header Length) instead ofsizeof(tcphdr struct)when adding to the tcph pointer. This ensures that the code correctly traverses to the end of the TCP header (or start of the payload), even in cases where the header length is greater than 20 bytes.By implementing these changes, I can ensure that IP and TCP packets with options are handled correctly, which improves the reliability and stability of the system.
Signed-off-by: SHANKAR [email protected]