Add a pool for the allocation of the pf_anchor struct.

It was possible to exhaust kernel memory by repeatedly calling
pfioctl DIOCXBEGIN with different anchor names.
OK bluhm@
Reported-by: syzbot+9dd98cbce69e26f0fc11@syzkaller.appspotmail.com

diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 9756425..a016297 100644 (file)
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pfctl.c,v 1.385 2021/11/11 12:49:53 sashan Exp $ */
+/*     $OpenBSD: pfctl.c,v 1.386 2022/07/20 09:33:11 mbuhl Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -158,6 +158,7 @@ static const struct {
        { "tables",             PF_LIMIT_TABLES },
        { "table-entries",      PF_LIMIT_TABLE_ENTRIES },
        { "pktdelay-pkts",      PF_LIMIT_PKTDELAY_PKTS },
+       { "anchors",            PF_LIMIT_ANCHORS },
        { NULL,                 0 }
 };
 

diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4
index db90604..6d3d186 100644 (file)
--- a/share/man/man4/pf.4
+++ b/share/man/man4/pf.4
@@ -1,4 +1,4 @@
-.\"    $OpenBSD: pf.4,v 1.92 2019/05/26 02:06:55 lteo Exp $
+.\"    $OpenBSD: pf.4,v 1.93 2022/07/20 09:33:11 mbuhl Exp $
 .\"
 .\" Copyright (C) 2001, Kjell Wooding.  All rights reserved.
 .\"
@@ -26,7 +26,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: May 26 2019 $
+.Dd $Mdocdate: July 20 2022 $
 .Dt PF 4
 .Os
 .Sh NAME
@@ -416,7 +416,7 @@ struct pfioc_limit {
 
 enum   { PF_LIMIT_STATES, PF_LIMIT_SRC_NODES, PF_LIMIT_FRAGS,
          PF_LIMIT_TABLES, PF_LIMIT_TABLE_ENTRIES, PF_LIMIT_PKTDELAY_PKTS,
-         PF_LIMIT_MAX };
+         PF_LIMIT_ANCHORS, PF_LIMIT_MAX };
 .Ed
 .It Dv DIOCGETLIMIT Fa "struct pfioc_limit *pl"
 Get the hard
@@ -1033,6 +1033,7 @@ static const struct {
        { "frags",              PF_LIMIT_FRAGS },
        { "tables",             PF_LIMIT_TABLES },
        { "table-entries",      PF_LIMIT_TABLE_ENTRIES },
+       { "anchors",            PF_LIMIT_ANCHORS },
        { NULL,                 0 }
 };
 

diff --git a/sys/net/pf.c b/sys/net/pf.c
index a7b1e1b..7183db9 100644 (file)
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1135 2022/06/28 13:48:06 henning Exp $ */
+/*     $OpenBSD: pf.c,v 1.1136 2022/07/20 09:33:11 mbuhl Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -276,7 +276,8 @@ struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = {
        { &pf_frent_pl, PFFRAG_FRENT_HIWAT, PFFRAG_FRENT_HIWAT },
        { &pfr_ktable_pl, PFR_KTABLE_HIWAT, PFR_KTABLE_HIWAT },
        { &pfr_kentry_pl, PFR_KENTRY_HIWAT, PFR_KENTRY_HIWAT },
-       { &pf_pktdelay_pl, PF_PKTDELAY_MAXPKTS, PF_PKTDELAY_MAXPKTS }
+       { &pf_pktdelay_pl, PF_PKTDELAY_MAXPKTS, PF_PKTDELAY_MAXPKTS },
+       { &pf_anchor_pl, PF_ANCHOR_HIWAT, PF_ANCHOR_HIWAT }
 };
 
 #define BOUND_IFACE(r, k) \

diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index a81d8a7..10062d1 100644 (file)
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf_ioctl.c,v 1.382 2022/06/26 11:37:08 mbuhl Exp $ */
+/*     $OpenBSD: pf_ioctl.c,v 1.383 2022/07/20 09:33:11 mbuhl Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -191,6 +191,8 @@ pfattach(int num)
            IPL_SOFTNET, 0, "pftag", NULL);
        pool_init(&pf_pktdelay_pl, sizeof(struct pf_pktdelay), 0,
            IPL_SOFTNET, 0, "pfpktdelay", NULL);
+       pool_init(&pf_anchor_pl, sizeof(struct pf_anchor), 0,
+           IPL_SOFTNET, 0, "pfanchor", NULL);
 
        hfsc_initialize();
        pfr_initialize();
@@ -200,6 +202,8 @@ pfattach(int num)
 
        pool_sethardlimit(pf_pool_limits[PF_LIMIT_STATES].pp,
            pf_pool_limits[PF_LIMIT_STATES].limit, NULL, 0);
+       pool_sethardlimit(pf_pool_limits[PF_LIMIT_ANCHORS].pp,
+           pf_pool_limits[PF_LIMIT_ANCHORS].limit, NULL, 0);
 
        if (physmem <= atop(100*1024*1024))
                pf_pool_limits[PF_LIMIT_TABLE_ENTRIES].limit =

diff --git a/sys/net/pf_ruleset.c b/sys/net/pf_ruleset.c
index f1e35b9..6131895 100644 (file)
--- a/sys/net/pf_ruleset.c
+++ b/sys/net/pf_ruleset.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf_ruleset.c,v 1.18 2018/12/27 16:54:01 kn Exp $ */
+/*     $OpenBSD: pf_ruleset.c,v 1.19 2022/07/20 09:33:11 mbuhl Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -40,6 +40,7 @@
 #ifdef _KERNEL
 #include <sys/systm.h>
 #include <sys/mbuf.h>
+#include <sys/pool.h>
 #endif /* _KERNEL */
 #include <sys/syslog.h>
 
