Subject: kern/2772: minor mistake in ip-input.c
To: None <gnats-bugs@gnats.netbsd.org>
From: None <daw@panix.com>
List: netbsd-bugs
Date: 09/20/1996 17:23:14
>Number: 2772
>Category: kern
>Synopsis: ip-input fragmentation code can be executed unnecessarily
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: kern-bug-people (Kernel Bug People)
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Sep 20 14:35:02 1996
>Last-Modified:
>Originator: Nathaniel D. Daw
>Organization:
Piermont Information Systems
>Release: NetBSD-current 9/20/96
>Environment:
System: NetBSD daw.dialup.access.net 1.2 NetBSD 1.2 (NAT2) #25: Fri Sep 20 16:52:18 EDT 1996 nat@daw.dialup.access.net:/usr/src/sys/arch/i386/compile/NAT2 i386
>Description:
The code in ip-input.c tests against the 16-bit fragmentation
structure in the IP header to determine whether an incoming
packet is fragmented and thus whether to run the (potentially
expensive) search through a linear queue to find the rest of
the packet's fragments. This test is faulty and could result
in running this search more often than necessary. Not a bad
bug, but a bit of a potential resource drain nonetheless.
The structure in question looks like this:
meaning: 0 DF MF offset
bits: 1 1 1 13
The 0 bit is reserved and is required to be zero.
The code masks out the DF bit and then, if any of the rest of
the bits in the structure are set, the packet is considered a
fragment. But this test will pass, and the fragment code run,
even if the packet is not a fragment and just the "must be
zero" reserved flag is set. Nothing enforces this must be zero
rule; anyone can set this bit in their communications with you
and cause you to do extra work for each packet they send.
>How-To-Repeat:
to see this actually happen you have to modify your kernel to
track the extra runs of the fragmentation code, and send it
faulty packets with the reserved bit set (which will also take
kernel mods.
>Fix:
Here are patches to ip-input.c and ip.h to correct this minor
error.
*** ip.h.orig Fri Sep 20 16:31:05 1996
--- ip.h Fri Sep 20 17:21:31 1996
***************
*** 61,66 ****
--- 61,67 ----
int16_t ip_len; /* total length */
u_int16_t ip_id; /* identification */
int16_t ip_off; /* fragment offset field */
+ #define IP_RF 0x8000 /* reserved fragment flag */
#define IP_DF 0x4000 /* dont fragment flag */
#define IP_MF 0x2000 /* more fragments flag */
#define IP_OFFMASK 0x1fff /* mask for fragmenting bits */
*** ip_input.c.orig Fri Sep 20 16:33:14 1996
--- ip_input.c Fri Sep 20 16:34:20 1996
***************
*** 345,351 ****
* if the packet was previously fragmented,
* but it's not worth the time; just let them time out.)
*/
! if (ip->ip_off &~ IP_DF) {
if (m->m_flags & M_EXT) { /* XXX */
if ((m = m_pullup(m, sizeof (struct ip))) == 0) {
ipstat.ips_toosmall++;
--- 345,351 ----
* if the packet was previously fragmented,
* but it's not worth the time; just let them time out.)
*/
! if (ip->ip_off &~ (IP_DF | IP_RF)) {
if (m->m_flags & M_EXT) { /* XXX */
if ((m = m_pullup(m, sizeof (struct ip))) == 0) {
ipstat.ips_toosmall++;
>Audit-Trail:
>Unformatted: