diff options
author | thompsa <thompsa@FreeBSD.org> | 2005-10-12 19:52:16 +0000 |
---|---|---|
committer | thompsa <thompsa@FreeBSD.org> | 2005-10-12 19:52:16 +0000 |
commit | d6130a47038460d2093bdc0fcdadd546393cc823 (patch) | |
tree | e685414c2d4a9a0e96a99f9d18fa6ce3780a6fbe | |
parent | 1917bf7b66cadbb6cbcc022f50fb4252b6996ff7 (diff) | |
download | FreeBSD-src-d6130a47038460d2093bdc0fcdadd546393cc823.zip FreeBSD-src-d6130a47038460d2093bdc0fcdadd546393cc823.tar.gz |
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
-rw-r--r-- | sys/contrib/pf/net/if_pflog.c | 3 | ||||
-rw-r--r-- | sys/contrib/pf/net/if_pfsync.c | 2 | ||||
-rw-r--r-- | sys/net/if_bridge.c | 3 | ||||
-rw-r--r-- | sys/net/if_clone.c | 18 | ||||
-rw-r--r-- | sys/net/if_disc.c | 20 | ||||
-rw-r--r-- | sys/net/if_faith.c | 19 | ||||
-rw-r--r-- | sys/net/if_gif.c | 24 | ||||
-rw-r--r-- | sys/net/if_gre.c | 27 | ||||
-rw-r--r-- | sys/net/if_ppp.c | 25 | ||||
-rw-r--r-- | sys/net/if_stf.c | 26 | ||||
-rw-r--r-- | sys/netinet/ip_carp.c | 3 |
11 files changed, 66 insertions, 104 deletions
diff --git a/sys/contrib/pf/net/if_pflog.c b/sys/contrib/pf/net/if_pflog.c index 1fd6a82..2497a94 100644 --- a/sys/contrib/pf/net/if_pflog.c +++ b/sys/contrib/pf/net/if_pflog.c @@ -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: diff --git a/sys/contrib/pf/net/if_pfsync.c b/sys/contrib/pf/net/if_pfsync.c index 971eea7..5f0e8a6 100644 --- a/sys/contrib/pf/net/if_pfsync.c +++ b/sys/contrib/pf/net/if_pfsync.c @@ -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; diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c index 348c5fc..8e41e0d 100644 --- a/sys/net/if_bridge.c +++ b/sys/net/if_bridge.c @@ -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; diff --git a/sys/net/if_clone.c b/sys/net/if_clone.c index 7dd3d59..746f08f 100644 --- a/sys/net/if_clone.c +++ b/sys/net/if_clone.c @@ -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 diff --git a/sys/net/if_disc.c b/sys/net/if_disc.c index d94eeeb..3b0804e 100644 --- a/sys/net/if_disc.c +++ b/sys/net/if_disc.c @@ -111,17 +111,6 @@ disc_clone_create(struct if_clone *ifc, int unit) } 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, 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); diff --git a/sys/net/if_faith.c b/sys/net/if_faith.c index d71a3fa..677e1b2 100644 --- a/sys/net/if_faith.c +++ b/sys/net/if_faith.c @@ -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); @@ -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 diff --git a/sys/net/if_gif.c b/sys/net/if_gif.c index 7b01bbb..da611cc 100644 --- a/sys/net/if_gif.c +++ b/sys/net/if_gif.c @@ -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); diff --git a/sys/net/if_gre.c b/sys/net/if_gre.c index aa57526..20a5b90 100644 --- a/sys/net/if_gre.c +++ b/sys/net/if_gre.c @@ -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 *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); diff --git a/sys/net/if_ppp.c b/sys/net/if_ppp.c index a4bb60c..45915c6 100644 --- a/sys/net/if_ppp.c +++ b/sys/net/if_ppp.c @@ -242,11 +242,15 @@ ppp_clone_create(struct if_clone *ifc, int unit) } 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, 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(); diff --git a/sys/net/if_stf.c b/sys/net/if_stf.c index 3ba3343..e779b17 100644 --- a/sys/net/if_stf.c +++ b/sys/net/if_stf.c @@ -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); diff --git a/sys/netinet/ip_carp.c b/sys/netinet/ip_carp.c index 7dd72c1..0e55543 100644 --- a/sys/netinet/ip_carp.c +++ b/sys/netinet/ip_carp.c @@ -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; |