@@ -58,6 +59,11 @@
 #ifdef _KERNEL
 #define rs_malloc(x)           malloc(x, M_TEMP, M_WAITOK|M_CANFAIL|M_ZERO)
 #define rs_free(x, siz)                free(x, M_TEMP, siz)
+#define rs_pool_get_anchor()   pool_get(&pf_anchor_pl, \
+                                   PR_WAITOK|PR_LIMITFAIL|PR_ZERO)
+#define rs_pool_put_anchor(x)  pool_put(&pf_anchor_pl, x)
+
+struct pool    pf_anchor_pl;
 
 #else  /* !_KERNEL */
 /* Userland equivalents so we can lend code to pfctl et al. */
@@ -67,8 +73,10 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#define rs_malloc(x)            calloc(1, x)
-#define rs_free(x, siz)                 freezero(x, siz)
+#define rs_malloc(x)           calloc(1, x)
+#define rs_free(x, siz)                freezero(x, siz)
+#define rs_pool_get_anchor()   calloc(1, sizeof(struct pf_anchor))
+#define rs_pool_put_anchor(x)  freezero(x, sizeof(struct pf_anchor))
 
 #ifdef PFDEBUG
 #include <sys/stdarg.h>        /* for DPFPRINTF() */
@@ -184,7 +192,7 @@ pf_create_anchor(struct pf_anchor *parent, const char *aname)
            ((parent != NULL) && (strlen(parent->path) >= PF_ANCHOR_MAXPATH)))
                return (NULL);
 
-       anchor = rs_malloc(sizeof(*anchor));
+       anchor = rs_pool_get_anchor();
        if (anchor == NULL)
                return (NULL);
 
@@ -204,7 +212,7 @@ pf_create_anchor(struct pf_anchor *parent, const char *aname)
                DPFPRINTF(LOG_NOTICE,
                    "%s: RB_INSERT to global '%s' '%s' collides with '%s' '%s'",
                    __func__, anchor->path, anchor->name, dup->path, dup->name);
-               rs_free(anchor, sizeof(*anchor));
+               rs_pool_put_anchor(anchor);
                return (NULL);
        }
 
@@ -218,7 +226,7 @@ pf_create_anchor(struct pf_anchor *parent, const char *aname)
                            dup->path, dup->name);
                        RB_REMOVE(pf_anchor_global, &pf_anchors,
                            anchor);
-                       rs_free(anchor, sizeof(*anchor));
+                       rs_pool_put_anchor(anchor);
                        return (NULL);
                }
        }
@@ -300,7 +308,7 @@ pf_remove_if_empty_ruleset(struct pf_ruleset *ruleset)
                if ((parent = ruleset->anchor->parent) != NULL)
                        RB_REMOVE(pf_anchor_node, &parent->children,
                            ruleset->anchor);
-               rs_free(ruleset->anchor, sizeof(*(ruleset->anchor)));
+               rs_pool_put_anchor(ruleset->anchor);
                if (parent == NULL)
                        return;
                ruleset = &parent->ruleset;

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 67c27ab..6f88ff6 100644 (file)
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pfvar.h,v 1.508 2022/06/26 11:37:08 mbuhl Exp $ */
+/*     $OpenBSD: pfvar.h,v 1.509 2022/07/20 09:33:11 mbuhl Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -136,7 +136,7 @@ enum        { PFTM_TCP_FIRST_PACKET, PFTM_TCP_OPENING, PFTM_TCP_ESTABLISHED,
 enum   { PF_NOPFROUTE, PF_ROUTETO, PF_DUPTO, PF_REPLYTO };
 enum   { PF_LIMIT_STATES, PF_LIMIT_SRC_NODES, PF_LIMIT_FRAGS,
          PF_LIMIT_TABLES, PF_LIMIT_TABLE_ENTRIES, PF_LIMIT_PKTDELAY_PKTS,
-         PF_LIMIT_MAX };
+         PF_LIMIT_ANCHORS, PF_LIMIT_MAX };
 #define PF_POOL_IDMASK         0x0f
 enum   { PF_POOL_NONE, PF_POOL_BITMASK, PF_POOL_RANDOM,
          PF_POOL_SRCHASH, PF_POOL_ROUNDROBIN, PF_POOL_LEASTSTATES };
@@ -476,6 +476,7 @@ union pf_rule_ptr {
 
 #define        PF_ANCHOR_NAME_SIZE      64
 #define        PF_ANCHOR_MAXPATH       (PATH_MAX - PF_ANCHOR_NAME_SIZE - 1)
+#define        PF_ANCHOR_HIWAT          512
 #define        PF_OPTIMIZER_TABLE_PFX  "__automatic_"
 
 struct pf_rule {
@@ -1703,7 +1704,7 @@ extern u_int32_t           ticket_pabuf;
 extern struct pool              pf_src_tree_pl, pf_sn_item_pl, pf_rule_pl;
 extern struct pool              pf_state_pl, pf_state_key_pl, pf_state_item_pl,
                                    pf_rule_item_pl, pf_queue_pl,
-                                   pf_pktdelay_pl;
+                                   pf_pktdelay_pl, pf_anchor_pl;
 extern struct pool              pf_state_scrub_pl;
 extern struct ifnet            *sync_ifp;
 extern struct pf_rule           pf_default_rule;