Page MenuHomeFreeBSD

Check that tryforward didn't changed mbuf
ClosedPublic

Authored by ae on Dec 12 2016, 12:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 11:13 PM
Unknown Object (File)
Tue, Nov 19, 2:09 AM
Unknown Object (File)
Mon, Nov 18, 11:09 AM
Unknown Object (File)
Mon, Nov 18, 12:27 AM
Unknown Object (File)
Mon, Nov 18, 12:10 AM
Unknown Object (File)
Sun, Nov 17, 9:57 AM
Unknown Object (File)
Thu, Nov 14, 12:04 PM
Unknown Object (File)
Sun, Nov 10, 2:49 PM
Subscribers

Details

Summary

ip[6]_input() calls ip[6]_tryforward() and expects if returned value
is not NULL, this means that it left mbuf unchanged for slow path processing.
This isn't always true, because tryforward does inbound and outbound packet
processing. This at least can result in changed mbuf pointer due to used
m_pullup(). Also in some cases tryforward can return this changed mbuf
back to the ip[6]_input() due to it decided that destination becomes our
own address. This wasn't the problem, when we called ip[6]_fastfwd() from
ether_input(), because netisr invoked ip[6]_input() and all pointers
was updated at the top of ip[6]_input().

To fix this I store the returned value from ip[6]_tryforward() into the
mbuf pointer, then check m_flags for M_FASTFWD_OURS flag. If it is present,
this means that ip[6]_tryforward() returned changed mbuf pointer.

Test Plan

I didn't tested this yet, but it seems it should be very easy to trigger the problem using ipfw fwd.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ae retitled this revision from to Check that tryforward didn't changed mbuf.
ae updated this object.
ae edited the test plan for this revision. (Show Details)
ae added a reviewer: melifaro.
ae added reviewers: network, gnn.

Fix copy-paste bug, use correct label in the ip_input.

It appears not so easy to reproduce the bug. It seems ip_input() always gets mbuf with m_len that enough to avoid m_pullup().
But I'm sure it possible in some cases trigger the problem.

This revision was automatically updated to reflect the committed changes.