Bug 227674

Summary: [ipfw] [ipv6] ICMPv6 echo replies incorrectly matched by kernel ipfw
Product: Base System Reporter: Eugene Grosbein <eugen>
Component: kernAssignee: Andrey V. Elsukov <ae>
Status: Closed FIXED    
Severity: Affects Some People CC: ae, glebius, ipfw
Priority: ---    
Version: 11.1-STABLE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Proposed patch none

Description Eugene Grosbein freebsd_committer freebsd_triage 2018-04-21 18:34:26 UTC 
This is very similar to old PR https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=131817 fixed 6 years ago with https://svnweb.freebsd.org/base?view=revision&revision=223753

Now ipfw rule "deny log ip from any to any out recv re0 xmit re0" incorrectly matches outgoing ICMPv6 echo replies sent by the system in response to incoming echo request. The reply should not have "recv" attribute and should not be matched.

I suspect that as in older ARP problem, the code re-uses mbuf and forgets to nullify m->m_pkthdr.rcvif
Comment 1 Andrey V. Elsukov freebsd_committer freebsd_triage 2018-04-23 10:43:43 UTC 
Created attachment 192745 [details]
Proposed patch
Comment 2 Eugene Grosbein freebsd_committer freebsd_triage 2018-04-23 11:16:01 UTC 
(In reply to Andrey V. Elsukov from comment #1)

It works, thanks! Please commit.
Comment 3 commit-hook freebsd_committer freebsd_triage 2018-04-23 12:20:45 UTC 
A commit references this bug:

Author: ae
Date: Mon Apr 23 12:20:08 UTC 2018
New revision: 332886
URL: https://svnweb.freebsd.org/changeset/base/332886

Log:
  icmp6_reflect() sends ICMPv6 message with new IPv6 header. So, it is
  considered as originated by our host packet. And thus rcvif should be
  NULL, since it is used by ipfw(4) to determine that packet was originated
  from this host. Some of icmp6_reflect() consumers reuse mbuf and m_pkthdr
  without resetting rcvif pointer. To avoid this always reset m_pkthdr.rcvif
  pointer to NULL in icmp6_reflect(). Also remove such line and comment
  describing this from icmp6_error(), since it does not longer matters.

  PR:		227674
  Reported by:	eugen
  MFC after:	1 week

Changes:
  head/sys/netinet6/icmp6.c
Comment 4 commit-hook freebsd_committer freebsd_triage 2018-05-03 08:16:34 UTC 
A commit references this bug:

Author: ae
Date: Thu May  3 08:15:32 UTC 2018
New revision: 333206
URL: https://svnweb.freebsd.org/changeset/base/333206

Log:
  MFC r332886:
    icmp6_reflect() sends ICMPv6 message with new IPv6 header. So, it is
    considered as originated by our host packet. And thus rcvif should be
    NULL, since it is used by ipfw(4) to determine that packet was originated
    from this host. Some of icmp6_reflect() consumers reuse mbuf and m_pkthdr
    without resetting rcvif pointer. To avoid this always reset m_pkthdr.rcvif
    pointer to NULL in icmp6_reflect(). Also remove such line and comment
    describing this from icmp6_error(), since it does not longer matters.

    PR:		227674

Changes:
_U  stable/11/
  stable/11/sys/netinet6/icmp6.c
Comment 5 Eugene Grosbein freebsd_committer freebsd_triage 2018-05-04 19:41:25 UTC 
(In reply to Andrey V. Elsukov from comment #1)

Have you any plans to merge this to stable/10 ?
Comment 6 Andrey V. Elsukov freebsd_committer freebsd_triage 2018-05-07 10:20:13 UTC 
(In reply to Eugene Grosbein from comment #5)
> (In reply to Andrey V. Elsukov from comment #1)
> 
> Have you any plans to merge this to stable/10 ?

If you are able to test it on stable/10, feel free to commit it.