Subject: kern/23413: if_attach issue
To: None <gnats-bugs@gnats.NetBSD.org>
From: None <managgarwal@hss.hns.com>
List: netbsd-bugs
Date: 11/11/2003 12:13:40
>Number:         23413
>Category:       kern
>Synopsis:       if_attach issue
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Nov 11 12:14:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     Manish Aggarwal
>Release:        1.6
>Organization:
Hughes Software System
>Environment:
NetBSD deepspace.hss.hns.com 1.6 NetBSD 1.6 (GENERIC)
>Description:
Hi ,

I faced crash in NetBSD kernel while creating and deleting interfaces when i executed a script which creates/destroy  interfaces in succession..(say create 128 vlan interfaces and delete 128 vlan interfaces)

I am using Netbsd version 1.6

On debugging I came across an issue in if_attach function...
Atleast preliminary study leads me to this conclusion ..

Issue with if_attach function..
------------------------------------------
It is related to extendible array approach for if ifindex2ifnet global variable which stores pointer to ifnet structure for a particular interface.
After fixed number of interfaces, this ifindex2ifnet array of pointers is malloc for double number of interfaces..

see following line of code from if.c
-----------------------------------------
	if (ifindex2ifnet == 0)
		if_index++;
	else
		while (ifindex2ifnet[ifp->if_index] != NULL) {              <--- issue at this line
			++if_index;
			if (if_index == 0)
				if_index = 1;
			/*
			 * If we hit USHRT_MAX, we skip back to 0 since
			 * there are a number of places where the value
			 * of if_index or if_index itself is compared
			 * to to or stored in an unsigned short.  By
			 * jumping back, we won't botch those assignments
			 * or comparisons.
			 */
			else if (if_index == USHRT_MAX) {
				/*
				 * However, if we have to jump back to
				 * zero *twice* without finding an empty
				 * slot in ifindex2ifnet[], then there
				 * there are too many (>65535) interfaces.
				 */
				if (indexlim++)
					panic("too many interfaces");
				else
					if_index = 1;
			}
			ifp->if_index = if_index;
		}
-----------------------------------------

--> crash occurs at line "while (ifindex2ifnet[ifp->if_index] != NULL) {     "
when ifp->if_index value is 1024 and total number of records malloc are 1024..
--> In this case index of array becomes 1024 while limit should have been from 0 to 1023..
--> Consider iteration when before entering while loop value of ifindex is 1023.After entering if_index is incremented, and ine the end assigned to ifp->if_index, In while statement, not-malloc area is accessed..
--> Actually this should cause problem everytime, when pointer for last ifnet interface is to be updated ..say when array ifindex2ifnet is to be increase dynamically from 8 records to 16 , 16 to 32 and so on..
--> But it gave problem to me only in case of 1024th record ..May be due to some boundary condition related to paging.. (4 k memory = 1024 * 4 bytes)
-->Interesting thing is that this problem occurs rarely under normal scenarios..like if you try to create/delte 10000 interfaces also in any existing netbsd system..it won't give error/crash. But by looking at code this problem seems logical..


Just want you feedback on this issue..If I have missed anything

>How-To-Repeat:
Reproducible under some scenarios only as already explained..
May be related to some boundary condition related to memory..
>Fix:
Solution 
------------
Just add following check before while loop ends..

------------------------------------
	       }

	       ifp->if_index = if_index;
                       /* Change starts */
                        if(ifp->if_index == if_indexlim)
                        {
                                break;
                        }
                       /* Change ends */
}
----------------------------------------

>Release-Note:
>Audit-Trail:
>Unformatted: