[1397] in linux-net channel archive
Re: Spontaneous reboots caused by bogons
daemon@ATHENA.MIT.EDU (Tom May)
Fri Nov 17 16:40:57 1995
Date: Wed, 15 Nov 1995 23:17:58 -0800
From: ftom@netcom.com (Tom May)
To: gpg109@rsphy1.anu.edu.au
CC: linux-net@vger.rutgers.edu, iialan@www.linux.org.uk,
torvalds@cs.Helsinki.FI, root@claw.gactr.uga.edu
In-reply-to: <9511160542.AA06595@rsphy9.anu.edu.au> (message from Paul Gortmaker on Thu, 16 Nov 1995 16:42:39 +1100 (EST))
> The magic reboots that people have been seeing are not because
> of the 8390 patches, but because of the fact that eth_copy_and_sum()
> was broken, as I expected. This stems from a change Tom did
> back in 1.3.19, that being:
>
> - if (ntohs(iph->tot_len)-sizeof(struct iphdr) <= length)
> - length=ntohs(iph->tot_len)-sizeof(struct iphdr);
> + ip_length = ntohs(iph->tot_len) - sizeof(struct iphdr);
> + if (ip_length <= length)
> + length=ip_length;
>
>
> Guess what happens if a bogon with ip_length<8 comes along? Before, the
> unsigned math saved the day. But now all hell breaks loose. So anybody
> on a noisy net with lots of bogons would get hit. This would affect
> lance cards as well as sh-mem 8390 cards. Here is a 2 line fix.
>
> --- /tmp/linux/net/ethernet/eth.c Tue Oct 10 23:45:28 1995
> +++ linux/net/ethernet/eth.c Thu Nov 16 15:48:15 1995
> @@ -265,7 +265,9 @@
> length -= 34;
> iph=(struct iphdr*)(src+14); /* 14 = Rx_addr+Tx_addr+type_field */
> ip_length = ntohs(iph->tot_len) - sizeof(struct iphdr);
> - if (ip_length <= length)
> +
> + /* Also watch out for bogons - min IP size is 8 (rfc-1042) */
> + if ((ip_length <= length) && (ip_length >7))
> length=ip_length;
>
> dest->csum=csum_partial_copy(src+34,dest->data+34,length,base);
Shouldn't bogons be dropped before eth_copy_and_sum() is ever called?
Otherwise it's just copying and summing useless garbage and wasting time.
Tom.
P.S. Thanks for finding this. The sizeof() forcing the compare to
unsigned is pretty subtle. Some days I prefer the clarity of asm.
