The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=a7d631f69d3fa993c701d681850d42a750886298
commit a7d631f69d3fa993c701d681850d42a750886298
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-06-17 13:55:12 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-06-25 20:04:15 +0000
pfctl: fix use-after-free and memory leak in pfctl_optimzie.c
OK bluhm@
Obtained from: OpenBSD, sashan <sas...@openbsd.org>, 43d70b8338
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sbin/pfctl/pfctl_optimize.c | 64 ++++++++++++++++++++++++++-------------------
sbin/pfctl/pfctl_parser.h | 1 +
2 files changed, 38 insertions(+), 27 deletions(-)
diff --git a/sbin/pfctl/pfctl_optimize.c b/sbin/pfctl/pfctl_optimize.c
index 530f1c241661..d6417e8e73a1 100644
--- a/sbin/pfctl/pfctl_optimize.c
+++ b/sbin/pfctl/pfctl_optimize.c
@@ -241,6 +241,8 @@ int skip_cmp_src_addr(struct pfctl_rule *, struct
pfctl_rule *);
int skip_cmp_src_port(struct pfctl_rule *, struct pfctl_rule *);
int superblock_inclusive(struct superblock *, struct pf_opt_rule *);
void superblock_free(struct pfctl *, struct superblock *);
+struct pf_opt_tbl *pf_opt_table_ref(struct pf_opt_tbl *);
+void pf_opt_table_unref(struct pf_opt_tbl *);
static int (*skip_comparitors[PF_SKIP_COUNT])(struct pfctl_rule *,
@@ -345,9 +347,11 @@ pfctl_optimize_ruleset(struct pfctl *pf, struct
pfctl_ruleset *rs)
TAILQ_INSERT_TAIL(
rs->rules[PF_RULESET_FILTER].active.ptr,
r, entries);
+ pf_opt_table_unref(por->por_src_tbl);
+ pf_opt_table_unref(por->por_dst_tbl);
free(por);
}
- free(block);
+ superblock_free(pf, block);
}
return (0);
@@ -355,16 +359,8 @@ pfctl_optimize_ruleset(struct pfctl *pf, struct
pfctl_ruleset *rs)
error:
while ((por = TAILQ_FIRST(&opt_queue))) {
TAILQ_REMOVE(&opt_queue, por, por_entry);
- if (por->por_src_tbl) {
- pfr_buf_clear(por->por_src_tbl->pt_buf);
- free(por->por_src_tbl->pt_buf);
- free(por->por_src_tbl);
- }
- if (por->por_dst_tbl) {
- pfr_buf_clear(por->por_dst_tbl->pt_buf);
- free(por->por_dst_tbl->pt_buf);
- free(por->por_dst_tbl);
- }
+ pf_opt_table_unref(por->por_src_tbl);
+ pf_opt_table_unref(por->por_dst_tbl);
free(por);
}
while ((block = TAILQ_FIRST(&superblocks))) {
@@ -537,12 +533,14 @@ combine_rules(struct pfctl *pf, struct superblock *block)
if (add_opt_table(pf, &p1->por_dst_tbl,
p1->por_rule.af, &p2->por_rule.dst))
return (1);
- p2->por_dst_tbl = p1->por_dst_tbl;
if (p1->por_dst_tbl->pt_rulecount >=
TABLE_THRESHOLD) {
TAILQ_REMOVE(&block->sb_rules, p2,
por_entry);
free(p2);
+ } else {
+ p2->por_dst_tbl =
+ pf_opt_table_ref(p1->por_dst_tbl);
}
} else if (!src_eq && dst_eq && p1->por_dst_tbl == NULL
&& p2->por_src_tbl == NULL &&
@@ -559,12 +557,14 @@ combine_rules(struct pfctl *pf, struct superblock *block)
if (add_opt_table(pf, &p1->por_src_tbl,
p1->por_rule.af, &p2->por_rule.src))
return (1);
- p2->por_src_tbl = p1->por_src_tbl;
if (p1->por_src_tbl->pt_rulecount >=
TABLE_THRESHOLD) {
TAILQ_REMOVE(&block->sb_rules, p2,
por_entry);
free(p2);
+ } else {
+ p2->por_src_tbl =
+ pf_opt_table_ref(p1->por_src_tbl);
}
}
}
@@ -1249,6 +1249,7 @@ add_opt_table(struct pfctl *pf, struct pf_opt_tbl **tbl,
sa_family_t af,
((*tbl)->pt_buf = calloc(1, sizeof(*(*tbl)->pt_buf))) ==
NULL)
err(1, "calloc");
+ (*tbl)->pt_refcnt = 1;
(*tbl)->pt_buf->pfrb_type = PFRB_ADDRS;
SIMPLEQ_INIT(&(*tbl)->pt_nodes);
@@ -1657,20 +1658,8 @@ superblock_free(struct pfctl *pf, struct superblock
*block)
struct pf_opt_rule *por;
while ((por = TAILQ_FIRST(&block->sb_rules))) {
TAILQ_REMOVE(&block->sb_rules, por, por_entry);
- if (por->por_src_tbl) {
- if (por->por_src_tbl->pt_buf) {
- pfr_buf_clear(por->por_src_tbl->pt_buf);
- free(por->por_src_tbl->pt_buf);
- }
- free(por->por_src_tbl);
- }
- if (por->por_dst_tbl) {
- if (por->por_dst_tbl->pt_buf) {
- pfr_buf_clear(por->por_dst_tbl->pt_buf);
- free(por->por_dst_tbl->pt_buf);
- }
- free(por->por_dst_tbl);
- }
+ pf_opt_table_unref(por->por_src_tbl);
+ pf_opt_table_unref(por->por_dst_tbl);
free(por);
}
if (block->sb_profiled_block)
@@ -1678,3 +1667,24 @@ superblock_free(struct pfctl *pf, struct superblock
*block)
free(block);
}
+struct pf_opt_tbl *
+pf_opt_table_ref(struct pf_opt_tbl *pt)
+{
+ /* parser does not run concurrently, we don't need atomic ops. */
+ if (pt != NULL)
+ pt->pt_refcnt++;
+
+ return (pt);
+}
+
+void
+pf_opt_table_unref(struct pf_opt_tbl *pt)
+{
+ if ((pt != NULL) && ((--pt->pt_refcnt) == 0)) {
+ if (pt->pt_buf != NULL) {
+ pfr_buf_clear(pt->pt_buf);
+ free(pt->pt_buf);
+ }
+ free(pt);
+ }
+}
diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h
index 718c05b306b2..45d9ebc45bc9 100644
--- a/sbin/pfctl/pfctl_parser.h
+++ b/sbin/pfctl/pfctl_parser.h
@@ -258,6 +258,7 @@ struct pf_opt_tbl {
char pt_name[PF_TABLE_NAME_SIZE];
int pt_rulecount;
int pt_generated;
+ uint32_t pt_refcnt;
struct node_tinithead pt_nodes;
struct pfr_buffer *pt_buf;
};
