panic: ifc_free_unit: bit is already cleared
Andrew Thompson
thompsa at freebsd.org
Tue Oct 11 14:06:09 PDT 2005
On Mon, Oct 10, 2005 at 01:29:00PM -0700, Brooks Davis wrote:
> On Mon, Oct 10, 2005 at 03:22:08PM +1300, Andrew Thompson wrote:
> > On Mon, Oct 10, 2005 at 03:28:49AM +0400, Yar Tikhiy wrote:
> > > 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:
>
> I don't see any reason why you can't just replace the specific destroy
> calls with calls to ifc_simple_destroy(). That would avoid expanding
> the API.
I have updated the patch and yes, its a nicer way to do it. Please
review.
Ive run through interations of create/kldunload with bridge, disc,
faith, gif, gre and ppp with extra printf's and its freeing correctly.
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 11 Oct 2005 19:19:06 -0000
@@ -370,7 +370,8 @@ 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)));
+ ifc_simple_destroy(&pflog_cloner,
+ SCP2IFP(LIST_FIRST(&pflog_list)));
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 11 Oct 2005 19:19:15 -0000
@@ -1852,7 +1852,7 @@ pfsync_modevent(module_t mod, int type,
case MOD_UNLOAD:
if_clone_detach(&pfsync_cloner);
while (!LIST_EMPTY(&pfsync_list))
- pfsync_clone_destroy(
+ ifc_simple_destroy(&pfsync_cloner,
SCP2IFP(LIST_FIRST(&pfsync_list)));
break;
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 11 Oct 2005 19:16:18 -0000
@@ -357,7 +357,8 @@ bridge_modevent(module_t mod, int type,
case MOD_UNLOAD:
if_clone_detach(&bridge_cloner);
while (!LIST_EMPTY(&bridge_list))
- bridge_clone_destroy(LIST_FIRST(&bridge_list)->sc_ifp);
+ ifc_simple_destroy(&bridge_cloner,
+ 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 11 Oct 2005 09:47:35 -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);
}
@@ -224,6 +220,10 @@ if_clone_detach(struct if_clone *ifc)
static void
if_clone_free(struct if_clone *ifc)
{
+ for (int bytoff = 0; bytoff < ifc->ifc_bmlen; bytoff++) {
+ KASSERT(ifc->ifc_units[bytoff] == 0x00,
+ ("ifc_units[%d] is not empty", bytoff));
+ }
IF_CLONE_LOCK_DESTROY(ifc);
free(ifc->ifc_units, M_CLONE);
@@ -352,7 +352,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 +378,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
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 11 Oct 2005 19:17:03 -0000
@@ -111,17 +111,6 @@ disc_clone_create(struct if_clone *ifc,
}
static void
-disc_destroy(struct disc_softc *sc)
-{
-
- bpfdetach(sc->sc_ifp);
- if_detach(sc->sc_ifp);
- if_free(sc->sc_ifp);
-
- free(sc, M_DISC);
-}
-
-static void
disc_clone_destroy(struct ifnet *ifp)
{
struct disc_softc *sc;
@@ -131,7 +120,11 @@ disc_clone_destroy(struct ifnet *ifp)
LIST_REMOVE(sc, sc_list);
mtx_unlock(&disc_mtx);
- disc_destroy(sc);
+ bpfdetach(ifp);
+ if_detach(ifp);
+ if_free(ifp);
+
+ free(sc, M_DISC);
}
static int
@@ -150,9 +143,8 @@ disc_modevent(module_t mod, int type, vo
mtx_lock(&disc_mtx);
while ((sc = LIST_FIRST(&disc_softc_list)) != NULL) {
- LIST_REMOVE(sc, sc_list);
mtx_unlock(&disc_mtx);
- disc_destroy(sc);
+ ifc_simple_destroy(&disc_cloner, sc->sc_ifp);
mtx_lock(&disc_mtx);
}
mtx_unlock(&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 11 Oct 2005 19:17:45 -0000
@@ -103,7 +103,6 @@ static LIST_HEAD(, faith_softc) faith_so
static int faith_clone_create(struct if_clone *, int);
static void faith_clone_destroy(struct ifnet *);
-static void faith_destroy(struct faith_softc *);
IFC_SIMPLE_DECLARE(faith, 0);
@@ -137,9 +136,8 @@ faithmodevent(mod, type, data)
mtx_lock(&faith_mtx);
while ((sc = LIST_FIRST(&faith_softc_list)) != NULL) {
- LIST_REMOVE(sc, sc_list);
mtx_unlock(&faith_mtx);
- faith_destroy(sc);
+ ifc_simple_destroy(&faith_cloner, sc->sc_ifp);
mtx_lock(&faith_mtx);
}
mtx_unlock(&faith_mtx);
@@ -195,16 +193,6 @@ faith_clone_create(ifc, unit)
}
static void
-faith_destroy(struct faith_softc *sc)
-{
-
- bpfdetach(sc->sc_ifp);
- if_detach(sc->sc_ifp);
- if_free(sc->sc_ifp);
- free(sc, M_FAITH);
-}
-
-static void
faith_clone_destroy(ifp)
struct ifnet *ifp;
{
@@ -214,7 +202,10 @@ faith_clone_destroy(ifp)
LIST_REMOVE(sc, sc_list);
mtx_unlock(&faith_mtx);
- faith_destroy(sc);
+ bpfdetach(ifp);
+ if_detach(ifp);
+ if_free(ifp);
+ free(sc, M_FAITH);
}
int
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 11 Oct 2005 19:17:56 -0000
@@ -186,10 +186,15 @@ gifattach0(sc)
}
static void
-gif_destroy(struct gif_softc *sc)
+gif_clone_destroy(ifp)
+ struct ifnet *ifp;
{
- struct ifnet *ifp = GIF2IFP(sc);
int err;
+ struct gif_softc *sc = ifp->if_softc;
+
+ mtx_lock(&gif_mtx);
+ LIST_REMOVE(sc, gif_list);
+ mtx_unlock(&gif_mtx);
gif_delete_tunnel(ifp);
#ifdef INET6
@@ -214,18 +219,6 @@ gif_destroy(struct gif_softc *sc)
free(sc, M_GIF);
}
-static void
-gif_clone_destroy(ifp)
- struct ifnet *ifp;
-{
- struct gif_softc *sc = ifp->if_softc;
-
- mtx_lock(&gif_mtx);
- LIST_REMOVE(sc, gif_list);
- mtx_unlock(&gif_mtx);
- gif_destroy(sc);
-}
-
static int
gifmodevent(mod, type, data)
module_t mod;
@@ -250,9 +243,8 @@ gifmodevent(mod, type, data)
mtx_lock(&gif_mtx);
while ((sc = LIST_FIRST(&gif_softc_list)) != NULL) {
- LIST_REMOVE(sc, gif_list);
mtx_unlock(&gif_mtx);
- gif_destroy(sc);
+ ifc_simple_destroy(&gif_cloner, GIF2IFP(sc));
mtx_lock(&gif_mtx);
}
mtx_unlock(&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 11 Oct 2005 19:18:08 -0000
@@ -202,20 +202,6 @@ gre_clone_create(ifc, unit)
}
static void
-gre_destroy(struct gre_softc *sc)
-{
-
-#ifdef INET
- if (sc->encap != NULL)
- encap_detach(sc->encap);
-#endif
- bpfdetach(GRE2IFP(sc));
- if_detach(GRE2IFP(sc));
- if_free(GRE2IFP(sc));
- free(sc, M_GRE);
-}
-
-static void
gre_clone_destroy(ifp)
struct ifnet *ifp;
{
@@ -224,7 +210,15 @@ gre_clone_destroy(ifp)
mtx_lock(&gre_mtx);
LIST_REMOVE(sc, sc_list);
mtx_unlock(&gre_mtx);
- gre_destroy(sc);
+
+#ifdef INET
+ if (sc->encap != NULL)
+ encap_detach(sc->encap);
+#endif
+ bpfdetach(ifp);
+ if_detach(ifp);
+ if_free(ifp);
+ free(sc, M_GRE);
}
/*
@@ -791,9 +785,8 @@ gremodevent(module_t mod, int type, void
mtx_lock(&gre_mtx);
while ((sc = LIST_FIRST(&gre_softc_list)) != NULL) {
- LIST_REMOVE(sc, sc_list);
mtx_unlock(&gre_mtx);
- gre_destroy(sc);
+ ifc_simple_destroy(&gre_cloner, GRE2IFP(sc));
mtx_lock(&gre_mtx);
}
mtx_unlock(&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 11 Oct 2005 19:18:23 -0000
@@ -242,11 +242,15 @@ ppp_clone_create(struct if_clone *ifc, i
}
static void
-ppp_destroy(struct ppp_softc *sc)
+ppp_clone_destroy(struct ifnet *ifp)
{
- struct ifnet *ifp;
+ struct ppp_softc *sc;
+
+ sc = ifp->if_softc;
+ PPP_LIST_LOCK();
+ LIST_REMOVE(sc, sc_list);
+ PPP_LIST_UNLOCK();
- ifp = PPP2IFP(sc);
bpfdetach(ifp);
if_detach(ifp);
if_free(ifp);
@@ -256,18 +260,6 @@ ppp_destroy(struct ppp_softc *sc)
free(sc, M_PPP);
}
-static void
-ppp_clone_destroy(struct ifnet *ifp)
-{
- struct ppp_softc *sc;
-
- sc = ifp->if_softc;
- PPP_LIST_LOCK();
- LIST_REMOVE(sc, sc_list);
- PPP_LIST_UNLOCK();
- ppp_destroy(sc);
-}
-
static int
ppp_modevent(module_t mod, int type, void *data)
{
@@ -296,9 +288,8 @@ ppp_modevent(module_t mod, int type, voi
PPP_LIST_LOCK();
while ((sc = LIST_FIRST(&ppp_softc_list)) != NULL) {
- LIST_REMOVE(sc, sc_list);
PPP_LIST_UNLOCK();
- ppp_destroy(sc);
+ ifc_simple_destroy(&ppp_cloner, PPP2IFP(sc));
PPP_LIST_LOCK();
}
PPP_LIST_LOCK_DESTROY();
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 11 Oct 2005 19:19:36 -0000
@@ -2144,7 +2144,8 @@ 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)));
+ ifc_simple_destroy(&carp_cloner,
+ SC2IFP(LIST_FIRST(&carpif_list)));
mtx_destroy(&carp_mtx);
break;
More information about the freebsd-current
mailing list