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

Extracting IP header from ICMP error message payload #420

Closed
wants to merge 0 commits into from

Conversation

sheharbano
Copy link
Contributor

The following line assumes that sizeof(struct icmp) == 8. It does not correctly extract the original IP header from ICMP error message payload.

struct ip *ip_inner = (struct ip *) &icmp[1];

I replaced the line above with this one:

// ICMP header is 8 bytes long
struct ip ip_inner = (struct ip) ((char *)icmp + 8);

@zakird
Copy link
Member

zakird commented Apr 11, 2017

It looks like this doesn't currently build. Can you take a look?

@sheharbano
Copy link
Contributor Author

Sorry about that. Can you please try building with the suggested change.

It should be
struct ip ip_inner = (struct ip) ((char *) icmp_h+8);

rather than
struct ip ip_inner = (struct ip) ((char *)icmp + 8);

@dadrian
Copy link
Member

dadrian commented Apr 14, 2017

@sheharbano It doesn't look like this merges or compiles. Can you update this PR?

@sheharbano
Copy link
Contributor Author

Updated. I just tried, successfully built on my end.

@zakird
Copy link
Member

zakird commented Apr 14, 2017

This looks like it still unfortunately conflicts with master and cannot be merged.

@sheharbano sheharbano closed this Apr 14, 2017
zakird pushed a commit that referenced this pull request Apr 16, 2017
…424)

* Fixed definition of ip_inner

* removing commented out code

* removing commented out code

* Update module_icmp_echo.c

* Update module_icmp_echo_time.c
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.

None yet

3 participants