From 8ebca97f5ef134b3d7eb67626cb70283347e529f Mon Sep 17 00:00:00 2001 From: "Alexander V. Chernikov" Date: Tue, 7 Oct 2014 10:54:53 +0000 Subject: [PATCH] * Fix crash in interface tracker due to using old "linked" field. * Ensure we're flushing entries without any locks held. * Free memory in (rare) case when interface tracker fails to register ifp. * Add KASSERT on table values refcounts. --- sys/netpfil/ipfw/ip_fw_iface.c | 3 +-- sys/netpfil/ipfw/ip_fw_private.h | 2 -- sys/netpfil/ipfw/ip_fw_table.c | 4 ++++ sys/netpfil/ipfw/ip_fw_table_algo.c | 4 +++- sys/netpfil/ipfw/ip_fw_table_value.c | 7 ++++--- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/sys/netpfil/ipfw/ip_fw_iface.c b/sys/netpfil/ipfw/ip_fw_iface.c index b201558530ec..7e9c992032da 100644 --- a/sys/netpfil/ipfw/ip_fw_iface.c +++ b/sys/netpfil/ipfw/ip_fw_iface.c @@ -392,8 +392,7 @@ ipfw_iface_del_notify(struct ip_fw_chain *ch, struct ipfw_ifc *ic) IPFW_UH_WLOCK_ASSERT(ch); iif = ic->iface; - if (ic->linked != 0) - TAILQ_REMOVE(&iif->consumers, ic, next); + TAILQ_REMOVE(&iif->consumers, ic, next); } /* diff --git a/sys/netpfil/ipfw/ip_fw_private.h b/sys/netpfil/ipfw/ip_fw_private.h index d40ad366be94..bd13ae651580 100644 --- a/sys/netpfil/ipfw/ip_fw_private.h +++ b/sys/netpfil/ipfw/ip_fw_private.h @@ -355,8 +355,6 @@ struct ipfw_ifc { struct ipfw_iface *iface; ipfw_ifc_cb *cb; void *cbdata; - int linked; - int spare; }; /* Macro for working with various counters */ diff --git a/sys/netpfil/ipfw/ip_fw_table.c b/sys/netpfil/ipfw/ip_fw_table.c index f2f56fdf6057..ec42187ddf89 100644 --- a/sys/netpfil/ipfw/ip_fw_table.c +++ b/sys/netpfil/ipfw/ip_fw_table.c @@ -564,10 +564,14 @@ add_table_entry(struct ip_fw_chain *ch, struct tid_info *ti, */ restart: if (ts.modified != 0) { + IPFW_UH_WUNLOCK(ch); flush_batch_buffer(ch, ta, tei, count, rollback, ta_buf_m, ta_buf); memset(&ts, 0, sizeof(ts)); + ta = NULL; + IPFW_UH_WLOCK(ch); } + error = find_ref_table(ch, ti, tei, count, OP_ADD, &tc); if (error != 0) { IPFW_UH_WUNLOCK(ch); diff --git a/sys/netpfil/ipfw/ip_fw_table_algo.c b/sys/netpfil/ipfw/ip_fw_table_algo.c index 8165428bdb5a..124e46994de9 100644 --- a/sys/netpfil/ipfw/ip_fw_table_algo.c +++ b/sys/netpfil/ipfw/ip_fw_table_algo.c @@ -2045,8 +2045,10 @@ ta_prepare_add_ifidx(struct ip_fw_chain *ch, struct tentry_info *tei, ife->ic.cb = if_notifier; ife->ic.cbdata = ife; - if (ipfw_iface_ref(ch, ifname, &ife->ic) != 0) + if (ipfw_iface_ref(ch, ifname, &ife->ic) != 0) { + free(ife, M_IPFW_TBL); return (EINVAL); + } /* Use ipfw_iface 'ifname' field as stable storage */ ife->no.name = ife->ic.iface->ifname; diff --git a/sys/netpfil/ipfw/ip_fw_table_value.c b/sys/netpfil/ipfw/ip_fw_table_value.c index 22a9cfd5c218..5a789459c28c 100644 --- a/sys/netpfil/ipfw/ip_fw_table_value.c +++ b/sys/netpfil/ipfw/ip_fw_table_value.c @@ -251,10 +251,9 @@ unref_table_value(struct namedobj_instance *vi, struct table_value *pval, { struct table_val_link *ptvl; - if (pval[kidx].refcnt > 1) { - pval[kidx].refcnt--; + KASSERT(pval[kidx].refcnt > 0, ("Refcount is 0 on kidx %d", kidx)); + if (--pval[kidx].refcnt > 0) return; - } /* Last reference, delete item */ ptvl = (struct table_val_link *)ipfw_objhash_lookup_kidx(vi, kidx); @@ -307,6 +306,8 @@ ipfw_unref_table_values(struct ip_fw_chain *ch, struct table_config *tc, { struct flush_args fa; + IPFW_UH_WLOCK_ASSERT(ch); + memset(&fa, 0, sizeof(fa)); fa.ch = ch; fa.ta = ta;