Author: melifaro
Date: Thu Feb 5 13:49:04 2015
New Revision: 278259
URL: https://svnweb.freebsd.org/changeset/base/278259
Log:
* Make sure table algorithm destroy hook is always called without locks
* Explicitly lock freeing interface references in ta_destroy_ifidx
* Change ipfw_iface_unref() to require UH lock
* Add forgotten ipfw_iface_unref() to destroy_ifidx_locked()
PR: kern/197276
Submitted by: lev
Sponsored by: Yandex LLC
Modified:
head/sys/netpfil/ipfw/ip_fw_iface.c (contents, props changed)
head/sys/netpfil/ipfw/ip_fw_private.h
head/sys/netpfil/ipfw/ip_fw_table.c
head/sys/netpfil/ipfw/ip_fw_table_algo.c
Modified: head/sys/netpfil/ipfw/ip_fw_iface.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_iface.c Thu Feb 5 13:07:41 2015
(r278258)
+++ head/sys/netpfil/ipfw/ip_fw_iface.c Thu Feb 5 13:49:04 2015
(r278259)
@@ -24,7 +24,7 @@
*/
#include <sys/cdefs.h>
-__FBSDID("$FreeBSD: projects/ipfw/sys/netpfil/ipfw/ip_fw_iface.c 267384
2014-06-12 09:59:11Z melifaro $");
+__FBSDID("$FreeBSD$");
/*
* Kernel interface tracking API.
@@ -397,20 +397,20 @@ ipfw_iface_del_notify(struct ip_fw_chain
/*
* Unreference interface specified by @ic.
- * Must be called without holding any locks.
+ * Must be called while holding UH lock.
*/
void
ipfw_iface_unref(struct ip_fw_chain *ch, struct ipfw_ifc *ic)
{
struct ipfw_iface *iif;
+ IPFW_UH_WLOCK_ASSERT(ch);
+
iif = ic->iface;
ic->iface = NULL;
- IPFW_UH_WLOCK(ch);
iif->no.refcnt--;
/* TODO: check for references & delete */
- IPFW_UH_WUNLOCK(ch);
}
/*
Modified: head/sys/netpfil/ipfw/ip_fw_private.h
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_private.h Thu Feb 5 13:07:41 2015
(r278258)
+++ head/sys/netpfil/ipfw/ip_fw_private.h Thu Feb 5 13:49:04 2015
(r278259)
@@ -429,6 +429,7 @@ struct ipfw_ifc {
#define IPFW_UH_RLOCK_ASSERT(_chain) rw_assert(&(_chain)->uh_lock,
RA_RLOCKED)
#define IPFW_UH_WLOCK_ASSERT(_chain) rw_assert(&(_chain)->uh_lock,
RA_WLOCKED)
+#define IPFW_UH_UNLOCK_ASSERT(_chain) rw_assert(&(_chain)->uh_lock,
RA_UNLOCKED)
#define IPFW_UH_RLOCK(p) rw_rlock(&(p)->uh_lock)
#define IPFW_UH_RUNLOCK(p) rw_runlock(&(p)->uh_lock)
Modified: head/sys/netpfil/ipfw/ip_fw_table.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_table.c Thu Feb 5 13:07:41 2015
(r278258)
+++ head/sys/netpfil/ipfw/ip_fw_table.c Thu Feb 5 13:49:04 2015
(r278259)
@@ -1198,7 +1198,7 @@ flush_table(struct ip_fw_chain *ch, stru
void *astate_old, *astate_new;
char algostate[64], *pstate;
struct tableop_state ts;
- int error;
+ int error, need_gc;
uint16_t kidx;
uint8_t tflags;
@@ -1212,6 +1212,9 @@ flush_table(struct ip_fw_chain *ch, stru
IPFW_UH_WUNLOCK(ch);
return (ESRCH);
}
+ need_gc = 0;
+ astate_new = NULL;
+ memset(&ti_new, 0, sizeof(ti_new));
restart:
/* Set up swap handler */
memset(&ts, 0, sizeof(ts));
@@ -1237,6 +1240,14 @@ restart:
IPFW_UH_WUNLOCK(ch);
/*
+ * Stage 1.5: if this is not the first attempt, destroy previous state
+ */
+ if (need_gc != 0) {
+ ta->destroy(astate_new, &ti_new);
+ need_gc = 0;
+ }
+
+ /*
* Stage 2: allocate new table instance using same algo.
*/
memset(&ti_new, 0, sizeof(struct table_info));
@@ -1262,7 +1273,8 @@ restart:
* complex checks.
*/
if (ts.modified != 0) {
- ta->destroy(astate_new, &ti_new);
+ /* Delay destroying data since we're holding UH lock */
+ need_gc = 1;
goto restart;
}
@@ -3042,6 +3054,7 @@ free_table_config(struct namedobj_instan
{
KASSERT(tc->linked == 0, ("free() on linked config"));
+ /* UH lock MUST NOT be held */
/*
* We're using ta without any locking/referencing.
Modified: head/sys/netpfil/ipfw/ip_fw_table_algo.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_table_algo.c Thu Feb 5 13:07:41 2015
(r278258)
+++ head/sys/netpfil/ipfw/ip_fw_table_algo.c Thu Feb 5 13:49:04 2015
(r278259)
@@ -97,7 +97,7 @@ __FBSDID("$FreeBSD$");
*
* -destroy: request to destroy table instance.
* typedef void (ta_destroy)(void *ta_state, struct table_info *ti);
- * MANDATORY, may be locked (UH+WLOCK). (M_NOWAIT).
+ * MANDATORY, unlocked. (M_WAITOK).
*
* Frees all table entries and all tables structures allocated by -init.
*
@@ -2134,6 +2134,7 @@ destroy_ifidx_locked(struct namedobj_ins
ife = (struct ifentry *)no;
ipfw_iface_del_notify(ch, &ife->ic);
+ ipfw_iface_unref(ch, &ife->ic);
free(ife, M_IPFW_TBL);
}
@@ -2153,7 +2154,9 @@ ta_destroy_ifidx(void *ta_state, struct
if (icfg->main_ptr != NULL)
free(icfg->main_ptr, M_IPFW);
+ IPFW_UH_WLOCK(ch);
ipfw_objhash_foreach(icfg->ii, destroy_ifidx_locked, ch);
+ IPFW_UH_WUNLOCK(ch);
ipfw_objhash_destroy(icfg->ii);
@@ -2333,8 +2336,9 @@ ta_del_ifidx(void *ta_state, struct tabl
/* Unlink from local list */
ipfw_objhash_del(icfg->ii, &ife->no);
- /* Unlink notifier */
+ /* Unlink notifier and deref */
ipfw_iface_del_notify(icfg->ch, &ife->ic);
+ ipfw_iface_unref(icfg->ch, &ife->ic);
icfg->count--;
tei->value = ife->value;
@@ -2357,11 +2361,8 @@ ta_flush_ifidx_entry(struct ip_fw_chain
tb = (struct ta_buf_ifidx *)ta_buf;
- if (tb->ife != NULL) {
- /* Unlink first */
- ipfw_iface_unref(ch, &tb->ife->ic);
+ if (tb->ife != NULL)
free(tb->ife, M_IPFW_TBL);
- }
}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
