From: mbuhl Date: Wed, 20 Jul 2022 09:33:11 +0000 (+0000) Subject: Add a pool for the allocation of the pf_anchor struct. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=fa90ac5c787b88bcd714c01650b5cded334d9725;p=openbsd 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 9756425ab0f..a016297bd94 100644 --- 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 db90604c06c..6d3d1865ab3 100644 --- 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 a7b1e1bc52f..7183db91254 100644 --- 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 a81d8a7876c..10062d16939 100644 --- 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 f1e35b9410a..6131895751e 100644 --- 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 #include +#include #endif /* _KERNEL */ #include @@ -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 #include #include -#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 /* 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 67c27abd4b3..6f88ff687ed 100644 --- 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;