On 11/09/15(Fri) 15:50, Alexander Bluhm wrote:
> 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?
ok mpi@
>
> 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);
>
