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

2 files changed, 82 insertions, 51 deletions

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index a61b87b..74b3a64 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -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_pdesc *pd, int tag)
 	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 pf_ruleset **rs, int n,
 }
 
 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 pf_ruleset **rs, int n,
 	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 pf_ruleset **rs, int n,
 		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 pf_state **sm, int direction,
 	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 pf_state **sm, int direction,
 
 	/* 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 pf_state **sm, int direction,
 					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, int direction, struct pfi_kif *kif,
 	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, int direction, struct pfi_kif *kif,
 					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;
diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index 5b47852..47ad5a3 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -58,7 +58,7 @@ static void		 pf_hash(struct pf_addr *, 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 pf_addr *hash,
 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, struct mbuf *m, int off,
 			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 mbuf *m, int off, int direction,
     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, struct mbuf *m, int off, int direction,
 
 	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)