Each ruleset's rules are stored in a TAILQ called `ptr' with `rcount'
representing the number of rules in the ruleset;  `ptr_array' points to
an array of the same length:

        struct pf_ruleset {
                struct {
                        ...
                        struct {
                                struct pf_rulequeue     *ptr;
                                struct pf_rule          **ptr_array;
                                u_int32_t                rcount;
                                ...
                        }                        active, inactive;
                }                        rules;
                ...
        };

`ptr' is backed by pool_get(9) and may change in size as "expired" rules
get removed from the ruleset -- see `once' in pf.conf(5).

`ptr_array' is allocated momentarily through mallocarray(9) and gets
filled with the TAILQ entries, the sole user pfsync(4) can then access
the list of rules by index to simply pick the n-th rule in a ruleset
during state insertion.

I came here due to the zero size in its free(9) call, but the diff below
removes the array completely and makes pfsync iterate over the TAILQ to
get "index" of the matching rule in the ruleset.

I've been successfully running this on amd64 firewalls with pfsync(4)
where I also added expiring `once' rules to further test with.

Feedback? OK?

Index: sys/net/if_pfsync.c
===================================================================
RCS file: /cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.274
diff -u -p -r1.274 if_pfsync.c
--- sys/net/if_pfsync.c 10 Jul 2020 13:26:42 -0000      1.274
+++ sys/net/if_pfsync.c 13 Jul 2020 16:54:34 -0000
@@ -490,6 +490,7 @@ pfsync_state_import(struct pfsync_state 
        struct pfi_kif  *kif;
        int pool_flags;
        int error = ENOMEM;
+       int n = 0;
 
        if (sp->creatorid == 0) {
                DPFPRINTF(LOG_NOTICE, "pfsync_state_import: "
@@ -514,9 +515,11 @@ pfsync_state_import(struct pfsync_state 
         */
        if (sp->rule != htonl(-1) && sp->anchor == htonl(-1) &&
            (flags & (PFSYNC_SI_IOCTL | PFSYNC_SI_CKSUM)) && ntohl(sp->rule) <
-           pf_main_ruleset.rules.active.rcount)
-               r = pf_main_ruleset.rules.active.ptr_array[ntohl(sp->rule)];
-       else
+           pf_main_ruleset.rules.active.rcount) {
+               TAILQ_FOREACH(r, pf_main_ruleset.rules.active.ptr, entries)
+                       if (ntohl(sp->rule) == n++)
+                               break;
+       } else
                r = &pf_default_rule;
 
        if ((r->max_states && r->states_cur >= r->max_states))
Index: sys/net/pf_ioctl.c
===================================================================
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.353
diff -u -p -r1.353 pf_ioctl.c
--- sys/net/pf_ioctl.c  24 Jun 2020 22:03:42 -0000      1.353
+++ sys/net/pf_ioctl.c  13 Jul 2020 21:35:19 -0000
@@ -803,7 +803,7 @@ int
 pf_commit_rules(u_int32_t ticket, char *anchor)
 {
        struct pf_ruleset       *rs;
-       struct pf_rule          *rule, **old_array;
+       struct pf_rule          *rule;
        struct pf_rulequeue     *old_rules;
        int                      error;
        u_int32_t                old_rcount;
@@ -826,13 +826,10 @@ pf_commit_rules(u_int32_t ticket, char *
        /* Swap rules, keep the old. */
        old_rules = rs->rules.active.ptr;
        old_rcount = rs->rules.active.rcount;
-       old_array = rs->rules.active.ptr_array;
 
        rs->rules.active.ptr = rs->rules.inactive.ptr;
-       rs->rules.active.ptr_array = rs->rules.inactive.ptr_array;
        rs->rules.active.rcount = rs->rules.inactive.rcount;
        rs->rules.inactive.ptr = old_rules;
-       rs->rules.inactive.ptr_array = old_array;
        rs->rules.inactive.rcount = old_rcount;
 
        rs->rules.active.ticket = rs->rules.inactive.ticket;
@@ -842,9 +839,6 @@ pf_commit_rules(u_int32_t ticket, char *
        /* Purge the old rule list. */
        while ((rule = TAILQ_FIRST(old_rules)) != NULL)
                pf_rm_rule(old_rules, rule);
-       if (rs->rules.inactive.ptr_array)
-               free(rs->rules.inactive.ptr_array, M_TEMP, 0);
-       rs->rules.inactive.ptr_array = NULL;
        rs->rules.inactive.rcount = 0;
        rs->rules.inactive.open = 0;
        pf_remove_if_empty_ruleset(rs);
@@ -863,21 +857,10 @@ pf_setup_pfsync_matching(struct pf_rules
        u_int8_t                 digest[PF_MD5_DIGEST_LENGTH];
 
        MD5Init(&ctx);
-       if (rs->rules.inactive.ptr_array)
-               free(rs->rules.inactive.ptr_array, M_TEMP, 0);
-       rs->rules.inactive.ptr_array = NULL;
 
        if (rs->rules.inactive.rcount) {
-               rs->rules.inactive.ptr_array =
-                   mallocarray(rs->rules.inactive.rcount, sizeof(caddr_t),
-                   M_TEMP, M_NOWAIT);
-
-               if (!rs->rules.inactive.ptr_array)
-                       return (ENOMEM);
-
                TAILQ_FOREACH(rule, rs->rules.inactive.ptr, entries) {
                        pf_hash_rule(&ctx, rule);
-                       (rs->rules.inactive.ptr_array)[rule->nr] = rule;
                }
        }
 
Index: sys/net/pfvar.h
===================================================================
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.493
diff -u -p -r1.493 pfvar.h
--- sys/net/pfvar.h     17 Nov 2019 08:25:05 -0000      1.493
+++ sys/net/pfvar.h     13 Jul 2020 16:55:20 -0000
@@ -923,7 +923,6 @@ struct pf_ruleset {
                struct pf_rulequeue      queues[2];
                struct {
                        struct pf_rulequeue     *ptr;
-                       struct pf_rule          **ptr_array;
                        u_int32_t                rcount;
                        u_int32_t                ticket;
                        int                      open;

Reply via email to