summaryrefslogtreecommitdiffstats
path: root/sys
diff options
context:
space:
mode:
authoriedowse <iedowse@FreeBSD.org>2001-03-08 19:03:26 +0000
committeriedowse <iedowse@FreeBSD.org>2001-03-08 19:03:26 +0000
commit9852c67f7c44b190f6a7c0c8b8a12205334411ab (patch)
treedf4def85f1ea1a92213e2397454c3c02c7e77718 /sys
parentfb925d8493457c06e0ec9cf4781746978ad8aa3d (diff)
downloadFreeBSD-src-9852c67f7c44b190f6a7c0c8b8a12205334411ab.zip
FreeBSD-src-9852c67f7c44b190f6a7c0c8b8a12205334411ab.tar.gz
It was possible for ip_forward() to supply to icmp_error()
an IP header with ip_len in network byte order. For certain values of ip_len, this could cause icmp_error() to write beyond the end of an mbuf, causing mbuf free-list corruption. This problem was observed during generation of ICMP redirects. We now make quite sure that the copy of the IP header kept for icmp_error() is stored in a non-shared mbuf header so that it will not be modified by ip_output(). Also: - Calculate the correct number of bytes that need to be retained for icmp_error(), instead of assuming that 64 is enough (it's not). - In icmp_error(), use m_copydata instead of bcopy() to copy from the supplied mbuf chain, in case the first 8 bytes of IP payload are not stored directly after the IP header. - Sanity-check ip_len in icmp_error(), and panic if it is less than sizeof(struct ip). Incoming packets with bad ip_len values are discarded in ip_input(), so this should only be triggered by bugs in the code, not by bad packets. This patch results from code and suggestions from Ruslan, Bosko, Jonathan Lemon and Matt Dillon, with important testing by Mike Tancsa, who could reproduce this problem at will. Reported by: Mike Tancsa <mike@sentex.net> Reviewed by: ru, bmilekic, jlemon, dillon
Diffstat (limited to 'sys')
-rw-r--r--sys/netinet/ip_icmp.c4
-rw-r--r--sys/netinet/ip_input.c21
2 files changed, 17 insertions, 8 deletions
diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c
index 5b9aa08..bdcc884 100644
--- a/sys/netinet/ip_icmp.c
+++ b/sys/netinet/ip_icmp.c
@@ -164,6 +164,8 @@ icmp_error(n, type, code, dest, destifp)
if (m == NULL)
goto freeit;
icmplen = min(oiplen + 8, oip->ip_len);
+ if (icmplen < sizeof(struct ip))
+ panic("icmp_error: bad length");
m->m_len = icmplen + ICMP_MINLEN;
MH_ALIGN(m, m->m_len);
icp = mtod(m, struct icmp *);
@@ -189,7 +191,7 @@ icmp_error(n, type, code, dest, destifp)
}
icp->icmp_code = code;
- bcopy((caddr_t)oip, (caddr_t)&icp->icmp_ip, icmplen);
+ m_copydata(n, 0, icmplen, (caddr_t)&icp->icmp_ip);
nip = &icp->icmp_ip;
/*
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 9d1fe46..78753dd 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -1563,12 +1563,21 @@ ip_forward(m, srcrt)
}
/*
- * Save at most 64 bytes of the packet in case
- * we need to generate an ICMP message to the src.
+ * Save the IP header and at most 8 bytes of the payload,
+ * in case we need to generate an ICMP message to the src.
+ *
+ * We don't use m_copy() because it might return a reference
+ * to a shared cluster. Both this function and ip_output()
+ * assume exclusive access to the IP header in `m', so any
+ * data in a cluster may change before we reach icmp_error().
*/
- mcopy = m_copy(m, 0, imin((int)ip->ip_len, 64));
- if (mcopy && (mcopy->m_flags & M_EXT))
- m_copydata(mcopy, 0, sizeof(struct ip), mtod(mcopy, caddr_t));
+ MGET(mcopy, M_DONTWAIT, m->m_type);
+ if (mcopy != NULL) {
+ M_COPY_PKTHDR(mcopy, m);
+ mcopy->m_len = imin((IP_VHL_HL(ip->ip_vhl) << 2) + 8,
+ (int)ip->ip_len);
+ m_copydata(m, 0, mcopy->m_len, mtod(mcopy, caddr_t));
+ }
#ifdef IPSTEALTH
if (!ipstealth) {
@@ -1715,8 +1724,6 @@ ip_forward(m, srcrt)
m_freem(mcopy);
return;
}
- if (mcopy->m_flags & M_EXT)
- m_copyback(mcopy, 0, sizeof(struct ip), mtod(mcopy, caddr_t));
icmp_error(mcopy, type, code, dest, destifp);
}
OpenPOWER on IntegriCloud