Fixed broken ICMP error generation, unified conversion of IP header

fields between host and network byte order.  The details:

o icmp_error() now does not add IP header length.  This fixes the problem
  when icmp_error() is called from ip_forward().  In this case the ip_len
  of the original IP datagram returned with ICMP error was wrong.

o icmp_error() expects all three fields, ip_len, ip_id and ip_off in host
  byte order, so DTRT and convert these fields back to network byte order
  before sending a message.  This fixes the problem described in PR 16240
  and PR 20877 (ip_id field was returned in host byte order).

o ip_ttl decrement operation in ip_forward() was moved down to make sure
  that it does not corrupt the copy of original IP datagram passed later
  to icmp_error().

o A copy of original IP datagram in ip_forward() was made a read-write,
  independent copy.  This fixes the problem I first reported to Garrett
  Wollman and Bill Fenner and later put in audit trail of PR 16240:
  ip_output() (not always) converts fields of original datagram to network
  byte order, but because copy (mcopy) and its original (m) most likely
  share the same mbuf cluster, ip_output()'s manipulations on original
  also corrupted the copy.

o ip_output() now expects all three fields, ip_len, ip_off and (what is
  significant) ip_id in host byte order.  It was a headache for years that
  ip_id was handled differently.  The only compatibility issue here is the
  raw IP socket interface with IP_HDRINCL socket option set and a non-zero
  ip_id field, but ip.4 manual page was unclear on whether in this case
  ip_id field should be in host or network byte order.
This commit is contained in:
Ruslan Ermilov 2000-09-01 12:33:03 +00:00
parent a8a87cc61b
commit 04287599db
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=65327
7 changed files with 47 additions and 26 deletions

View File

@ -293,6 +293,7 @@ div_output(so, m, addr, control)
/* Convert fields to host order for ip_output() */
NTOHS(ip->ip_len);
NTOHS(ip->ip_id);
NTOHS(ip->ip_off);
/* Send packet to output processing */

View File

@ -191,7 +191,13 @@ icmp_error(n, type, code, dest, destifp)
icp->icmp_code = code;
bcopy((caddr_t)oip, (caddr_t)&icp->icmp_ip, icmplen);
nip = &icp->icmp_ip;
nip->ip_len = htons((u_short)(nip->ip_len + oiplen));
/*
* Convert fields to network representation.
*/
HTONS(nip->ip_len);
HTONS(nip->ip_id);
HTONS(nip->ip_off);
/*
* Now, copy old ip header (without options)

View File

@ -526,18 +526,12 @@ ip_input(struct mbuf *m)
* The packet is returned (relatively) intact; if
* ip_mforward() returns a non-zero value, the packet
* must be discarded, else it may be accepted below.
*
* (The IP ident field is put in the same byte order
* as expected when ip_mforward() is called from
* ip_output().)
*/
ip->ip_id = htons(ip->ip_id);
if (ip_mforward(ip, m->m_pkthdr.rcvif, m, 0) != 0) {
ipstat.ips_cantforward++;
m_freem(m);
return;
}
ip->ip_id = ntohs(ip->ip_id);
/*
* The process-level routing demon needs to receive
@ -1290,7 +1284,6 @@ ip_dooptions(m)
}
return (0);
bad:
ip->ip_len -= IP_VHL_HL(ip->ip_vhl) << 2; /* XXX icmp_error adds in hdr length */
icmp_error(m, type, code, 0, 0);
ipstat.ips_badoptions++;
return (1);
@ -1496,7 +1489,6 @@ ip_forward(m, srcrt)
m_freem(m);
return;
}
HTONS(ip->ip_id);
#ifdef IPSTEALTH
if (!ipstealth) {
#endif
@ -1505,7 +1497,6 @@ ip_forward(m, srcrt)
dest, 0);
return;
}
ip->ip_ttl -= IPTTLDEC;
#ifdef IPSTEALTH
}
#endif
@ -1534,6 +1525,16 @@ ip_forward(m, srcrt)
* we need to generate an ICMP message to the src.
*/
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));
#ifdef IPSTEALTH
if (!ipstealth) {
#endif
ip->ip_ttl -= IPTTLDEC;
#ifdef IPSTEALTH
}
#endif
/*
* If forwarding packet using same interface that it came in on,
@ -1672,6 +1673,8 @@ 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);
}

View File

@ -1581,7 +1581,7 @@ encap_send(ip, vifp, m)
*/
ip_copy = mtod(mb_copy, struct ip *);
*ip_copy = multicast_encap_iphdr;
ip_copy->ip_id = htons(ip_id++);
ip_copy->ip_id = ip_id++;
ip_copy->ip_len += len;
ip_copy->ip_src = vifp->v_lcl_addr;
ip_copy->ip_dst = vifp->v_rmt_addr;
@ -1592,6 +1592,7 @@ encap_send(ip, vifp, m)
ip = (struct ip *)((caddr_t)ip_copy + sizeof(multicast_encap_iphdr));
--ip->ip_ttl;
HTONS(ip->ip_len);
HTONS(ip->ip_id);
HTONS(ip->ip_off);
ip->ip_sum = 0;
mb_copy->m_data += sizeof(multicast_encap_iphdr);

View File

