ath@lcs.mit.edu (Andrew Heybey) (04/14/90)
Description:
[uVax III running 4.3+tahoe, Wisconsin NFS, various local hacks]
If one attempts to send an IP packet longer than 2^15 bytes
(ie one that has a negative length when interpreted as a signed
short), the machine panics because it tries to send the packet to the
interface directly without fragmenting it. This is because the ip_len
field in the ip header defined in netinet/ip.h is declared to be short
rather than u_short, and the test that decides whether the packet is
short enough to be sent without fragmentation does not cast ip_len to
unsigned before comparing to the mtu of the interface. There are
several other similar bugs scattered throughout the code. A large
packet that is received will eventually be dropped or will time out on
the reassembly queue, though it will not crash the machine.
According to the comment in ip.h, ip_len and ip_off were
declared to be short so that comparisons to negative numbers would
work. It would be better to declare them as unsigned, then cast them
to int when doing comparisons to signed numbers.
Of course, most people don't send datagrams this big, but it
does provide a reliable way to crash the machine from user code.
Repeat-By:
Write a program to open a socket and send a UDP packet with a length
of 32760 bytes (or so) or more.
Fix:
Change the declarations of all ip_len and ip_off fields in the
various structures in ip.h, ip_var.h, and udp.h to be u_short rather
than short. Then cast these values to int where ever they are
compared to signed numbers in the network code. Diffs for ip.h,
ip_var.h, udp.h, ip_input.c, ip_output.c, tcp_input.c, udp_usrreq.c
and netns/ns_ip.c follow.
*** /tmp/,RCSt1003614 Fri Apr 13 14:08:15 1990
- --- ip.h Thu Apr 12 11:23:29 1990
***************
*** 24,29 ****
- --- 24,37 ----
* pragmatically since otherwise unsigned comparisons can result
* against negative integers quite easily, and fail in subtle ways.
*/
+
+ /*
+ * On the other hand, if someone tries to send a UDP datagram larger
+ * than 32k bytes, the code fails in different not-so-subtle ways
+ * and panics. I have changed ip_len, ip_off, and the udp length field
+ * to be u_shorts. I have also cast them to int in all comparisons
+ * against signed integers to solve the above-mentioned problem. -ath
+ */
struct ip {
#ifdef vax
u_char ip_hl:4, /* header length */
***************
*** 30,38 ****
ip_v:4; /* version */
#endif
u_char ip_tos; /* type of service */
! short ip_len; /* total length */
u_short ip_id; /* identification */
! short ip_off; /* fragment offset field */
#define IP_DF 0x4000 /* dont fragment flag */
#define IP_MF 0x2000 /* more fragments flag */
u_char ip_ttl; /* time to live */
- --- 38,46 ----
ip_v:4; /* version */
#endif
u_char ip_tos; /* type of service */
! u_short ip_len; /* total length */
u_short ip_id; /* identification */
! u_short ip_off; /* fragment offset field */
#define IP_DF 0x4000 /* dont fragment flag */
#define IP_MF 0x2000 /* more fragments flag */
u_char ip_ttl; /* time to live */
*** /tmp/,RCSt1003619 Fri Apr 13 14:08:16 1990
- --- ip_input.c Thu Apr 12 11:32:31 1990
***************
*** 143,149 ****
* Convert fields to host representation.
*/
ip->ip_len = ntohs((u_short)ip->ip_len);
! if (ip->ip_len < hlen) {
ipstat.ips_badlen++;
goto bad;
}
- --- 143,149 ----
* Convert fields to host representation.
*/
ip->ip_len = ntohs((u_short)ip->ip_len);
! if ((int)ip->ip_len < hlen) {
ipstat.ips_badlen++;
goto bad;
}
***************
*** 398,404 ****
*/
while (q != (struct ipasfrag *)fp && ip->ip_off + ip->ip_len > q->ip_off) {
i = (ip->ip_off + ip->ip_len) - q->ip_off;
! if (i < q->ip_len) {
q->ip_len -= i;
q->ip_off += i;
m_adj(dtom(q), i);
- --- 398,404 ----
*/
while (q != (struct ipasfrag *)fp && ip->ip_off + ip->ip_len > q->ip_off) {
i = (ip->ip_off + ip->ip_len) - q->ip_off;
! if (i < (int)q->ip_len) {
q->ip_len -= i;
q->ip_off += i;
m_adj(dtom(q), i);
***************
*** 417,423 ****
ip_enq(ip, q->ipf_prev);
next = 0;
for (q = fp->ipq_next; q != (struct ipasfrag *)fp; q = q->ipf_next) {
! if (q->ip_off != next)
return (0);
next += q->ip_len;
}
- --- 417,423 ----
ip_enq(ip, q->ipf_prev);
next = 0;
for (q = fp->ipq_next; q != (struct ipasfrag *)fp; q = q->ipf_next) {
! if ((int)q->ip_off != next)
return (0);
next += q->ip_len;
}
*** /tmp/,RCSt1003625 Fri Apr 13 14:08:19 1990
- --- ip_output.c Thu Apr 12 11:19:26 1990
***************
*** 220,226 ****
goto bad;
}
/* don't allow broadcast messages to be fragmented */
! if (ip->ip_len > ifp->if_mtu) {
error = EMSGSIZE;
goto bad;
}
- --- 220,226 ----
goto bad;
}
/* don't allow broadcast messages to be fragmented */
! if ((int)ip->ip_len > ifp->if_mtu) {
error = EMSGSIZE;
goto bad;
}
***************
*** 229,235 ****
/*
* If small enough for interface, can just send directly.
*/
! if (ip->ip_len <= ifp->if_mtu) {
ip->ip_len = htons((u_short)ip->ip_len);
ip->ip_off = htons((u_short)ip->ip_off);
ip->ip_sum = 0;
- --- 229,235 ----
/*
* If small enough for interface, can just send directly.
*/
! if ((int)ip->ip_len <= ifp->if_mtu) {
ip->ip_len = htons((u_short)ip->ip_len);
ip->ip_off = htons((u_short)ip->ip_off);
ip->ip_sum = 0;
***************
*** 259,265 ****
*/
m->m_len -= sizeof (struct ip);
m->m_off += sizeof (struct ip);
! for (off = 0; off < ip->ip_len-hlen; off += len) {
struct mbuf *mh = m_get(M_DONTWAIT, MT_HEADER);
struct ip *mhip;
- --- 259,265 ----
*/
m->m_len -= sizeof (struct ip);
m->m_off += sizeof (struct ip);
! for (off = 0; off < (int)ip->ip_len-hlen; off += len) {
struct mbuf *mh = m_get(M_DONTWAIT, MT_HEADER);
struct ip *mhip;
***************
*** 278,284 ****
mhip->ip_off = (off >> 3) + (ip->ip_off & ~IP_MF);
if (ip->ip_off & IP_MF)
mhip->ip_off |= IP_MF;
! if (off + len >= ip->ip_len-hlen)
len = mhip->ip_len = ip->ip_len - hlen - off;
else {
mhip->ip_len = len;
- --- 278,284 ----
mhip->ip_off = (off >> 3) + (ip->ip_off & ~IP_MF);
if (ip->ip_off & IP_MF)
mhip->ip_off |= IP_MF;
! if (off + len >= (int)ip->ip_len-hlen)
len = mhip->ip_len = ip->ip_len - hlen - off;
else {
mhip->ip_len = len;
*** /tmp/,RCSt1003630 Fri Apr 13 14:08:22 1990
- --- ip_var.h Fri Apr 13 11:32:53 1990
***************
*** 18,24 ****
caddr_t ih_next, ih_prev; /* for protocol sequence q's */
u_char ih_x1; /* (unused) */
u_char ih_pr; /* protocol */
! short ih_len; /* protocol length */
struct in_addr ih_src; /* source internet address */
struct in_addr ih_dst; /* destination internet address */
};
- --- 18,24 ----
caddr_t ih_next, ih_prev; /* for protocol sequence q's */
u_char ih_x1; /* (unused) */
u_char ih_pr; /* protocol */
! u_short ih_len; /* protocol length */
struct in_addr ih_src; /* source internet address */
struct in_addr ih_dst; /* destination internet address */
};
***************
*** 50,58 ****
ip_v:4;
#endif
u_char ipf_mff; /* copied from (ip_off&IP_MF) */
! short ip_len;
u_short ip_id;
! short ip_off;
u_char ip_ttl;
u_char ip_p;
u_short ip_sum;
- --- 50,58 ----
ip_v:4;
#endif
u_char ipf_mff; /* copied from (ip_off&IP_MF) */
! u_short ip_len;
u_short ip_id;
! u_short ip_off;
u_char ip_ttl;
u_char ip_p;
u_short ip_sum;
*** /tmp/,RCSt1003635 Fri Apr 13 14:08:23 1990
- --- tcp_input.c Fri Apr 13 14:07:43 1990
***************
*** 117,123 ****
register int i = (ti->ti_seq + ti->ti_len) - q->ti_seq;
if (i <= 0)
break;
! if (i < q->ti_len) {
q->ti_seq += i;
q->ti_len -= i;
m_adj(dtom(q), i);
- --- 117,123 ----
register int i = (ti->ti_seq + ti->ti_len) - q->ti_seq;
if (i <= 0)
break;
! if (i < (int)q->ti_len) {
q->ti_seq += i;
q->ti_len -= i;
m_adj(dtom(q), i);
***************
*** 527,534 ****
tiflags &= ~TH_URG;
todrop--;
}
! if (todrop > ti->ti_len ||
! todrop == ti->ti_len && (tiflags&TH_FIN) == 0) {
#ifdef TCP_COMPAT_42
/*
* Don't toss RST in response to 4.2-style keepalive.
- --- 527,534 ----
tiflags &= ~TH_URG;
todrop--;
}
! if (todrop > (int)ti->ti_len ||
! todrop == (int)ti->ti_len && (tiflags&TH_FIN) == 0) {
#ifdef TCP_COMPAT_42
/*
* Don't toss RST in response to 4.2-style keepalive.
*** /tmp/,RCSt1003640 Fri Apr 13 14:08:26 1990
- --- udp.h Thu Apr 12 11:13:28 1990
***************
*** 18,23 ****
struct udphdr {
u_short uh_sport; /* source port */
u_short uh_dport; /* destination port */
! short uh_ulen; /* udp length */
u_short uh_sum; /* udp checksum */
};
- --- 18,23 ----
struct udphdr {
u_short uh_sport; /* source port */
u_short uh_dport; /* destination port */
! u_short uh_ulen; /* udp length */
u_short uh_sum; /* udp checksum */
};
*** /tmp/,RCSt1003645 Fri Apr 13 14:08:27 1990
- --- udp_usrreq.c Thu Apr 12 11:19:25 1990
***************
*** 85,92 ****
* If not enough data to reflect UDP length, drop.
*/
len = ntohs((u_short)ui->ui_ulen);
! if (((struct ip *)ui)->ip_len != len) {
! if (len > ((struct ip *)ui)->ip_len) {
udpstat.udps_badlen++;
goto bad;
}
- --- 85,92 ----
* If not enough data to reflect UDP length, drop.
*/
len = ntohs((u_short)ui->ui_ulen);
! if ((int)(((struct ip *)ui)->ip_len) != len) {
! if (len > (int)(((struct ip *)ui)->ip_len)) {
udpstat.udps_badlen++;
goto bad;
}
*** /tmp/,RCSt1003651 Fri Apr 13 14:08:48 1990
- --- ns_ip.c Thu Apr 12 11:19:24 1990
***************
*** 169,176 ****
idp = mtod(m, struct idp *);
len = ntohs(idp->idp_len);
if (len & 1) len++; /* Preserve Garbage Byte */
! if (ip->ip_len != len) {
! if (len > ip->ip_len) {
nsipif.if_ierrors++;
if (nsip_badlen) m_freem(nsip_badlen);
nsip_badlen = m;
- --- 169,176 ----
idp = mtod(m, struct idp *);
len = ntohs(idp->idp_len);
if (len & 1) len++; /* Preserve Garbage Byte */
! if ((int)ip->ip_len != len) {
! if (len > (int)ip->ip_len) {
nsipif.if_ierrors++;
if (nsip_badlen) m_freem(nsip_badlen);
nsip_badlen = m;
Andrew Heybey, ath@ptt.lcs.mit.edu
--
Andrew Heybey, ath@ptt.lcs.mit.edu, uunet!ptt.lcs.mit.edu!ath