From nobody Wed Jun 25 20:04:44 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4bSCTT260jz60SfZ; Wed, 25 Jun 2025 20:04:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bSCTT0DXNz3JCp; Wed, 25 Jun 2025 20:04:45 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1750881885; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=JOyE82eombOhL/q4TbJold6xQLwQXla62vnK6L7JClI=; b=sBItL6uhRCU1WYQBQ2TuABUymt37b7N8KRQaUOTdJ47RcNbZyqkGBeDJv6kvCh/zA4v2oa 2khuLVFgBP+9pflMioPOhPe9aqbdHyKTAy9x6IxzA5yi0qMDuD1eQOtWHaeLfLZejDDimH OrMjR7+AcBAByFQrQi0Q/+Q3MEBEAUbYyhjMzsKEo9TCqdw6U+eje+vwJtv1DM+hFQnJ7f cuOHhbRmEmfU7N8xUXGzVhRR9yuTi6Iu8P6BPy3ycTn//W82Fh+2EJgrFxJrLIxHYpam45 knRvJqSAekcP3YXoVxwx5CtypkJKlg3lwxDgk7TdPp3x4TGI5Zzxn8wnIBTm2g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1750881885; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=JOyE82eombOhL/q4TbJold6xQLwQXla62vnK6L7JClI=; b=G4kM23NGxTFF3uAlaPBLff5mgSgV243Hil+MBag/FR9WRZD7RhADzNFS9uiT+ru58tbBiJ jIpFHzmtf3Xg/4hTaZ49Y+t47dfKIo6yrjWm/HIMDwSnM0v5MNQs+0HSnL7R/QzupsrH+k UXWP+ZYelDVlp9hLgzbgdLHuohkQjwy8EnoGP/GBd4oZTozCUNsulNoxCQg1piWZz2NmgD gOkLKDPwC+DbMWU+9YvXDAkCDCVZMOf8Rt8PE+LhIPzHICuwHQtY78A2guJdMz3bLjYYK6 ifTg/CAAGJYvXOjGi3A21/lnejkH3ooVwFAIsWIMbEHWB/p0Gwo9m8H2TO6CZw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1750881885; a=rsa-sha256; cv=none; b=do/Y/odLwvUIkIWTSbtJPcLz3T8LEiG6S5sxRu2Nxn9Wa+5NSzIl8IaeQkb0R+Nbp/lWCo tXl79CYDyiKXI8H5UK/snmDq/byl7GzYStUgcRRsf3iFS3HpYEOilMKZoOHMCJmfQStSnN 9TINsics+vKSPU+6Ag5FxhNR5CnhwLmiBpe6mBPGpTNkrhlJPNntpQZsp/ZG3fmumC/TXO mzan2cB+tS3CyPT/RfKQ+czM6ESpZ9Cc28lyiLDSqfl3juiFZM/Xl5f2KkofPrfsNC6B9C gIDbyMZ5d1L80+sIrbAmISdr9rduengr5lLacvqNPqbIIp9MWrsiNKxkfuM11w== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4bSCTS62L6zYMZ; Wed, 25 Jun 2025 20:04:44 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 55PK4iMi080707; Wed, 25 Jun 2025 20:04:44 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 55PK4igF080704; Wed, 25 Jun 2025 20:04:44 GMT (envelope-from git) Date: Wed, 25 Jun 2025 20:04:44 GMT Message-Id: <202506252004.55PK4igF080704@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: a7d631f69d3f - main - pfctl: fix use-after-free and memory leak in pfctl_optimzie.c List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: a7d631f69d3fa993c701d681850d42a750886298 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=a7d631f69d3fa993c701d681850d42a750886298 commit a7d631f69d3fa993c701d681850d42a750886298 Author: Kristof Provost AuthorDate: 2025-06-17 13:55:12 +0000 Commit: Kristof Provost 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 , 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; };