git: 285b505a4ee0 - stable/14 - pf: fix mem leaks upon vnet destroy

The branch stable/14 has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=285b505a4ee0ab2fdc8f970e7d23a6d3091dc686

commit 285b505a4ee0ab2fdc8f970e7d23a6d3091dc686
Author:     Igor Ostapenko <pm@igoro.pro>
AuthorDate: 2023-11-29 12:35:41 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2023-12-06 09:08:25 +0000

    pf: fix mem leaks upon vnet destroy
    
    Add missing cleanup actions:
    - remove user defined anchor rulesets
    - remove user defined ether anchor rulesets
    - remove tables linked to user defined anchors
    - deal with wildcard anchor peculiarities to get them removed correctly
    
    PR:             274310
    Reviewed by:    kp
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D42747
    
    (cherry picked from commit 0626d30e41cba64b41667314c3a4f7611f0eb685)
---
 sys/netpfil/pf/pf_ioctl.c | 67 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 7 deletions(-)

diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index de43570176cb..a397b9e99261 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -6028,6 +6028,7 @@ pf_clear_tables(void)
 	int error;
 
 	bzero(&io, sizeof(io));
+	io.pfrio_flags |= PFR_FLAG_ALLRSETS;
 
 	error = pfr_clr_tables(&io.pfrio_table, &io.pfrio_ndel,
 	    io.pfrio_flags);
@@ -6438,8 +6439,54 @@ shutdown_pf(void)
 	int error = 0;
 	u_int32_t t[5];
 	char nn = '\0';
+	struct pf_kanchor *anchor;
+	struct pf_keth_anchor *eth_anchor;
+	int rs_num;
 
 	do {
+		/* Unlink rules of all user defined anchors */
+		RB_FOREACH(anchor, pf_kanchor_global, &V_pf_anchors) {
+			/* Wildcard based anchors may not have a respective
+			 * explicit anchor rule or they may be left empty
+			 * without rules. It leads to anchor.refcnt=0, and the
+			 * rest of the logic does not expect it. */
+			if (anchor->refcnt == 0)
+				anchor->refcnt = 1;
+			for (rs_num = 0; rs_num < PF_RULESET_MAX; ++rs_num) {
+				if ((error = pf_begin_rules(&t[rs_num], rs_num,
+				    anchor->path)) != 0) {
+					DPFPRINTF(PF_DEBUG_MISC, ("shutdown_pf: "
+					    "anchor.path=%s rs_num=%d\n",
+					    anchor->path, rs_num));
+					goto error;	/* XXX: rollback? */
+				}
+			}
+			for (rs_num = 0; rs_num < PF_RULESET_MAX; ++rs_num) {
+				error = pf_commit_rules(t[rs_num], rs_num,
+				    anchor->path);
+				MPASS(error == 0);
+			}
+		}
+
+		/* Unlink rules of all user defined ether anchors */
+		RB_FOREACH(eth_anchor, pf_keth_anchor_global,
+		    &V_pf_keth_anchors) {
+			/* Wildcard based anchors may not have a respective
+			 * explicit anchor rule or they may be left empty
+			 * without rules. It leads to anchor.refcnt=0, and the
+			 * rest of the logic does not expect it. */
+			if (eth_anchor->refcnt == 0)
+				eth_anchor->refcnt = 1;
+			if ((error = pf_begin_eth(&t[0], eth_anchor->path))
+			    != 0) {
+				DPFPRINTF(PF_DEBUG_MISC, ("shutdown_pf: eth "
+				    "anchor.path=%s\n", eth_anchor->path));
+				goto error;
+			}
+			error = pf_commit_eth(t[0], eth_anchor->path);
+			MPASS(error == 0);
+		}
+
 		if ((error = pf_begin_rules(&t[0], PF_RULESET_SCRUB, &nn))
 		    != 0) {
 			DPFPRINTF(PF_DEBUG_MISC, ("shutdown_pf: SCRUB\n"));
@@ -6466,12 +6513,16 @@ shutdown_pf(void)
 			break;		/* XXX: rollback? */
 		}
 
-		/* XXX: these should always succeed here */
-		pf_commit_rules(t[0], PF_RULESET_SCRUB, &nn);
-		pf_commit_rules(t[1], PF_RULESET_FILTER, &nn);
-		pf_commit_rules(t[2], PF_RULESET_NAT, &nn);
-		pf_commit_rules(t[3], PF_RULESET_BINAT, &nn);
-		pf_commit_rules(t[4], PF_RULESET_RDR, &nn);
+		error = pf_commit_rules(t[0], PF_RULESET_SCRUB, &nn);
+		MPASS(error == 0);
+		error = pf_commit_rules(t[1], PF_RULESET_FILTER, &nn);
+		MPASS(error == 0);
+		error = pf_commit_rules(t[2], PF_RULESET_NAT, &nn);
+		MPASS(error == 0);
+		error = pf_commit_rules(t[3], PF_RULESET_BINAT, &nn);
+		MPASS(error == 0);
+		error = pf_commit_rules(t[4], PF_RULESET_RDR, &nn);
+		MPASS(error == 0);
 
 		if ((error = pf_clear_tables()) != 0)
 			break;
@@ -6480,7 +6531,8 @@ shutdown_pf(void)
 			DPFPRINTF(PF_DEBUG_MISC, ("shutdown_pf: eth\n"));
 			break;
 		}
-		pf_commit_eth(t[0], &nn);
+		error = pf_commit_eth(t[0], &nn);
+		MPASS(error == 0);
 
 #ifdef ALTQ
 		if ((error = pf_begin_altq(&t[0])) != 0) {
@@ -6498,6 +6550,7 @@ shutdown_pf(void)
 		/* fingerprints and interfaces have their own cleanup code */
 	} while(0);
 
+error:
 	return (error);
 }