Page MenuHomeFreeBSD

D60526.id189241.diff
No OneTemporary

D60526.id189241.diff

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -286,6 +286,31 @@
#define PF_OVERLOADQ_LOCK() mtx_lock(&pf_overloadqueue_mtx)
#define PF_OVERLOADQ_UNLOCK() mtx_unlock(&pf_overloadqueue_mtx)
+/*
+ * Source limiter overload table changes, applied by pf_source_table_task().
+ * Tables can only be modified with the rules lock held exclusively, which
+ * packet processing does not hold.
+ */
+struct pf_source_table_entry {
+ STAILQ_ENTRY(pf_source_table_entry) next;
+ struct pfr_addr addr;
+ struct pfr_ktable *table; /* compared only */
+ uint32_t id;
+ bool insert;
+};
+
+STAILQ_HEAD(pf_source_table_head, pf_source_table_entry);
+VNET_DEFINE_STATIC(struct pf_source_table_head, pf_source_tablequeue);
+#define V_pf_source_tablequeue VNET(pf_source_tablequeue)
+VNET_DEFINE_STATIC(struct task, pf_source_tabletask);
+#define V_pf_source_tabletask VNET(pf_source_tabletask)
+
+static struct mtx_padalign pf_source_tablequeue_mtx;
+MTX_SYSINIT(pf_source_tablequeue_mtx, &pf_source_tablequeue_mtx,
+ "pf source limiter table queue", MTX_DEF);
+#define PF_SOURCE_TABLEQ_LOCK() mtx_lock(&pf_source_tablequeue_mtx)
+#define PF_SOURCE_TABLEQ_UNLOCK() mtx_unlock(&pf_source_tablequeue_mtx)
+
VNET_DEFINE(struct pf_krulequeue, pf_unlinked_rules);
struct mtx_padalign pf_unlnkdrules_mtx;
MTX_SYSINIT(pf_unlnkdrules_mtx, &pf_unlnkdrules_mtx, "pf unlinked rules",
@@ -402,6 +427,9 @@
struct pf_krule *, struct pf_kruleset *,
struct pf_krule_slist *);
static void pf_overload_task(void *v, int pending);
+static void pf_source_table_task(void *v, int pending);
+static bool pf_source_table_enqueue(struct pf_source *sr,
+ bool insert);
static u_short pf_insert_src_node(struct pf_ksrc_node *[PF_SN_MAX],
struct pf_srchash *[PF_SN_MAX], struct pf_krule *,
struct pf_addr *, sa_family_t, struct pf_addr *,
@@ -571,6 +599,15 @@
srlim->pfsrlim_rate.seconds + 1)
continue;
+ /* Retry a removal pf_source_rele() failed to queue. */
+ if (sr->pfsr_intable &&
+ srlim->pfsrlim_overload.table != NULL &&
+ srlim->pfsrlim_overload.lwm > 0) {
+ if (!pf_source_table_enqueue(sr, false))
+ continue;
+ sr->pfsr_intable = 0;
+ }
+
TAILQ_REMOVE(&pf_source_gc, sr, pfsr_empty_gc);
RB_REMOVE(pf_source_tree, &srlim->pfsrlim_sources, sr);
@@ -603,11 +640,79 @@
}
}
+/*
+ * Queue an overload table change for pf_source_table_task(). The entry
+ * identifies the limiter by id and remembers its table: either may be gone
+ * or replaced by the time the task runs, in which case the change is
+ * dropped.
+ */
+static bool
+pf_source_table_enqueue(struct pf_source *sr, bool insert)
+{
+ struct pf_source_table_entry *pfste;
+
+ pfste = malloc(sizeof(*pfste), M_PFTEMP, M_NOWAIT);
+ if (pfste == NULL)
+ return (false);
+
+ pf_source_pfr_addr(&pfste->addr, sr);
+ pfste->table = sr->pfsr_parent->pfsrlim_overload.table;
+ pfste->id = sr->pfsr_parent->pfsrlim_id;
+ pfste->insert = insert;
+
+ PF_SOURCE_TABLEQ_LOCK();
+ STAILQ_INSERT_TAIL(&V_pf_source_tablequeue, pfste, next);
+ PF_SOURCE_TABLEQ_UNLOCK();
+ taskqueue_enqueue(taskqueue_swi, &V_pf_source_tabletask);
+
+ return (true);
+}
+
+static void
+pf_source_table_task(void *v, int pending __unused)
+{
+ struct pf_source_table_head queue = STAILQ_HEAD_INITIALIZER(queue);
+ struct pf_source_table_entry *pfste, *npfste;
+ struct pf_sourcelim *srlim;
+ struct pfr_ktable *t;
+
+ CURVNET_SET((struct vnet *)v);
+
+ PF_SOURCE_TABLEQ_LOCK();
+ STAILQ_CONCAT(&queue, &V_pf_source_tablequeue);
+ PF_SOURCE_TABLEQ_UNLOCK();
+
+ /* A previous run may already have processed the queue. */
+ if (STAILQ_EMPTY(&queue)) {
+ CURVNET_RESTORE();
+ return;
+ }
+
+ PF_RULES_WLOCK();
+ STAILQ_FOREACH(pfste, &queue, next) {
+ srlim = pf_sourcelim_find(pfste->id);
+ if (srlim == NULL ||
+ (t = srlim->pfsrlim_overload.table) == NULL ||
+ t != pfste->table)
+ continue;
+
+ if (pfste->insert)
+ pfr_insert_kentry(t, &pfste->addr, time_second);
+ else
+ pfr_remove_kentry(t, &pfste->addr);
+ }
+ PF_RULES_WUNLOCK();
+
+ STAILQ_FOREACH_SAFE(pfste, &queue, next, npfste)
+ free(pfste, M_PFTEMP);
+
+ CURVNET_RESTORE();
+}
+
static void
pf_source_used(struct pf_source *sr)
{
struct pf_sourcelim *srlim = sr->pfsr_parent;
- struct pfr_ktable *t;
unsigned int used;
used = sr->pfsr_inuse++;
@@ -615,14 +720,10 @@
if (used == 0)
TAILQ_REMOVE(&pf_source_gc, sr, pfsr_empty_gc);
- else if ((t = srlim->pfsrlim_overload.table) != NULL &&
+ else if (srlim->pfsrlim_overload.table != NULL &&
used >= srlim->pfsrlim_overload.hwm && !sr->pfsr_intable) {
- struct pfr_addr p;
-
- pf_source_pfr_addr(&p, sr);
-
- pfr_insert_kentry(t, &p, time_second);
- sr->pfsr_intable = 1;
+ if (pf_source_table_enqueue(sr, true))
+ sr->pfsr_intable = 1;
}
}
@@ -630,20 +731,14 @@
pf_source_rele(struct pf_source *sr)
{
struct pf_sourcelim *srlim = sr->pfsr_parent;
- struct pfr_ktable *t;
unsigned int used;
used = --sr->pfsr_inuse;
- t = srlim->pfsrlim_overload.table;
- if (t != NULL && sr->pfsr_intable &&
+ if (srlim->pfsrlim_overload.table != NULL && sr->pfsr_intable &&
used < srlim->pfsrlim_overload.lwm) {
- struct pfr_addr p;
-
- pf_source_pfr_addr(&p, sr);
-
- pfr_remove_kentry(t, &p);
- sr->pfsr_intable = 0;
+ if (pf_source_table_enqueue(sr, false))
+ sr->pfsr_intable = 0;
}
if (used == 0) {
@@ -1543,6 +1638,8 @@
STAILQ_INIT(&V_pf_sendqueue);
SLIST_INIT(&V_pf_overloadqueue);
TASK_INIT(&V_pf_overloadtask, 0, pf_overload_task, curvnet);
+ STAILQ_INIT(&V_pf_source_tablequeue);
+ TASK_INIT(&V_pf_source_tabletask, 0, pf_source_table_task, curvnet);
/* Unlinked, but may be referenced rules. */
TAILQ_INIT(&V_pf_unlinked_rules);
@@ -2977,6 +3074,9 @@
pf_purge_expired_src_nodes();
pf_source_purge();
+ /* All states are gone, so no more table changes can be queued. */
+ taskqueue_drain(taskqueue_swi, &V_pf_source_tabletask);
+
/*
* Now all kifs & rules should be unreferenced,
* thus should be successfully freed.
diff --git a/tests/sys/netpfil/pf/limiters.sh b/tests/sys/netpfil/pf/limiters.sh
--- a/tests/sys/netpfil/pf/limiters.sh
+++ b/tests/sys/netpfil/pf/limiters.sh
@@ -306,6 +306,65 @@
pft_cleanup
}
+atf_test_case "source_table" "cleanup"
+source_table_head()
+{
+ atf_set descr 'Test the table of a source limiter'
+ atf_set require.user root
+}
+
+source_table_body()
+{
+ pft_init
+
+ epair=$(vnet_mkepair)
+
+ ifconfig ${epair}a 192.0.2.2/24 up
+
+ vnet_mkjail alcatraz ${epair}b
+ jexec alcatraz ifconfig ${epair}b 192.0.2.1/24 up
+
+ # Sanity check
+ atf_check -s exit:0 -o ignore \
+ ping -c 1 192.0.2.1
+
+ jexec alcatraz pfctl -e
+
+ # This used to panic a kernel with INVARIANTS: the table was changed
+ # while the packet was handled, with the rules lock held for reading.
+ pft_set_rules alcatraz \
+ "set timeout icmp.error 120" \
+ "table <many> persist" \
+ "source limiter \"server\" id 1 entries 128 limit 100 table <many> above 2 below 2" \
+ "pass in proto icmp source limiter \"server\""
+
+ # Each ping creates a state. Two states do not exceed the mark.
+ atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+ atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+ sleep 1
+ atf_check -s exit:0 -o empty \
+ jexec alcatraz pfctl -t many -T show
+
+ atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+ atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+ # The address is added to the table by a task.
+ sleep 1
+ atf_check -s exit:0 -o "match:192.0.2.2" \
+ jexec alcatraz pfctl -t many -T show
+
+ # Once the states are gone the source is below the mark and is removed.
+ atf_check -s exit:0 -e ignore \
+ jexec alcatraz pfctl -F states
+ sleep 1
+ atf_check -s exit:0 -o empty \
+ jexec alcatraz pfctl -t many -T show
+}
+
+source_table_cleanup()
+{
+ pft_cleanup
+}
+
atf_test_case "out" "cleanup"
out_head()
{
@@ -358,5 +417,6 @@
atf_add_test_case "state_block"
atf_add_test_case "state_multiple"
atf_add_test_case "source_basic"
+ atf_add_test_case "source_table"
atf_add_test_case "out"
}

File Metadata

Mime Type
text/plain
Expires
Sat, Oct 10, 3:35 PM (19 h, 58 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
40549519
Default Alt Text
D60526.id189241.diff (7 KB)

Event Timeline