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
This commit is contained in:
Gleb Smirnoff 2012-09-18 10:54:56 +00:00
parent 9e8c4accee
commit 1d6139c0e4
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=240641
3 changed files with 92 additions and 58 deletions

View file

@ -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, entry_node, pf_anchor_compare);
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 *, pf_osfp_t);
#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(struct pf_pdesc *, struct mbuf *,
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);

View file

@ -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;

View file

@ -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)