From b2e3e909a01cf40b23f7613172345ab3dc53584e Mon Sep 17 00:00:00 2001 From: pelikan Date: Sat, 23 Aug 2014 00:11:03 +0000 Subject: [PATCH] when you specify queues in a rule, make sure they have been defined. DIOCADDRULE EBUSY turns into an error message that pfctl -n catches. DIOCXCOMMIT EINVAL after the kernel rejected the rules was reported to occur, possibly from hfsc.c: this should be fixed as well. ok henning mikeb sthen --- regress/sbin/pfctl/Makefile | 4 ++-- regress/sbin/pfctl/pfail54.in | 3 +++ regress/sbin/pfctl/pfail54.ok | 1 + sbin/pfctl/parse.y | 24 +++++++++++++++++++++++- sbin/pfctl/pfctl.c | 27 ++++++++++++++++++--------- sbin/pfctl/pfctl_parser.h | 20 +++++++++++--------- 6 files changed, 58 insertions(+), 21 deletions(-) create mode 100644 regress/sbin/pfctl/pfail54.in create mode 100644 regress/sbin/pfctl/pfail54.ok diff --git a/regress/sbin/pfctl/Makefile b/regress/sbin/pfctl/Makefile index a8c365ebf30..56ab9f909d5 100644 --- a/regress/sbin/pfctl/Makefile +++ b/regress/sbin/pfctl/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.219 2014/04/18 10:48:23 henning Exp $ +# $OpenBSD: Makefile,v 1.220 2014/08/23 00:11:03 pelikan Exp $ # TARGETS # pf: feed pfNN.in through pfctl and check whether the output matches pfNN.ok @@ -18,7 +18,7 @@ PFTESTS+=52 53 54 55 56 57 60 61 65 66 67 68 69 70 71 72 73 PFTESTS+=74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 PFTESTS+=97 98 99 100 101 PFFAIL=1 2 3 4 5 6 7 8 11 12 13 14 15 16 17 19 20 23 25 27 -PFFAIL+=30 37 38 39 40 41 42 43 47 48 49 50 51 52 53 +PFFAIL+=30 37 38 39 40 41 42 43 47 48 49 50 51 52 53 54 PFSIMPLE=1 2 PFSETUP=1 4 PFLOAD=1 2 3 4 5 7 8 9 10 11 12 13 14 15 16 17 18 19 20 23 24 25 26 27 28 29 diff --git a/regress/sbin/pfctl/pfail54.in b/regress/sbin/pfctl/pfail54.in new file mode 100644 index 00000000000..6cfa5da97e0 --- /dev/null +++ b/regress/sbin/pfctl/pfail54.in @@ -0,0 +1,3 @@ +queue root on lo1000000 bandwidth 1G +queue def on lo1000000 parent root bandwidth 1M default +pass set queue x diff --git a/regress/sbin/pfctl/pfail54.ok b/regress/sbin/pfctl/pfail54.ok new file mode 100644 index 00000000000..075eb1a507c --- /dev/null +++ b/regress/sbin/pfctl/pfail54.ok @@ -0,0 +1 @@ +stdin:3: queue x is not defined diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index a6eab7ab1a2..4175d171f73 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.637 2014/08/21 15:09:27 mikeb Exp $ */ +/* $OpenBSD: parse.y,v 1.638 2014/08/23 00:11:03 pelikan Exp $ */ /* * Copyright (c) 2001 Markus Friedl. All rights reserved. @@ -3454,14 +3454,36 @@ label : STRING { ; qname : STRING { + struct pfctl_qsitem *qsi; + + if ((qsi = pfctl_find_queue($1, &qspecs)) == NULL) { + yyerror("queue %s is not defined", $1); + YYERROR; + } $$.qname = $1; $$.pqname = NULL; } | '(' STRING ')' { + struct pfctl_qsitem *qsi; + + if ((qsi = pfctl_find_queue($2, &qspecs)) == NULL) { + yyerror("queue %s is not defined", $2); + YYERROR; + } $$.qname = $2; $$.pqname = NULL; } | '(' STRING comma STRING ')' { + struct pfctl_qsitem *qsi, *pqsi; + + if ((qsi = pfctl_find_queue($2, &qspecs)) == NULL) { + yyerror("queue %s is not defined", $2); + YYERROR; + } + if ((pqsi = pfctl_find_queue($4, &qspecs)) == NULL) { + yyerror("queue %s is not defined", $4); + YYERROR; + } $$.qname = $2; $$.pqname = $4; } diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 37b2db0415a..3c4979a7223 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pfctl.c,v 1.325 2014/04/19 14:22:32 henning Exp $ */ +/* $OpenBSD: pfctl.c,v 1.326 2014/08/23 00:11:03 pelikan Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -227,6 +227,10 @@ static const char *optiopt_list[] = { "none", "basic", "profile", NULL }; +struct pf_qihead qspecs = TAILQ_HEAD_INITIALIZER(qspecs); +struct pf_qihead rootqs = TAILQ_HEAD_INITIALIZER(rootqs); + + void usage(void) { @@ -1104,9 +1108,6 @@ pfctl_ruleset_trans(struct pfctl *pf, char *path, struct pf_anchor *a) return (0); } -TAILQ_HEAD(qspecs, pfctl_qsitem) qspecs = TAILQ_HEAD_INITIALIZER(qspecs); -TAILQ_HEAD(rootqs, pfctl_qsitem) rootqs = TAILQ_HEAD_INITIALIZER(rootqs); - int pfctl_add_queue(struct pfctl *pf, struct pf_queuespec *q) { @@ -1130,6 +1131,18 @@ pfctl_add_queue(struct pfctl *pf, struct pf_queuespec *q) return (0); } +struct pfctl_qsitem * +pfctl_find_queue(char *what, struct pf_qihead *where) +{ + struct pfctl_qsitem *q; + + TAILQ_FOREACH(q, where, entries) + if (strcmp(q->qs.qname, what) == 0) + return (q); + + return (NULL); +} + u_int pfctl_find_childqs(struct pfctl_qsitem *qi) { @@ -1144,11 +1157,7 @@ pfctl_find_childqs(struct pfctl_qsitem *qi) if (++p->matches > 10000) errx(1, "pfctl_find_childqs: excessive matches, loop?"); - /* check wether a children with that name is already there */ - TAILQ_FOREACH(q, &qi->children, entries) - if (!strcmp(q->qs.qname, p->qs.qname)) - break; - if (q == NULL) { + if ((q = pfctl_find_queue(p->qs.qname, &qi->children)) == NULL) { /* insert */ if ((n = calloc(1, sizeof(*n))) == NULL) err(1, "calloc"); diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h index 9762c9b66a7..3b957b855e5 100644 --- a/sbin/pfctl/pfctl_parser.h +++ b/sbin/pfctl/pfctl_parser.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfctl_parser.h,v 1.102 2014/04/19 14:22:32 henning Exp $ */ +/* $OpenBSD: pfctl_parser.h,v 1.103 2014/08/23 00:11:03 pelikan Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -192,6 +192,15 @@ struct pf_opt_rule { TAILQ_HEAD(pf_opt_queue, pf_opt_rule); +extern TAILQ_HEAD(pf_qihead, pfctl_qsitem) qspecs, rootqs; +struct pfctl_qsitem { + TAILQ_ENTRY(pfctl_qsitem) entries; + struct pf_queuespec qs; + struct pf_qihead children; + int matches; +}; + + int pfctl_rules(int, char *, int, int, char *, struct pfr_buffer *); int pfctl_optimize_ruleset(struct pfctl *, struct pf_ruleset *); int pf_opt_create_table(struct pfctl *, struct pf_opt_tbl *); @@ -218,6 +227,7 @@ int pfctl_load_anchors(int, struct pfctl *, struct pfr_buffer *); int pfctl_load_queues(struct pfctl *); int pfctl_add_queue(struct pfctl *, struct pf_queuespec *); +struct pfctl_qsitem * pfctl_find_queue(char *, struct pf_qihead *); void print_pool(struct pf_pool *, u_int16_t, u_int16_t, sa_family_t, int, int); void print_src_node(struct pf_src_node *, int); @@ -276,12 +286,4 @@ int append_addr(struct pfr_buffer *, char *, int); int append_addr_host(struct pfr_buffer *, struct node_host *, int, int); -TAILQ_HEAD(pf_qihead, pfctl_qsitem); -struct pfctl_qsitem { - TAILQ_ENTRY(pfctl_qsitem) entries; - struct pf_queuespec qs; - struct pf_qihead children; - int matches; -}; - #endif /* _PFCTL_PARSER_H_ */ -- 2.20.1