panic: ifc_free_unit: bit is already cleared
Andrew Thompson
thompsa at freebsd.org
Sun Oct 9 19:22:10 PDT 2005
On Mon, Oct 10, 2005 at 03:28:49AM +0400, Yar Tikhiy wrote:
> On Thu, Oct 06, 2005 at 10:09:50AM +1300, Andrew Thompson wrote:
> > On Wed, Oct 05, 2005 at 01:55:15PM -0700, Brooks Davis wrote:
> > > On Wed, Oct 05, 2005 at 10:36:39PM +0200, Pawel Jakub Dawidek wrote:
> > > > On Wed, Oct 05, 2005 at 03:49:03PM +1300, Andrew Thompson wrote:
> > > > +> Hi,
> > > > +>
> > > > +> I have found a repeatable panic with network device cloning, unfortunatly I am
> > > > +> unable to dump on this box. This is sparc64 with a 2 day old current.
> > > >
> > > > The order is wrong in vlan_modevent().
> > > >
> > > > if_clone_detach() is freeing ifc_units field, so ifc_free_unit() should not
> > > > be called after that.
> > > >
> > > > This patch should fix the problem:
> > > >
> > > > http://people.freebsd.org/~pjd/patches/if_vlan.c.patch
> > >
> > > Yes. This does introduce a race in that a new interface could
> > > be created between the vlan_clone_destroy loop and the call to
> > > if_clone_detach.
> >
> > I dont think this is the problem. IF_CLONE_REMREF(ifc) is freeing
> > ifc->ifc_units in if_clone_detach(). It look like the ref counting isnt
> > working quite right.
>
> FWIW, I tried to look at the $subject problem since I had had it
> before, but just got a different panic:
>
> Memory modified after free 0xc140b000(4092) val=deadc0dc @ 0xc140b000
> panic: Most recently used by clone
>
> The clone code seems to have decremented something (refcount?) twice
> after freeing the memory chunk.
I have been testing this patch and I think it fixes all the problems
discussed.
It changes refcounting to count the number of cloned interfaces so
ifc_units is only freed when its safe. A new function has been added to
decrement this when a simple cloner module is unloaded. The cloner is
still detached first to prevent the race.
In most cases the change is as simple as:
while ((sc = LIST_FIRST(&gre_softc_list)) != NULL) {
LIST_REMOVE(sc, sc_list);
mtx_unlock(&gre_mtx);
+ ifc_simple_free(&gre_cloner, GRE2IFP(sc));
gre_destroy(sc);
mtx_lock(&gre_mtx);
}
Andrew
-------------- next part --------------
Index: contrib/pf/net/if_pflog.c
===================================================================
RCS file: /home/ncvs/src/sys/contrib/pf/net/if_pflog.c,v
retrieving revision 1.15
diff -u -p -r1.15 if_pflog.c
--- contrib/pf/net/if_pflog.c 9 Aug 2005 11:59:02 -0000 1.15
+++ contrib/pf/net/if_pflog.c 10 Oct 2005 01:56:26 -0000
@@ -360,6 +360,7 @@ static int
pflog_modevent(module_t mod, int type, void *data)
{
int error = 0;
+ struct ifnet *ifp;
switch (type) {
case MOD_LOAD:
@@ -369,8 +370,11 @@ pflog_modevent(module_t mod, int type, v
case MOD_UNLOAD:
if_clone_detach(&pflog_cloner);
- while (!LIST_EMPTY(&pflog_list))
- pflog_clone_destroy(SCP2IFP(LIST_FIRST(&pflog_list)));
+ while (!LIST_EMPTY(&pflog_list)) {
+ ifp = SCP2IFP(LIST_FIRST(&pflog_list));
+ ifc_simple_free(&pflog_cloner, ifp);
+ pflog_clone_destroy(ifp);
+ }
break;
default:
Index: contrib/pf/net/if_pfsync.c
===================================================================
RCS file: /home/ncvs/src/sys/contrib/pf/net/if_pfsync.c,v
retrieving revision 1.23
diff -u -p -r1.23 if_pfsync.c
--- contrib/pf/net/if_pfsync.c 11 Sep 2005 11:55:39 -0000 1.23
+++ contrib/pf/net/if_pfsync.c 10 Oct 2005 01:56:32 -0000
@@ -1842,6 +1842,7 @@ static int
pfsync_modevent(module_t mod, int type, void *data)
{
int error = 0;
+ struct ifnet *ifp;
switch (type) {
case MOD_LOAD:
@@ -1851,9 +1852,11 @@ pfsync_modevent(module_t mod, int type,
case MOD_UNLOAD:
if_clone_detach(&pfsync_cloner);
- while (!LIST_EMPTY(&pfsync_list))
- pfsync_clone_destroy(
- SCP2IFP(LIST_FIRST(&pfsync_list)));
+ while (!LIST_EMPTY(&pfsync_list)) {
+ ifp = SCP2IFP(LIST_FIRST(&pfsync_list));
+ ifc_simple_free(&pfsync_cloner, ifp);
+ pfsync_clone_destroy(ifp);
+ }
break;
default:
Index: net/if_bridge.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_bridge.c,v
retrieving revision 1.23
diff -u -p -r1.23 if_bridge.c
--- net/if_bridge.c 2 Oct 2005 19:15:56 -0000 1.23
+++ net/if_bridge.c 10 Oct 2005 01:56:43 -0000
@@ -356,8 +356,11 @@ bridge_modevent(module_t mod, int type,
break;
case MOD_UNLOAD:
if_clone_detach(&bridge_cloner);
- while (!LIST_EMPTY(&bridge_list))
+ while (!LIST_EMPTY(&bridge_list)) {
+ ifc_simple_free(&bridge_cloner,
+ LIST_FIRST(&bridge_list)->sc_ifp);
bridge_clone_destroy(LIST_FIRST(&bridge_list)->sc_ifp);
+ }
uma_zdestroy(bridge_rtnode_zone);
bridge_input_p = NULL;
bridge_output_p = NULL;
Index: net/if_clone.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_clone.c,v
retrieving revision 1.6
diff -u -p -r1.6 if_clone.c
--- net/if_clone.c 24 Feb 2005 13:14:41 -0000 1.6
+++ net/if_clone.c 10 Oct 2005 01:09:08 -0000
@@ -124,7 +124,6 @@ if_clone_create(char *name, size_t len)
IF_CLONERS_LOCK();
LIST_FOREACH(ifc, &if_cloners, ifc_list) {
if (ifc->ifc_match(ifc, name)) {
- IF_CLONE_ADDREF(ifc);
break;
}
}
@@ -134,7 +133,6 @@ if_clone_create(char *name, size_t len)
return (EINVAL);
err = (*ifc->ifc_create)(ifc, name, len);
- IF_CLONE_REMREF(ifc);
return (err);
}
@@ -156,7 +154,6 @@ if_clone_destroy(const char *name)
IF_CLONERS_LOCK();
LIST_FOREACH(ifc, &if_cloners, ifc_list) {
if (strcmp(ifc->ifc_name, ifp->if_dname) == 0) {
- IF_CLONE_ADDREF(ifc);
break;
}
}
@@ -172,7 +169,6 @@ if_clone_destroy(const char *name)
err = (*ifc->ifc_destroy)(ifc, ifp);
done:
- IF_CLONE_REMREF(ifc);
return (err);
}
@@ -352,7 +348,10 @@ ifc_alloc_unit(struct if_clone *ifc, int
/*
* Allocate the unit in the bitmap.
*/
+ KASSERT((ifc->ifc_units[bytoff] & (1 << bitoff)) == 0,
+ ("%s: bit is already set", __func__));
ifc->ifc_units[bytoff] |= (1 << bitoff);
+ IF_CLONE_ADDREF_LOCKED(ifc);
done:
IF_CLONE_UNLOCK(ifc);
@@ -375,7 +374,7 @@ ifc_free_unit(struct if_clone *ifc, int
KASSERT((ifc->ifc_units[bytoff] & (1 << bitoff)) != 0,
("%s: bit is already cleared", __func__));
ifc->ifc_units[bytoff] &= ~(1 << bitoff);
- IF_CLONE_UNLOCK(ifc);
+ IF_CLONE_REMREF_LOCKED(ifc); /* releases lock */
}
void
@@ -477,6 +476,22 @@ ifc_simple_destroy(struct if_clone *ifc,
ifcs->ifcs_destroy(ifp);
+ ifc_simple_free(ifc, ifp);
+
+ return (0);
+}
+
+int
+ifc_simple_free(struct if_clone *ifc, struct ifnet *ifp)
+{
+ int unit;
+ struct ifc_simple_data *ifcs = ifc->ifc_data;
+
+ unit = ifp->if_dunit;
+
+ if (unit < ifcs->ifcs_minifs)
+ return (EINVAL);
+
ifc_free_unit(ifc, unit);
return (0);
Index: net/if_clone.h
===================================================================
RCS file: /home/ncvs/src/sys/net/if_clone.h,v
retrieving revision 1.2
diff -u -p -r1.2 if_clone.h
--- net/if_clone.h 7 Jan 2005 01:45:34 -0000 1.2
+++ net/if_clone.h 9 Oct 2005 23:50:05 -0000
@@ -107,6 +107,7 @@ void ifc_simple_attach(struct if_clone *
int ifc_simple_match(struct if_clone *, const char *);
int ifc_simple_create(struct if_clone *, char *, size_t);
int ifc_simple_destroy(struct if_clone *, struct ifnet *);
+int ifc_simple_free(struct if_clone *, struct ifnet *);
#endif /* _KERNEL */
Index: net/if_disc.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_disc.c,v
retrieving revision 1.48
diff -u -p -r1.48 if_disc.c
--- net/if_disc.c 26 Jun 2005 18:11:10 -0000 1.48
+++ net/if_disc.c 10 Oct 2005 01:57:12 -0000
@@ -152,6 +152,7 @@ disc_modevent(module_t mod, int type, vo
while ((sc = LIST_FIRST(&disc_softc_list)) != NULL) {
LIST_REMOVE(sc, sc_list);
mtx_unlock(&disc_mtx);
+ ifc_simple_free(&disc_cloner, sc->sc_ifp);
disc_destroy(sc);
mtx_lock(&disc_mtx);
}
Index: net/if_faith.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_faith.c,v
retrieving revision 1.37
diff -u -p -r1.37 if_faith.c
--- net/if_faith.c 9 Aug 2005 10:19:58 -0000 1.37
+++ net/if_faith.c 10 Oct 2005 01:57:18 -0000
@@ -139,6 +139,7 @@ faithmodevent(mod, type, data)
while ((sc = LIST_FIRST(&faith_softc_list)) != NULL) {
LIST_REMOVE(sc, sc_list);
mtx_unlock(&faith_mtx);
+ ifc_simple_free(&faith_cloner, sc->sc_ifp);
faith_destroy(sc);
mtx_lock(&faith_mtx);
}
Index: net/if_gif.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_gif.c,v
retrieving revision 1.54
diff -u -p -r1.54 if_gif.c
--- net/if_gif.c 9 Aug 2005 10:19:58 -0000 1.54
+++ net/if_gif.c 10 Oct 2005 01:57:23 -0000
@@ -252,6 +252,7 @@ gifmodevent(mod, type, data)
while ((sc = LIST_FIRST(&gif_softc_list)) != NULL) {
LIST_REMOVE(sc, gif_list);
mtx_unlock(&gif_mtx);
+ ifc_simple_free(&gif_cloner, GIF2IFP(sc));
gif_destroy(sc);
mtx_lock(&gif_mtx);
}
Index: net/if_gre.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_gre.c,v
retrieving revision 1.34
diff -u -p -r1.34 if_gre.c
--- net/if_gre.c 9 Aug 2005 10:19:58 -0000 1.34
+++ net/if_gre.c 10 Oct 2005 01:57:31 -0000
@@ -793,6 +793,7 @@ gremodevent(module_t mod, int type, void
while ((sc = LIST_FIRST(&gre_softc_list)) != NULL) {
LIST_REMOVE(sc, sc_list);
mtx_unlock(&gre_mtx);
+ ifc_simple_free(&gre_cloner, GRE2IFP(sc));
gre_destroy(sc);
mtx_lock(&gre_mtx);
}
Index: net/if_ppp.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_ppp.c,v
retrieving revision 1.108
diff -u -p -r1.108 if_ppp.c
--- net/if_ppp.c 19 Sep 2005 22:27:07 -0000 1.108
+++ net/if_ppp.c 10 Oct 2005 01:57:38 -0000
@@ -298,6 +298,7 @@ ppp_modevent(module_t mod, int type, voi
while ((sc = LIST_FIRST(&ppp_softc_list)) != NULL) {
LIST_REMOVE(sc, sc_list);
PPP_LIST_UNLOCK();
+ ifc_simple_free(&ppp_cloner, PPP2IFP(sc));
ppp_destroy(sc);
PPP_LIST_LOCK();
}
Index: netinet/ip_carp.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.31
diff -u -p -r1.31 ip_carp.c
--- netinet/ip_carp.c 9 Sep 2005 08:41:39 -0000 1.31
+++ netinet/ip_carp.c 10 Oct 2005 01:57:52 -0000
@@ -2133,6 +2133,7 @@ static int
carp_modevent(module_t mod, int type, void *data)
{
int error = 0;
+ struct ifnet *ifp;
switch (type) {
case MOD_LOAD:
@@ -2143,8 +2144,11 @@ carp_modevent(module_t mod, int type, vo
case MOD_UNLOAD:
if_clone_detach(&carp_cloner);
- while (!LIST_EMPTY(&carpif_list))
- carp_clone_destroy(SC2IFP(LIST_FIRST(&carpif_list)));
+ while (!LIST_EMPTY(&carpif_list)) {
+ ifp = SC2IFP(LIST_FIRST(&carpif_list));
+ ifc_simple_free(&carp_cloner, ifp);
+ carp_clone_destroy(ifp);
+ }
mtx_destroy(&carp_mtx);
break;
More information about the freebsd-current
mailing list