Page MenuHomeFreeBSD
Files F175284649

D60522.diff
No OneTemporary
Actions

D60522.diff
View Options

diff --git a/sys/netpfil/pf/pf_nl.c b/sys/netpfil/pf/pf_nl.c
--- a/sys/netpfil/pf/pf_nl.c
+++ b/sys/netpfil/pf/pf_nl.c
@@ -31,6 +31,7 @@
#include "opt_inet6.h"
#include <sys/param.h>
+#include <sys/systm.h>
#include <sys/malloc.h>
#include <sys/mbuf.h>
#include <sys/priv.h>
@@ -51,6 +52,20 @@
#include <netlink/netlink_debug.h>
_DECLARE_DEBUG(LOG_DEBUG);
+static uint16_t family_id;
+
+/* Per-socket storage of the family, see genl_sock_priv(). */
+struct pf_nl_sock_priv {
+ /* The addresses of a table replace, until its last message arrives. */
+ struct {
+ struct pfr_table table;
+ struct pfr_addr *addrs;
+ size_t count;
+ size_t size;
+ bool started;
+ } setaddrs;
+};
+
static bool nlattr_add_labels(struct nl_writer *nw, int attrtype,
const struct pf_krule *r);
static bool nlattr_add_rule(struct nl_writer *nw, const struct pf_krule *rule);
@@ -2284,6 +2299,87 @@
return (error);
}
+/*
+ * A replace can take more than one message (PFR_FLAG_START on the first,
+ * PFR_FLAG_DONE on the last). Collect the addresses in the socket's storage
+ * and replace the table once, so that concurrent replaces of one table do
+ * not mix.
+ */
+static void
+pf_nl_setaddrs_reset(struct pf_nl_sock_priv *ps)
+{
+ free(ps->setaddrs.addrs, M_PF);
+ memset(&ps->setaddrs, 0, sizeof(ps->setaddrs));
+}
+
+static void
+pf_nl_sock_priv_destroy(void *priv)
+{
+ pf_nl_setaddrs_reset(priv);
+}
+
+static int
+pf_nl_setaddrs(struct nl_parsed_table_addrs *attrs, struct nlpcb *nlp)
+{
+ struct pf_nl_sock_priv *ps;
+ size_t count;
+ unsigned int limit;
+ int error;
+
+ ps = genl_sock_priv(family_id, nlp);
+ if ((attrs->flags & PFR_FLAG_ALLMASK & ~(PFR_FLAG_START |
+ PFR_FLAG_DONE | PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK)) != 0) {
+ pf_nl_setaddrs_reset(ps);
+ return (EINVAL);
+ }
+ if (attrs->flags & PFR_FLAG_START) {
+ /* The replace begun before was not completed. */
+ pf_nl_setaddrs_reset(ps);
+ ps->setaddrs.table = attrs->table;
+ ps->setaddrs.started = true;
+ } else if (!ps->setaddrs.started ||
+ strcmp(ps->setaddrs.table.pfrt_name, attrs->table.pfrt_name) != 0 ||
+ strcmp(ps->setaddrs.table.pfrt_anchor,
+ attrs->table.pfrt_anchor) != 0) {
+ pf_nl_setaddrs_reset(ps);
+ return (EINVAL);
+ }
+
+ /* 0 is unlimited, as in pf_ioctl_set_limit(). */
+ limit = V_pf_limits[PF_LIMIT_TABLE_ENTRIES].limit;
+ if (limit == 0 || limit > INT_MAX)
+ limit = INT_MAX;
+ count = ps->setaddrs.count + attrs->addr_count;
+ if (count > limit ||
+ count > SIZE_MAX / sizeof(*ps->setaddrs.addrs) / 2) {
+ pf_nl_setaddrs_reset(ps);
+ return (ENOMEM);
+ }
+ if (count > ps->setaddrs.size) {
+ ps->setaddrs.size = MAX(count, 2 * ps->setaddrs.size);
+ ps->setaddrs.addrs = realloc(ps->setaddrs.addrs,
+ ps->setaddrs.size * sizeof(*ps->setaddrs.addrs), M_PF,
+ M_WAITOK);
+ }
+ if (attrs->addr_count > 0)
+ memcpy(&ps->setaddrs.addrs[ps->setaddrs.count], attrs->addrs,
+ attrs->addr_count * sizeof(*ps->setaddrs.addrs));
+ ps->setaddrs.count = count;
+
+ if ((attrs->flags & PFR_FLAG_DONE) == 0)
+ return (0);
+
+ PF_RULES_WLOCK();
+ error = pfr_set_addrs(&ps->setaddrs.table, ps->setaddrs.addrs,
+ ps->setaddrs.count, NULL, &attrs->nadd, &attrs->ndel,
+ &attrs->nchange,
+ attrs->flags | PFR_FLAG_START | PFR_FLAG_USERIOCTL, 0);
+ PF_RULES_WUNLOCK();
+ pf_nl_setaddrs_reset(ps);
+
+ return (error);
+}
+
static int
pf_handle_table_set_addrs(struct nlmsghdr *hdr, struct nl_pstate *npt)
{
@@ -2296,11 +2392,7 @@
if (error != 0)
return (error);
- PF_RULES_WLOCK();
- error = pfr_set_addrs(&attrs.table, &attrs.addrs[0],
- attrs.addr_count, NULL, &attrs.nadd, &attrs.ndel, &attrs.nchange,
- attrs.flags | PFR_FLAG_USERIOCTL, 0);
- PF_RULES_WUNLOCK();
+ error = pf_nl_setaddrs(&attrs, npt->nlp);
if (!nlmsg_reply(nw, hdr, sizeof(struct genlmsghdr)))
return (ENOMEM);
@@ -3107,8 +3199,6 @@
&osfp_parser,
};
-static uint16_t family_id;
-
static const struct genl_cmd pf_cmds[] = {
{
.cmd_num = PFNL_CMD_GETSTATES,
@@ -3527,6 +3617,8 @@
family_id = genl_register_family(PFNL_FAMILY_NAME, 0, 2, PFNL_CMD_MAX);
genl_register_cmds(family_id, pf_cmds, nitems(pf_cmds));
+ genl_register_sock_priv(family_id, sizeof(struct pf_nl_sock_priv),
+ pf_nl_sock_priv_destroy);
}
void
diff --git a/tests/sys/netpfil/pf/table.sh b/tests/sys/netpfil/pf/table.sh
--- a/tests/sys/netpfil/pf/table.sh
+++ b/tests/sys/netpfil/pf/table.sh
@@ -915,6 +915,57 @@
pft_cleanup
}
+atf_test_case "replace_concurrent" "cleanup"
+replace_concurrent_head()
+{
+ atf_set descr 'Test concurrent replaces of one table'
+ atf_set require.user root
+}
+
+replace_concurrent_body()
+{
+ pft_init
+ pwd=$(pwd)
+
+ vnet_mkjail alcatraz
+ jexec alcatraz pfctl -e
+ pft_set_rules alcatraz \
+ "table <foo>" \
+ "pass in from <foo>"
+
+ # Each replace takes 40 messages.
+ for i in `seq 1 40`; do
+ for j in `seq 1 250`; do
+ echo "10.1.${i}.${j}" >> ${pwd}/a.lst
+ echo "10.2.${i}.${j}" >> ${pwd}/b.lst
+ done
+ done
+ sort ${pwd}/a.lst > ${pwd}/a.sorted
+ sort ${pwd}/b.lst > ${pwd}/b.sorted
+
+ # The table holds one list or the other, never a mix of both.
+ for i in `seq 1 5`; do
+ jexec alcatraz pfctl -t foo -T replace -f ${pwd}/a.lst &
+ a=$!
+ jexec alcatraz pfctl -t foo -T replace -f ${pwd}/b.lst &
+ b=$!
+ wait $a || atf_fail "Replace with a.lst failed"
+ wait $b || atf_fail "Replace with b.lst failed"
+ atf_check -s exit:0 -o save:${pwd}/foo.out \
+ jexec alcatraz pfctl -t foo -T show
+ tr -d ' ' < ${pwd}/foo.out | sort > ${pwd}/foo.sorted
+ if ! cmp -s ${pwd}/foo.sorted ${pwd}/a.sorted &&
+ ! cmp -s ${pwd}/foo.sorted ${pwd}/b.sorted; then
+ atf_fail "Table has $(wc -l < ${pwd}/foo.sorted) entries"
+ fi
+ done
+}
+
+replace_concurrent_cleanup()
+{
+ pft_cleanup
+}
+
atf_test_case "load" "cleanup"
load_head()
{
@@ -1094,6 +1145,7 @@
atf_add_test_case "replace"
atf_add_test_case "replace_verbose"
atf_add_test_case "replace_create"
+ atf_add_test_case "replace_concurrent"
atf_add_test_case "load"
atf_add_test_case "test"
atf_add_test_case "test_verbose"

File Metadata

Mime Type
text/plain
Expires
Sat, Oct 10, 4:37 PM (21 h, 1 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
40556900
Default Alt Text
D60522.diff (5 KB)

Event Timeline