Hi,
The checksum of a ICMP "need to frag" packet for TCP was wrong when
created from a ICMP6 "too big" packet. The function pf_change_icmp_af()
has code to adjust the pseudo-header checksum in the ICMP6 case,
but pf_test_state_icmp() changed the proto before the case was
entered.
So call pf_change_icmp_af() before the pd->proto is converted in
the TCP and UDP payload case like it was already done for ICMP and
ICMP6 payload case.
This was found by the sys/net/pf_forward regress test.
My printf debugging revealed that dlen is positive and d may be
negative. So declare the variables with the correct sign in
pf_change_icmp_af().
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.989
diff -u -p -r1.989 pf.c
--- net/pf.c 9 Oct 2016 18:01:57 -0000 1.989
+++ net/pf.c 17 Oct 2016 20:00:41 -0000
@@ -2340,7 +2340,8 @@ pf_change_icmp_af(struct mbuf *m, int ip
struct mbuf *n = NULL;
struct ip *ip4;
struct ip6_hdr *ip6;
- u_int hlen, ohlen, d;
+ u_int hlen, ohlen, dlen;
+ int d;
if (af == naf || (af != AF_INET && af != AF_INET6) ||
(naf != AF_INET && naf != AF_INET6))
@@ -2417,7 +2418,7 @@ pf_change_icmp_af(struct mbuf *m, int ip
if (pd->proto == IPPROTO_ICMPV6) {
/* fixup pseudo-header */
- int dlen = pd->tot_len - pd->off;
+ dlen = pd->tot_len - pd->off;
pf_cksum_fixup(pd->pcksum,
htons(dlen), htons(dlen + d), pd->proto);
}
@@ -5104,6 +5105,10 @@ pf_test_state_icmp(struct pf_pdesc *pd,
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr),
pd->hdr.icmp6, M_NOWAIT);
+ if (pf_change_icmp_af(pd->m, ipoff2,
+ pd, &pd2, &nk->addr[sidx],
+ &nk->addr[didx], pd->af, nk->af))
+ return (PF_DROP);
if (nk->af == AF_INET)
pd->proto = IPPROTO_ICMP;
else
@@ -5117,11 +5122,6 @@ pf_test_state_icmp(struct pf_pdesc *pd,
&nk->addr[pd2.didx], nk->af);
pd->naf = nk->af;
- if (pf_change_icmp_af(pd->m, ipoff2,
- pd, &pd2, &nk->addr[sidx],
- &nk->addr[didx], pd->af, nk->af))
- return (PF_DROP);
-
pf_patch_16(pd,
&th.th_sport, nk->port[sidx]);
pf_patch_16(pd,
@@ -5220,6 +5220,10 @@ pf_test_state_icmp(struct pf_pdesc *pd,
m_copyback(pd->m, pd->off,
sizeof(struct icmp6_hdr),
pd->hdr.icmp6, M_NOWAIT);
+ if (pf_change_icmp_af(pd->m, ipoff2,
+ pd, &pd2, &nk->addr[sidx],
+ &nk->addr[didx], pd->af, nk->af))
+ return (PF_DROP);
if (nk->af == AF_INET)
pd->proto = IPPROTO_ICMP;
else
@@ -5232,11 +5236,6 @@ pf_test_state_icmp(struct pf_pdesc *pd,
PF_ACPY(&pd->ndaddr,
&nk->addr[pd2.didx], nk->af);
pd->naf = nk->af;
-
- if (pf_change_icmp_af(pd->m, ipoff2,
- pd, &pd2, &nk->addr[sidx],
- &nk->addr[didx], pd->af, nk->af))
- return (PF_DROP);
pf_patch_16(pd,
&uh.uh_sport, nk->port[sidx]);
