svn commit: r302033 - projects/vnet/sys/netpfil/pf
Bjoern A. Zeeb
bz at FreeBSD.org
Mon Jun 20 22:06:00 UTC 2016
Author: bz
Date: Mon Jun 20 22:05:59 2016
New Revision: 302033
URL: https://svnweb.freebsd.org/changeset/base/302033
Log:
Free counters after the other things as we still update them during
teardown; this avoids memory modified after free panics.
Add missing locks around a few places to avoid race conditions.
Set some cached 'kif' values to NULL before freeing it so we will
not try to access freed memory anymore.
Add V_pf_vnet_active in addition to hooked and running to indicate
the state of the VNET. We need a dedicated flag mostly for external
events.
Add various checks for kif being valid, and V_pf_vnet_active being
true on global external events to avoid running code for VNETs
shutting down.
Having some simple ruleset with state and a hundred or so connections
this seems getting stable now.
Sponsored by: The FreeBSD Foundation
Modified:
projects/vnet/sys/netpfil/pf/pf_if.c
projects/vnet/sys/netpfil/pf/pf_ioctl.c
Modified: projects/vnet/sys/netpfil/pf/pf_if.c
==============================================================================
--- projects/vnet/sys/netpfil/pf/pf_if.c Mon Jun 20 19:00:47 2016 (r302032)
+++ projects/vnet/sys/netpfil/pf/pf_if.c Mon Jun 20 22:05:59 2016 (r302033)
@@ -58,6 +58,9 @@ static VNET_DEFINE(long, pfi_update);
#define V_pfi_update VNET(pfi_update)
#define PFI_BUFFER_MAX 0x10000
+VNET_DECLARE(int, pf_vnet_active);
+#define V_pf_vnet_active VNET(pf_vnet_active)
+
static VNET_DEFINE(struct pfr_addr *, pfi_buffer);
static VNET_DEFINE(int, pfi_buffer_cnt);
static VNET_DEFINE(int, pfi_buffer_max);
@@ -152,18 +155,26 @@ pfi_initialize(void)
void
pfi_cleanup_vnet(void)
{
- struct pfi_kif *p;
+ struct pfi_kif *kif;
+
+ PF_RULES_WASSERT();
V_pfi_all = NULL;
- while ((p = RB_MIN(pfi_ifhead, &V_pfi_ifs))) {
- RB_REMOVE(pfi_ifhead, &V_pfi_ifs, p);
- free(p, PFI_MTYPE);
+ while ((kif = RB_MIN(pfi_ifhead, &V_pfi_ifs))) {
+ RB_REMOVE(pfi_ifhead, &V_pfi_ifs, kif);
+ if (kif->pfik_group)
+ kif->pfik_group->ifg_pf_kif = NULL;
+ if (kif->pfik_ifp)
+ kif->pfik_ifp->if_pf_kif = NULL;
+ free(kif, PFI_MTYPE);
}
- while ((p = LIST_FIRST(&V_pfi_unlinked_kifs))) {
- LIST_REMOVE(p, pfik_list);
- free(p, PFI_MTYPE);
+ mtx_lock(&pfi_unlnkdkifs_mtx);
+ while ((kif = LIST_FIRST(&V_pfi_unlinked_kifs))) {
+ LIST_REMOVE(kif, pfik_list);
+ free(kif, PFI_MTYPE);
}
+ mtx_unlock(&pfi_unlnkdkifs_mtx);
free(V_pfi_buffer, PFI_MTYPE);
}
@@ -678,7 +689,7 @@ pfi_update_status(const char *name, stru
bzero(pfs->bcounters, sizeof(pfs->bcounters));
}
TAILQ_FOREACH(ifgm, &ifg_members, ifgm_next) {
- if (ifgm->ifgm_ifp == NULL)
+ if (ifgm->ifgm_ifp == NULL || ifgm->ifgm_ifp->if_pf_kif == NULL)
continue;
p = (struct pfi_kif *)ifgm->ifgm_ifp->if_pf_kif;
@@ -790,6 +801,11 @@ pfi_attach_ifnet_event(void *arg __unuse
{
CURVNET_SET(ifp->if_vnet);
+ if (V_pf_vnet_active == 0) {
+ /* Avoid teardown race in the least expensie way. */
+ CURVNET_RESTORE();
+ return;
+ }
pfi_attach_ifnet(ifp);
#ifdef ALTQ
PF_RULES_WLOCK();
@@ -804,7 +820,15 @@ pfi_detach_ifnet_event(void *arg __unuse
{
struct pfi_kif *kif = (struct pfi_kif *)ifp->if_pf_kif;
+ if (kif == NULL)
+ return;
+
CURVNET_SET(ifp->if_vnet);
+ if (V_pf_vnet_active == 0) {
+ /* Avoid teardown race in the least expensie way. */
+ CURVNET_RESTORE();
+ return;
+ }
PF_RULES_WLOCK();
V_pfi_update++;
pfi_kif_update(kif);
@@ -823,6 +847,11 @@ pfi_attach_group_event(void *arg , struc
{
CURVNET_SET((struct vnet *)arg);
+ if (V_pf_vnet_active == 0) {
+ /* Avoid teardown race in the least expensie way. */
+ CURVNET_RESTORE();
+ return;
+ }
pfi_attach_ifgroup(ifg);
CURVNET_RESTORE();
}
@@ -832,9 +861,14 @@ pfi_change_group_event(void *arg, char *
{
struct pfi_kif *kif;
- kif = malloc(sizeof(*kif), PFI_MTYPE, M_WAITOK);
-
CURVNET_SET((struct vnet *)arg);
+ if (V_pf_vnet_active == 0) {
+ /* Avoid teardown race in the least expensie way. */
+ CURVNET_RESTORE();
+ return;
+ }
+
+ kif = malloc(sizeof(*kif), PFI_MTYPE, M_WAITOK);
PF_RULES_WLOCK();
V_pfi_update++;
kif = pfi_kif_attach(kif, gname);
@@ -848,7 +882,15 @@ pfi_detach_group_event(void *arg, struct
{
struct pfi_kif *kif = (struct pfi_kif *)ifg->ifg_pf_kif;
+ if (kif == NULL)
+ return;
+
CURVNET_SET((struct vnet *)arg);
+ if (V_pf_vnet_active == 0) {
+ /* Avoid teardown race in the least expensie way. */
+ CURVNET_RESTORE();
+ return;
+ }
PF_RULES_WLOCK();
V_pfi_update++;
@@ -861,8 +903,15 @@ pfi_detach_group_event(void *arg, struct
static void
pfi_ifaddr_event(void *arg __unused, struct ifnet *ifp)
{
+ if (ifp->if_pf_kif == NULL)
+ return;
CURVNET_SET(ifp->if_vnet);
+ if (V_pf_vnet_active == 0) {
+ /* Avoid teardown race in the least expensie way. */
+ CURVNET_RESTORE();
+ return;
+ }
PF_RULES_WLOCK();
if (ifp && ifp->if_pf_kif) {
V_pfi_update++;
Modified: projects/vnet/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- projects/vnet/sys/netpfil/pf/pf_ioctl.c Mon Jun 20 19:00:47 2016 (r302032)
+++ projects/vnet/sys/netpfil/pf/pf_ioctl.c Mon Jun 20 22:05:59 2016 (r302033)
@@ -188,6 +188,15 @@ static struct cdevsw pf_cdevsw = {
static volatile VNET_DEFINE(int, pf_pfil_hooked);
#define V_pf_pfil_hooked VNET(pf_pfil_hooked)
+
+/*
+ * We need a flag that is neither hooked nor running to know when
+ * the VNET is "valid". We primarily need this to control (global)
+ * external event, e.g., eventhandlers.
+ */
+VNET_DEFINE(int, pf_vnet_active);
+#define V_pf_vnet_active VNET(pf_vnet_active)
+
int pf_end_threads;
struct rwlock pf_rules_lock;
@@ -3465,21 +3474,6 @@ shutdown_pf(void)
u_int32_t t[5];
char nn = '\0';
- V_pf_status.running = 0;
-
- counter_u64_free(V_pf_default_rule.states_cur);
- counter_u64_free(V_pf_default_rule.states_tot);
- counter_u64_free(V_pf_default_rule.src_nodes);
-
- for (int i = 0; i < PFRES_MAX; i++)
- counter_u64_free(V_pf_status.counters[i]);
- for (int i = 0; i < LCNT_MAX; i++)
- counter_u64_free(V_pf_status.lcounters[i]);
- for (int i = 0; i < FCNT_MAX; i++)
- counter_u64_free(V_pf_status.fcounters[i]);
- for (int i = 0; i < SCNT_MAX; i++)
- counter_u64_free(V_pf_status.scounters[i]);
-
do {
if ((error = pf_begin_rules(&t[0], PF_RULESET_SCRUB, &nn))
!= 0) {
@@ -3531,6 +3525,20 @@ shutdown_pf(void)
/* status does not use malloced mem so no need to cleanup */
/* fingerprints and interfaces have their own cleanup code */
+
+ /* Free counters last as we updated them during shutdown. */
+ counter_u64_free(V_pf_default_rule.states_cur);
+ counter_u64_free(V_pf_default_rule.states_tot);
+ counter_u64_free(V_pf_default_rule.src_nodes);
+
+ for (int i = 0; i < PFRES_MAX; i++)
+ counter_u64_free(V_pf_status.counters[i]);
+ for (int i = 0; i < LCNT_MAX; i++)
+ counter_u64_free(V_pf_status.lcounters[i]);
+ for (int i = 0; i < FCNT_MAX; i++)
+ counter_u64_free(V_pf_status.fcounters[i]);
+ for (int i = 0; i < SCNT_MAX; i++)
+ counter_u64_free(V_pf_status.scounters[i]);
} while(0);
return (error);
@@ -3698,6 +3706,7 @@ pf_load_vnet(void)
VNET_LIST_RUNLOCK();
pfattach_vnet();
+ V_pf_vnet_active = 1;
}
static int
@@ -3729,6 +3738,7 @@ pf_unload_vnet()
{
int error;
+ V_pf_vnet_active = 0;
V_pf_status.running = 0;
swi_remove(V_pf_swi_cookie);
error = dehook_pf();
@@ -3749,7 +3759,9 @@ pf_unload_vnet()
PF_RULES_WUNLOCK();
pf_normalize_cleanup();
+ PF_RULES_WLOCK();
pfi_cleanup_vnet();
+ PF_RULES_WUNLOCK();
pfr_cleanup();
pf_osfp_flush();
pf_cleanup();
More information about the svn-src-projects
mailing list