Author: glebius
Date: Tue Sep 18 10:54:56 2012
New Revision: 240641
URL: http://svn.freebsd.org/changeset/base/240641
Log:
Make ruleset anchors in pf(4) reentrant. We've got two problems here:
1) Ruleset parser uses a global variable for anchor stack.
2) When processing a wildcard anchor, matching anchors are marked.
To fix the first one:
o Allocate anchor processing stack on stack. To make this allocation
as small as possible, following measures taken:
- Maximum stack size reduced from 64 to 32.
- The struct pf_anchor_stackframe trimmed by one pointer - parent.
We can always obtain the parent via the rule pointer.
- When pf_test_rule() calls pf_get_translation(), the former lends
its stack to the latter, to avoid recursive allocation 32 entries.
The second one appeared more tricky. The code, that marks anchors was
added in OpenBSD rev. 1.516 of pf.c. According to commit log, the idea
is to enable the "quick" keyword on an anchor rule. The feature isn't
documented anywhere. The most obscure part of the 1.516 was that code
examines the "match" mark on a just processed child, which couldn't be
put here by current frame. Since this wasn't documented even in the
commit message and functionality of this is not clear to me, I decided
to drop this examination for now. The rest of 1.516 is redone in a
thread safe manner - the mark isn't put on the anchor itself, but on
current stack frame. To avoid growing stack frame, we utilize LSB
from the rule pointer, relying on kernel malloc(9) returning pointer
aligned addresses.
Discussed with: dhartmei
Modified:
head/sys/net/pfvar.h
head/sys/netpfil/pf/pf.c
head/sys/netpfil/pf/pf_lb.c
Modified: head/sys/net/pfvar.h
==============================================================================
--- head/sys/net/pfvar.h Tue Sep 18 10:52:46 2012 (r240640)
+++ head/sys/net/pfvar.h Tue Sep 18 10:54:56 2012 (r240641)
@@ -989,7 +989,7 @@ struct pf_anchor {
char path[MAXPATHLEN];
struct pf_ruleset ruleset;
int refcnt; /* anchor rules */
- int match;
+ int match; /* XXX: used for pfctl black magic */
};
RB_PROTOTYPE(pf_anchor_global, pf_anchor, entry_global, pf_anchor_compare);
RB_PROTOTYPE(pf_anchor_node, pf_anchor, entry_node, pf_anchor_compare);
@@ -1019,6 +1019,8 @@ RB_PROTOTYPE(pf_anchor_node, pf_anchor,
PFR_TFLAG_REFDANCHOR | \
PFR_TFLAG_COUNTERS)
+struct pf_anchor_stackframe;
+
struct pfr_table {
char pfrt_anchor[MAXPATHLEN];
char pfrt_name[PF_TABLE_NAME_SIZE];
@@ -1938,11 +1940,12 @@ int pf_osfp_match(struct pf_osfp_enlist
#ifdef _KERNEL
void pf_print_host(struct pf_addr *, u_int16_t, u_int8_t);
-void pf_step_into_anchor(int *, struct pf_ruleset **, int,
- struct pf_rule **, struct pf_rule **, int *);
-int pf_step_out_of_anchor(int *, struct pf_ruleset **,
- int, struct pf_rule **, struct pf_rule **,
- int *);
+void pf_step_into_anchor(struct pf_anchor_stackframe *, int
*,
+ struct pf_ruleset **, int, struct pf_rule **,
+ struct pf_rule **, int *);
+int pf_step_out_of_anchor(struct pf_anchor_stackframe *,
int *,
+ struct pf_ruleset **, int, struct pf_rule **,
+ struct pf_rule **, int *);
int pf_map_addr(u_int8_t, struct pf_rule *,
struct pf_addr *, struct pf_addr *,
@@ -1951,7 +1954,7 @@ struct pf_rule *pf_get_translation(stru
int, int, struct pfi_kif *, struct pf_src_node **,
struct pf_state_key **, struct pf_state_key **,
struct pf_addr *, struct pf_addr *,
- u_int16_t, u_int16_t);
+ uint16_t, uint16_t, struct pf_anchor_stackframe *);
struct pf_state_key *pf_state_key_setup(struct pf_pdesc *, struct pf_addr *,
struct pf_addr *, u_int16_t, u_int16_t);
Modified: head/sys/netpfil/pf/pf.c
==============================================================================
--- head/sys/netpfil/pf/pf.c Tue Sep 18 10:52:46 2012 (r240640)
+++ head/sys/netpfil/pf/pf.c Tue Sep 18 10:54:56 2012 (r240641)
@@ -127,15 +127,6 @@ VNET_DEFINE(int, pf_tcp_secret_init);
VNET_DEFINE(int, pf_tcp_iss_off);
#define V_pf_tcp_iss_off VNET(pf_tcp_iss_off)
-struct pf_anchor_stackframe {
- struct pf_ruleset *rs;
- struct pf_rule *r;
- struct pf_anchor_node *parent;
- struct pf_anchor *child;
-};
-VNET_DEFINE(struct pf_anchor_stackframe, pf_anchor_stack[64]);
-#define V_pf_anchor_stack VNET(pf_anchor_stack)
-
/*
* Queue for pf_intr() sends.
*/
@@ -2461,37 +2452,56 @@ pf_tag_packet(struct mbuf *m, struct pf_
return (0);
}
+#define PF_ANCHOR_STACKSIZE 32
+struct pf_anchor_stackframe {
+ struct pf_ruleset *rs;
+ struct pf_rule *r; /* XXX: + match bit */
+ struct pf_anchor *child;
+};
+
+/*
+ * XXX: We rely on malloc(9) returning pointer aligned addresses.
+ */
+#define PF_ANCHORSTACK_MATCH 0x00000001
+#define PF_ANCHORSTACK_MASK (PF_ANCHORSTACK_MATCH)
+
+#define PF_ANCHOR_MATCH(f) ((uintptr_t)(f)->r &
PF_ANCHORSTACK_MATCH)
+#define PF_ANCHOR_RULE(f) (struct pf_rule *)
\
+ ((uintptr_t)(f)->r & ~PF_ANCHORSTACK_MASK)
+#define PF_ANCHOR_SET_MATCH(f) do { (f)->r = (void *)
\
+ ((uintptr_t)(f)->r | PF_ANCHORSTACK_MATCH); \
+} while (0)
+
void
-pf_step_into_anchor(int *depth, struct pf_ruleset **rs, int n,
- struct pf_rule **r, struct pf_rule **a, int *match)
+pf_step_into_anchor(struct pf_anchor_stackframe *stack, int *depth,
+ struct pf_ruleset **rs, int n, struct pf_rule **r, struct pf_rule **a,
+ int *match)
{
struct pf_anchor_stackframe *f;
PF_RULES_RASSERT();
- (*r)->anchor->match = 0;
if (match)
*match = 0;
- if (*depth >= sizeof(V_pf_anchor_stack) /
- sizeof(V_pf_anchor_stack[0])) {
- printf("pf_step_into_anchor: stack overflow\n");
+ if (*depth >= PF_ANCHOR_STACKSIZE) {
+ printf("%s: anchor stack overflow on %s\n",
+ __func__, (*r)->anchor->name);
*r = TAILQ_NEXT(*r, entries);
return;
} else if (*depth == 0 && a != NULL)
*a = *r;
- f = V_pf_anchor_stack + (*depth)++;
+ f = stack + (*depth)++;
f->rs = *rs;
f->r = *r;
if ((*r)->anchor_wildcard) {
- f->parent = &(*r)->anchor->children;
- if ((f->child = RB_MIN(pf_anchor_node, f->parent)) ==
- NULL) {
+ struct pf_anchor_node *parent = &(*r)->anchor->children;
+
+ if ((f->child = RB_MIN(pf_anchor_node, parent)) == NULL) {
*r = NULL;
return;
}
*rs = &f->child->ruleset;
} else {
- f->parent = NULL;
f->child = NULL;
*rs = &(*r)->anchor->ruleset;
}
@@ -2499,10 +2509,12 @@ pf_step_into_anchor(int *depth, struct p
}
int
-pf_step_out_of_anchor(int *depth, struct pf_ruleset **rs, int n,
- struct pf_rule **r, struct pf_rule **a, int *match)
+pf_step_out_of_anchor(struct pf_anchor_stackframe *stack, int *depth,
+ struct pf_ruleset **rs, int n, struct pf_rule **r, struct pf_rule **a,
+ int *match)
{
struct pf_anchor_stackframe *f;
+ struct pf_rule *fr;
int quick = 0;
PF_RULES_RASSERT();
@@ -2510,14 +2522,26 @@ pf_step_out_of_anchor(int *depth, struct
do {
if (*depth <= 0)
break;
- f = V_pf_anchor_stack + *depth - 1;
- if (f->parent != NULL && f->child != NULL) {
- if (f->child->match ||
- (match != NULL && *match)) {
- f->r->anchor->match = 1;
+ f = stack + *depth - 1;
+ fr = PF_ANCHOR_RULE(f);
+ if (f->child != NULL) {
+ struct pf_anchor_node *parent;
+
+ /*
+ * This block traverses through
+ * a wildcard anchor.
+ */
+ parent = &fr->anchor->children;
+ if (match != NULL && *match) {
+ /*
+ * If any of "*" matched, then
+ * "foo/ *" matched, mark frame
+ * appropriately.
+ */
+ PF_ANCHOR_SET_MATCH(f);
*match = 0;
}
- f->child = RB_NEXT(pf_anchor_node, f->parent, f->child);
+ f->child = RB_NEXT(pf_anchor_node, parent, f->child);
if (f->child != NULL) {
*rs = &f->child->ruleset;
*r = TAILQ_FIRST((*rs)->rules[n].active.ptr);
@@ -2531,9 +2555,9 @@ pf_step_out_of_anchor(int *depth, struct
if (*depth == 0 && a != NULL)
*a = NULL;
*rs = f->rs;
- if (f->r->anchor->match || (match != NULL && *match))
- quick = f->r->quick;
- *r = TAILQ_NEXT(f->r, entries);
+ if (PF_ANCHOR_MATCH(f) || (match != NULL && *match))
+ quick = fr->quick;
+ *r = TAILQ_NEXT(fr, entries);
} while (*r == NULL);
return (quick);
@@ -2887,6 +2911,7 @@ pf_test_rule(struct pf_rule **rm, struct
u_int16_t sport = 0, dport = 0;
u_int16_t bproto_sum = 0, bip_sum = 0;
u_int8_t icmptype = 0, icmpcode = 0;
+ struct pf_anchor_stackframe anchor_stack[PF_ANCHOR_STACKSIZE];
PF_RULES_RASSERT();
@@ -2950,7 +2975,7 @@ pf_test_rule(struct pf_rule **rm, struct
/* check packet for BINAT/NAT/RDR */
if ((nr = pf_get_translation(pd, m, off, direction, kif, &nsn, &sk,
- &nk, saddr, daddr, sport, dport)) != NULL) {
+ &nk, saddr, daddr, sport, dport, anchor_stack)) != NULL) {
KASSERT(sk != NULL, ("%s: null sk", __func__));
KASSERT(nk != NULL, ("%s: null nk", __func__));
@@ -3150,11 +3175,12 @@ pf_test_rule(struct pf_rule **rm, struct
break;
r = TAILQ_NEXT(r, entries);
} else
- pf_step_into_anchor(&asd, &ruleset,
- PF_RULESET_FILTER, &r, &a, &match);
+ pf_step_into_anchor(anchor_stack, &asd,
+ &ruleset, PF_RULESET_FILTER, &r, &a,
+ &match);
}
- if (r == NULL && pf_step_out_of_anchor(&asd, &ruleset,
- PF_RULESET_FILTER, &r, &a, &match))
+ if (r == NULL && pf_step_out_of_anchor(anchor_stack, &asd,
+ &ruleset, PF_RULESET_FILTER, &r, &a, &match))
break;
}
r = *rm;
@@ -3527,6 +3553,7 @@ pf_test_fragment(struct pf_rule **rm, in
int tag = -1;
int asd = 0;
int match = 0;
+ struct pf_anchor_stackframe anchor_stack[PF_ANCHOR_STACKSIZE];
PF_RULES_RASSERT();
@@ -3577,11 +3604,12 @@ pf_test_fragment(struct pf_rule **rm, in
break;
r = TAILQ_NEXT(r, entries);
} else
- pf_step_into_anchor(&asd, &ruleset,
- PF_RULESET_FILTER, &r, &a, &match);
+ pf_step_into_anchor(anchor_stack, &asd,
+ &ruleset, PF_RULESET_FILTER, &r, &a,
+ &match);
}
- if (r == NULL && pf_step_out_of_anchor(&asd, &ruleset,
- PF_RULESET_FILTER, &r, &a, &match))
+ if (r == NULL && pf_step_out_of_anchor(anchor_stack, &asd,
+ &ruleset, PF_RULESET_FILTER, &r, &a, &match))
break;
}
r = *rm;
Modified: head/sys/netpfil/pf/pf_lb.c
==============================================================================
--- head/sys/netpfil/pf/pf_lb.c Tue Sep 18 10:52:46 2012 (r240640)
+++ head/sys/netpfil/pf/pf_lb.c Tue Sep 18 10:54:56 2012 (r240641)
@@ -58,7 +58,7 @@ static void pf_hash(struct pf_addr *,
static struct pf_rule *pf_match_translation(struct pf_pdesc *, struct mbuf *,
int, int, struct pfi_kif *,
struct pf_addr *, u_int16_t, struct pf_addr *,
- u_int16_t, int);
+ uint16_t, int, struct pf_anchor_stackframe *);
static int pf_get_sport(sa_family_t, u_int8_t, struct pf_rule *,
struct pf_addr *, struct pf_addr *, u_int16_t,
struct pf_addr *, u_int16_t*, u_int16_t, u_int16_t,
@@ -124,7 +124,8 @@ pf_hash(struct pf_addr *inaddr, struct p
static struct pf_rule *
pf_match_translation(struct pf_pdesc *pd, struct mbuf *m, int off,
int direction, struct pfi_kif *kif, struct pf_addr *saddr, u_int16_t sport,
- struct pf_addr *daddr, u_int16_t dport, int rs_num)
+ struct pf_addr *daddr, uint16_t dport, int rs_num,
+ struct pf_anchor_stackframe *anchor_stack)
{
struct pf_rule *r, *rm = NULL;
struct pf_ruleset *ruleset = NULL;
@@ -189,12 +190,12 @@ pf_match_translation(struct pf_pdesc *pd
if (r->anchor == NULL) {
rm = r;
} else
- pf_step_into_anchor(&asd, &ruleset, rs_num,
- &r, NULL, NULL);
+ pf_step_into_anchor(anchor_stack, &asd,
+ &ruleset, rs_num, &r, NULL, NULL);
}
if (r == NULL)
- pf_step_out_of_anchor(&asd, &ruleset, rs_num, &r,
- NULL, NULL);
+ pf_step_out_of_anchor(anchor_stack, &asd, &ruleset,
+ rs_num, &r, NULL, NULL);
}
if (tag > 0 && pf_tag_packet(m, pd, tag))
@@ -499,7 +500,7 @@ pf_get_translation(struct pf_pdesc *pd,
struct pfi_kif *kif, struct pf_src_node **sn,
struct pf_state_key **skp, struct pf_state_key **nkp,
struct pf_addr *saddr, struct pf_addr *daddr,
- u_int16_t sport, u_int16_t dport)
+ uint16_t sport, uint16_t dport, struct pf_anchor_stackframe *anchor_stack)
{
struct pf_rule *r = NULL;
struct pf_addr *naddr;
@@ -511,16 +512,18 @@ pf_get_translation(struct pf_pdesc *pd,
if (direction == PF_OUT) {
r = pf_match_translation(pd, m, off, direction, kif, saddr,
- sport, daddr, dport, PF_RULESET_BINAT);
+ sport, daddr, dport, PF_RULESET_BINAT, anchor_stack);
if (r == NULL)
r = pf_match_translation(pd, m, off, direction, kif,
- saddr, sport, daddr, dport, PF_RULESET_NAT);
+ saddr, sport, daddr, dport, PF_RULESET_NAT,
+ anchor_stack);
} else {
r = pf_match_translation(pd, m, off, direction, kif, saddr,
- sport, daddr, dport, PF_RULESET_RDR);
+ sport, daddr, dport, PF_RULESET_RDR, anchor_stack);
if (r == NULL)
r = pf_match_translation(pd, m, off, direction, kif,
- saddr, sport, daddr, dport, PF_RULESET_BINAT);
+ saddr, sport, daddr, dport, PF_RULESET_BINAT,
+ anchor_stack);
}
if (r == NULL)
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