@ -211,7 +211,7 @@ ip_output(m0, opt, ro, flags, imo)
if ((flags & (IP_FORWARDING|IP_RAWOUTPUT)) == 0) {
ip->ip_vhl = IP_MAKE_VHL(IPVERSION, hlen >> 2);
ip->ip_off &= IP_DF;
ip->ip_id = htons(ip_id++);
ip->ip_id = ip_id++;
ipstat.ips_localout++;
} else {
hlen = IP_VHL_HL(ip->ip_vhl) << 2;
@ -520,6 +520,7 @@ ip_output(m0, opt, ro, flags, imo)
/* Restore packet header fields to original values */
HTONS(ip->ip_len);
HTONS(ip->ip_id);
HTONS(ip->ip_off);
/* Deliver packet to divert input routine */
@ -593,8 +594,9 @@ ip_output(m0, opt, ro, flags, imo)
}
m->m_pkthdr.csum_flags |=
CSUM_IP_CHECKED | CSUM_IP_VALID;
ip->ip_len = htons((u_short)ip->ip_len);
ip->ip_off = htons((u_short)ip->ip_off);
HTONS(ip->ip_len);
HTONS(ip->ip_id);
HTONS(ip->ip_off);
ip_input(m);
goto done;
}
@ -712,8 +714,9 @@ ip_output(m0, opt, ro, flags, imo)
m->m_pkthdr.csum_flags &= ~CSUM_DELAY_DATA;
}
ip->ip_len = htons((u_short)ip->ip_len);
ip->ip_off = htons((u_short)ip->ip_off);
HTONS(ip->ip_len);
HTONS(ip->ip_id);
HTONS(ip->ip_off);
error = ipsec4_output(&state, sp, flags);
@ -772,8 +775,9 @@ ip_output(m0, opt, ro, flags, imo)
}
/* make it flipped, again. */
ip->ip_len = ntohs((u_short)ip->ip_len);
ip->ip_off = ntohs((u_short)ip->ip_off);
NTOHS(ip->ip_len);
NTOHS(ip->ip_id);
NTOHS(ip->ip_off);
skip_ipsec:
#endif /*IPSEC*/
@ -791,8 +795,9 @@ ip_output(m0, opt, ro, flags, imo)
*/
if ((u_short)ip->ip_len <= ifp->if_mtu ||
ifp->if_hwassist & CSUM_FRAGMENT) {
ip->ip_len = htons((u_short)ip->ip_len);
ip->ip_off = htons((u_short)ip->ip_off);
HTONS(ip->ip_len);
HTONS(ip->ip_id);
HTONS(ip->ip_off);
ip->ip_sum = 0;
if (sw_csum & CSUM_DELAY_IP) {
if (ip->ip_vhl == IP_VHL_BORING) {
@ -887,7 +892,8 @@ ip_output(m0, opt, ro, flags, imo)
m->m_pkthdr.len = mhlen + len;
m->m_pkthdr.rcvif = (struct ifnet *)0;
m->m_pkthdr.csum_flags = m0->m_pkthdr.csum_flags;
mhip->ip_off = htons((u_short)mhip->ip_off);
HTONS(mhip->ip_id);
HTONS(mhip->ip_off);
mhip->ip_sum = 0;
if (sw_csum & CSUM_DELAY_IP) {
if (mhip->ip_vhl == IP_VHL_BORING) {
@ -915,7 +921,9 @@ ip_output(m0, opt, ro, flags, imo)
m_adj(m, hlen + firstlen - (u_short)ip->ip_len);
m->m_pkthdr.len = hlen + firstlen;
ip->ip_len = htons((u_short)m->m_pkthdr.len);
ip->ip_off = htons((u_short)(ip->ip_off | IP_MF));
HTONS(ip->ip_id);
ip->ip_off |= IP_MF;
HTONS(ip->ip_off);
ip->ip_sum = 0;
if (sw_csum & CSUM_DELAY_IP) {
if (ip->ip_vhl == IP_VHL_BORING) {
@ -1855,8 +1863,9 @@ ip_mloopback(ifp, m, dst, hlen)
* than the interface's MTU. Can this possibly matter?
*/
ip = mtod(copym, struct ip *);
ip->ip_len = htons((u_short)ip->ip_len);
ip->ip_off = htons((u_short)ip->ip_off);
HTONS(ip->ip_len);
HTONS(ip->ip_id);
HTONS(ip->ip_off);
ip->ip_sum = 0;
if (ip->ip_vhl == IP_VHL_BORING) {
ip->ip_sum = in_cksum_hdr(ip);

View File

@ -221,7 +221,7 @@ rip_output(m, so, dst)
return EINVAL;
}
if (ip->ip_id == 0)
ip->ip_id = htons(ip_id++);
ip->ip_id = ip_id++;
/* XXX prevent ip_output from overwriting header fields */
flags |= IP_RAWOUTPUT;
ipstat.ips_rawout++;

View File

@ -353,11 +353,12 @@ udp_input(m, off, proto)
udpstat.udps_noportbcast++;
goto bad;
}
*ip = save_ip;
if (badport_bandlim(0) < 0)
goto bad;
if (blackhole)
goto bad;
*ip = save_ip;
ip->ip_len += iphlen;
icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_PORT, 0, 0);
return;
}