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

We're extracting the MTU of the incoming packet's interface incorrectly. #84

Closed
ydahhrk opened this Issue Mar 15, 2014 · 5 comments

Comments

Projects
None yet
1 participant
@ydahhrk
Member

ydahhrk commented Mar 15, 2014

From translate_packet_4to6.c:

icmpv6_hdr->icmp6_mtu = icmp6_minimum_mtu(...,
    ...,
    in->skb->dev->mtu + 20,
    ...);

The problem is that in->skb->dev can be null.

We need to figure out a reliable way to get the MTU of the incoming packet's interface.

translate_packet_6to4.c also has this problem.

Thanks to Philar Law for reporting this.

@ydahhrk ydahhrk added this to the 3.1.3 milestone Mar 15, 2014

ydahhrk added a commit that referenced this issue Mar 18, 2014

Temporal workaround for issue #84.
The module should no longer crash the kernel, but the behavior might disobey RFC 6146.
@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Mar 18, 2014

Member

The commit above should prevent the kernel panic. Jool still doesn't completely behave as expected, but at least the kernel doesn't crash horribly anymore.
Downgrading priority and moving milestone so I can focus on #79.

Member

ydahhrk commented Mar 18, 2014

The commit above should prevent the kernel panic. Jool still doesn't completely behave as expected, but at least the kernel doesn't crash horribly anymore.
Downgrading priority and moving milestone so I can focus on #79.

@ydahhrk ydahhrk modified the milestones: 3.1.4, 3.1.3 Mar 18, 2014

ydahhrk added a commit that referenced this issue Mar 22, 2014

Issue #79 solved, issue #84 apparently also solved.
I still want to run a few more tests. Guess the milestone will be held back for a day.
@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Mar 25, 2014

Member

As far as I can tell, this is already fixed.

Though the solution is already in the master branch, I'll wait for some user feedback before I close this.

Member

ydahhrk commented Mar 25, 2014

As far as I can tell, this is already fixed.

Though the solution is already in the master branch, I'll wait for some user feedback before I close this.

@ydahhrk ydahhrk self-assigned this Mar 27, 2014

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Apr 7, 2014

Member

A kernel crash remains, but only in newer kernels, though it appears it's a separate problem:

If the IPv4 pool runs out of addresses and a new connection is initiated, the attempt to send a ICMP error appears to be the cause of a freeze.

This has been confirmed in kernel 3.12.

Member

ydahhrk commented Apr 7, 2014

A kernel crash remains, but only in newer kernels, though it appears it's a separate problem:

If the IPv4 pool runs out of addresses and a new connection is initiated, the attempt to send a ICMP error appears to be the cause of a freeze.

This has been confirmed in kernel 3.12.

@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Apr 16, 2014

Member

The latter problem, though related, is a separate one. I moved it to issue #90.

Member

ydahhrk commented Apr 16, 2014

The latter problem, though related, is a separate one. I moved it to issue #90.

ydahhrk added a commit that referenced this issue Apr 21, 2014

Last tweaks to issue #84.
Made the code more conformant to the RFC and revised the reference
counting of dst_entries.
I think I don't have any more details to worry about, though I'm going
to test a little more.

ydahhrk added a commit that referenced this issue Apr 23, 2014

Better implementation of issue #84. I got rid of an
obnoxious temp field in the fragment structure.
I also noticed that addresses in inner packets used
to be swapped, which is actually far more critical.
@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Apr 23, 2014

Member

Fixed and collapsed to the master branch; closing.

Member

ydahhrk commented Apr 23, 2014

Fixed and collapsed to the master branch; closing.

@ydahhrk ydahhrk closed this Apr 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment