On Fri, Sep 11, 2015 at 02:16:12PM +0200, Martin Pieuchot wrote:
> On 11/09/15(Fri) 13:20, Alexander Bluhm wrote:
> > Hi,
> >
> > When pf modifies a TCP packet, it sets the M_TCP_CSUM_OUT flag in
> > the mbuf packet header. If the packet is later dropped in
> > ip6_forward(), the TCP mbuf is copied and passed to icmp6_error().
> > The inherited M_TCP_CSUM_OUT flag generates ICMP6 packets with
> > incorrect checksum. So reset the csum_flags when packets are
> > generated by icmp6_reflect() or icmp6_redirect_output().
>
> Don't you think the same problem can occur in IPv4 and icmp_send()
> could also benefit from the same fix?
IPv6 uses m_copym() and M_PREPEND() which preserve the packet header.
IPv4 does m_copydata() into a fresh mbuf. There m_inithdr() clears
the packet header, so the problem does not occur.
But setting the csum_flags explicitly also makes sense for icmp_send().
Do not or it to a value that is 0 because of some function calls
before.
ok?
bluhm
Index: netinet/ip_icmp.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.139
diff -u -p -r1.139 ip_icmp.c
--- netinet/ip_icmp.c 10 Sep 2015 12:10:52 -0000 1.139
+++ netinet/ip_icmp.c 11 Sep 2015 13:30:59 -0000
@@ -846,7 +846,7 @@ icmp_send(struct mbuf *m, struct mbuf *o
hlen = ip->ip_hl << 2;
icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
icp->icmp_cksum = 0;
- m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
+ m->m_pkthdr.csum_flags = M_ICMP_CSUM_OUT;
#ifdef ICMPPRINTFS
if (icmpprintfs) {
char dst[INET_ADDRSTRLEN], src[INET_ADDRSTRLEN];
Index: netinet6/icmp6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.167
diff -u -p -r1.167 icmp6.c
--- netinet6/icmp6.c 11 Sep 2015 08:17:06 -0000 1.167
+++ netinet6/icmp6.c 11 Sep 2015 12:36:25 -0000
@@ -1283,7 +1283,7 @@ icmp6_reflect(struct mbuf *m, size_t off
ip6->ip6_hlim = ip6_defhlim;
icmp6->icmp6_cksum = 0;
- m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
+ m->m_pkthdr.csum_flags = M_ICMP_CSUM_OUT;
/*
* XXX option handling
@@ -1786,7 +1786,7 @@ noredhdropt:
ip6->ip6_plen = htons(m->m_pkthdr.len - sizeof(struct ip6_hdr));
nd_rd->nd_rd_cksum = 0;
- m->m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
+ m->m_pkthdr.csum_flags = M_ICMP_CSUM_OUT;
/* send the packet to outside... */
ip6_output(m, NULL, NULL, 0, NULL, NULL);
