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: