Change the reference counting to count the number of cloned interfaces for each

cloner. This ensures that ifc->ifc_units is not prematurely freed in
if_clone_detach() before the clones are destroyed, resulting in memory modified
after free. This could be triggered with if_vlan.

Assert that all cloners have been destroyed when freeing the memory.

Change all simple cloners to destroy their clones with ifc_simple_destroy() on
module unload so the reference count is properly updated. This also cleans up
the interface destroy routines and allows future optimisation.

Discussed with:	brooks, pjd, -current
Reviewed by:	brooks
This commit is contained in:
Andrew Thompson 2005-10-12 19:52:16 +00:00
parent 680d937a4b
commit febd0759f3
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=151266
11 changed files with 67 additions and 105 deletions

View file

@ -370,7 +370,8 @@ pflog_modevent(module_t mod, int type, void *data)
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:

View file

@ -1852,7 +1852,7 @@ pfsync_modevent(module_t mod, int type, void *data)
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;

View file

@ -357,7 +357,8 @@ bridge_modevent(module_t mod, int type, void *data)
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;

View file

@ -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);
}
@ -212,18 +208,27 @@ if_clone_attach(struct if_clone *ifc)
void
if_clone_detach(struct if_clone *ifc)
{
struct ifc_simple_data *ifcs = ifc->ifc_data;
IF_CLONERS_LOCK();
LIST_REMOVE(ifc, ifc_list);
if_cloners_count--;
IF_CLONERS_UNLOCK();
/* Allow all simples to be destroyed */
if (ifc->ifc_attach == ifc_simple_attach)
ifcs->ifcs_minifs = 0;
IF_CLONE_REMREF(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 +357,10 @@ ifc_alloc_unit(struct if_clone *ifc, int *unit)
/*
* 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 +383,7 @@ ifc_free_unit(struct if_clone *ifc, int unit)
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

View file

@ -110,17 +110,6 @@ disc_clone_create(struct if_clone *ifc, int unit)
return (0);
}
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)
{
@ -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, void *data)
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);

View file

@ -103,7 +103,6 @@ static LIST_HEAD(, faith_softc) faith_softc_list;
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);
@ -194,16 +192,6 @@ faith_clone_create(ifc, unit)
return (0);
}
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

View file

@ -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);

View file

@ -201,20 +201,6 @@ gre_clone_create(ifc, unit)
return (0);
}
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 *data)
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);

View file

@ -241,21 +241,6 @@ ppp_clone_create(struct if_clone *ifc, int unit)
return (0);
}
static void
ppp_destroy(struct ppp_softc *sc)
{
struct ifnet *ifp;
ifp = PPP2IFP(sc);
bpfdetach(ifp);
if_detach(ifp);
if_free(ifp);
mtx_destroy(&sc->sc_rawq.ifq_mtx);
mtx_destroy(&sc->sc_fastq.ifq_mtx);
mtx_destroy(&sc->sc_inq.ifq_mtx);
free(sc, M_PPP);
}
static void
ppp_clone_destroy(struct ifnet *ifp)
{
@ -265,7 +250,14 @@ ppp_clone_destroy(struct ifnet *ifp)
PPP_LIST_LOCK();
LIST_REMOVE(sc, sc_list);
PPP_LIST_UNLOCK();
ppp_destroy(sc);
bpfdetach(ifp);
if_detach(ifp);
if_free(ifp);
mtx_destroy(&sc->sc_rawq.ifq_mtx);
mtx_destroy(&sc->sc_fastq.ifq_mtx);
mtx_destroy(&sc->sc_inq.ifq_mtx);
free(sc, M_PPP);
}
static int
@ -296,9 +288,8 @@ ppp_modevent(module_t mod, int type, void *data)
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();

View file

@ -252,30 +252,23 @@ stf_clone_create(struct if_clone *ifc, char *name, size_t len)
return (0);
}
static void
stf_destroy(struct stf_softc *sc)
{
int err;
err = encap_detach(sc->encap_cookie);
KASSERT(err == 0, ("Unexpected error detaching encap_cookie"));
bpfdetach(STF2IFP(sc));
if_detach(STF2IFP(sc));
if_free(STF2IFP(sc));
free(sc, M_STF);
}
static int
stf_clone_destroy(struct if_clone *ifc, struct ifnet *ifp)
{
struct stf_softc *sc = ifp->if_softc;
int err;
mtx_lock(&stf_mtx);
LIST_REMOVE(sc, sc_list);
mtx_unlock(&stf_mtx);
stf_destroy(sc);
err = encap_detach(sc->encap_cookie);
KASSERT(err == 0, ("Unexpected error detaching encap_cookie"));
bpfdetach(ifp);
if_detach(ifp);
if_free(ifp);
free(sc, M_STF);
ifc_free_unit(ifc, STFUNIT);
return (0);
@ -301,9 +294,8 @@ stfmodevent(mod, type, data)
mtx_lock(&stf_mtx);
while ((sc = LIST_FIRST(&stf_softc_list)) != NULL) {
LIST_REMOVE(sc, sc_list);
mtx_unlock(&stf_mtx);
stf_destroy(sc);
stf_clone_destroy(&stf_cloner, STF2IFP(sc));
mtx_lock(&stf_mtx);
}
mtx_unlock(&stf_mtx);

View file

@ -2144,7 +2144,8 @@ carp_modevent(module_t mod, int type, void *data)
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;