d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

When removing ifnets, we should first remove the reference to ifnet

Browse Source 
from the interface index, then decrease refcount, not vice versa.

Otherwise there is a race (reproducible) when if_free_internal()
contests on IFNET_WLOCK(), and we got a zero-refed ifnet in the
index for a long time. It may be picked by some other thread,
that runs ifnet_byindex_ref(), who takes the ifnet from index,
and bumps refcount. When reader drops the lock, if_free_internal()
proceeds with free. Then reader tries to free it a second time.
This commit is contained in:
glebius 2011-04-04 07:45:08 +00:00
parent 44e946c1dd
commit 8c3b1b83f9
1 changed files with 10 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sys/net/if.c
View File

@ -465,8 +465,8 @@ if_alloc(u_char type)
}
/*
* Do the actual work of freeing a struct ifnet, associated index, and layer
* 2 common structure. This call is made when the last reference to an
* Do the actual work of freeing a struct ifnet, and layer 2 common
* structure. This call is made when the last reference to an
* interface is released.
*/
static void
@ -476,13 +476,6 @@ if_free_internal(struct ifnet *ifp)
KASSERT((ifp->if_flags & IFF_DYING),
("if_free_internal: interface not dying"));
IFNET_WLOCK();
KASSERT(ifp == ifnet_byindex_locked(ifp->if_index),
("%s: freeing unallocated ifnet", ifp->if_xname));
ifindex_free_locked(ifp->if_index);
IFNET_WUNLOCK();
if (if_com_free[ifp->if_alloctype] != NULL)
if_com_free[ifp->if_alloctype](ifp->if_l2com,
ifp->if_alloctype);
@ -513,6 +506,14 @@ if_free_type(struct ifnet *ifp, u_char type)
ifp->if_alloctype));
ifp->if_flags |= IFF_DYING; /* XXX: Locking */
IFNET_WLOCK();
KASSERT(ifp == ifnet_byindex_locked(ifp->if_index),
("%s: freeing unallocated ifnet", ifp->if_xname));
ifindex_free_locked(ifp->if_index);
IFNET_WUNLOCK();
if (!refcount_release(&ifp->if_refcount))
return;
if_free_internal(ifp);