Dear tech@,
I am investigating a syzkaller reproducer found in the
"no output from test machine (7)" crashes:
https://syzkaller.appspot.com/bug?id=d93e92fde3857c69df2cf46b4244d9814c4318a7
https://syzkaller.appspot.com/text?tag=ReproC&x=116ee2e0080000
The code calls DIOCXBEGIN, with different anchor names
until the malloc bucket for size 2048 fills up.
So the following reproducer does the same:
#include <sys/ioctl.h>
#include <net/if.h>
#include <net/pfvar.h>
#include <err.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
int calls;
void
iterate(char *str)
{
char *end = str + 64;
*end = '\0';
while (*str == '\255') {
*str = '\1';
str++;
}
if (str == end) return;
if (*str == '\0') *str = '\1';
*str = ++*str;
}
int
main(void)
{
struct pfioc_trans trans;
struct pfioc_trans_e elem;
int fd, ret;
char it[64] = "\0";
fd = open("/dev/pf", O_RDWR);
while (1) {
trans.size = 1;
trans.esize = 0x408;
trans.array = &elem;
elem.type = 0;
iterate(it);
strcpy(elem.anchor, it);
elem.ticket = 0;
calls++;
if (ioctl(fd, DIOCXBEGIN, &trans) == -1)
err(1, "icoctl: %d calls, '%s'", calls, elem.anchor);
}
}
A solution would be to move the allocation of the pf_anchor struct
into a pool. One final question would be regarding the size of the
hiwat or hardlimit. Any suggestions?
Any other comments?
mbuhl
Index: sys/net/pf.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1135
diff -u -p -r1.1135 pf.c
--- sys/net/pf.c 28 Jun 2022 13:48:06 -0000 1.1135
+++ sys/net/pf.c 19 Jul 2022 11:34:23 -0000
@@ -269,6 +269,7 @@ void pf_log_matches(struct pf_pdesc
*
extern struct pool pfr_ktable_pl;
extern struct pool pfr_kentry_pl;
+extern struct pool pf_anchor_pl;
struct pf_pool_limit pf_pool_limits[PF_LIMIT_MAX] = {
{ &pf_state_pl, PFSTATE_HIWAT, PFSTATE_HIWAT },
@@ -276,7 +277,8 @@ struct pf_pool_limit pf_pool_limits[PF_L
{ &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) \
Index: sys/net/pf_ioctl.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.382
diff -u -p -r1.382 pf_ioctl.c
--- sys/net/pf_ioctl.c 26 Jun 2022 11:37:08 -0000 1.382
+++ sys/net/pf_ioctl.c 19 Jul 2022 16:56:23 -0000
@@ -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, PR_WAITOK, "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 =
Index: sys/net/pf_ruleset.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sys/net/pf_ruleset.c,v
retrieving revision 1.18
diff -u -p -r1.18 pf_ruleset.c
--- sys/net/pf_ruleset.c 27 Dec 2018 16:54:01 -0000 1.18
+++ sys/net/pf_ruleset.c 19 Jul 2022 12:56:35 -0000
@@ -40,6 +40,7 @@
#ifdef _KERNEL
#include <sys/systm.h>
#include <sys/mbuf.h>
+#include <sys/pool.h>
#endif /* _KERNEL */
#include <sys/syslog.h>
@@ -55,6 +56,7 @@
#endif /* INET6 */
+struct pool pf_anchor_pl;
#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)
@@ -107,12 +109,12 @@ pf_find_anchor(const char *path)
{
struct pf_anchor *key, *found;
- key = rs_malloc(sizeof(*key));
+ key = pool_get(&pf_anchor_pl, PR_NOWAIT | PR_ZERO);
if (key == NULL)
return (NULL);
strlcpy(key->path, path, sizeof(key->path));
found = RB_FIND(pf_anchor_global, &pf_anchors, key);
- rs_free(key, sizeof(*key));
+ pool_put(&pf_anchor_pl, key);
return (found);
}
@@ -184,7 +186,7 @@ pf_create_anchor(struct pf_anchor *paren
((parent != NULL) && (strlen(parent->path) >= PF_ANCHOR_MAXPATH)))
return (NULL);
- anchor = rs_malloc(sizeof(*anchor));
+ anchor = pool_get(&pf_anchor_pl, PR_NOWAIT | PR_ZERO);
if (anchor == NULL)
return (NULL);
@@ -204,7 +206,7 @@ pf_create_anchor(struct pf_anchor *paren
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));
+ pool_put(&pf_anchor_pl, anchor);
return (NULL);
}
@@ -218,7 +220,7 @@ pf_create_anchor(struct pf_anchor *paren
dup->path, dup->name);
RB_REMOVE(pf_anchor_global, &pf_anchors,
anchor);
- rs_free(anchor, sizeof(*anchor));
+ pool_put(&pf_anchor_pl, anchor);
return (NULL);
}
}
@@ -300,7 +302,7 @@ pf_remove_if_empty_ruleset(struct pf_rul
if ((parent = ruleset->anchor->parent) != NULL)
RB_REMOVE(pf_anchor_node, &parent->children,
ruleset->anchor);
- rs_free(ruleset->anchor, sizeof(*(ruleset->anchor)));
+ pool_put(&pf_anchor_pl, ruleset->anchor);
if (parent == NULL)
return;
ruleset = &parent->ruleset;
Index: sys/net/pfvar.h
===================================================================
RCS file: /mount/openbsd/cvs/src/sys/net/pfvar.h,v
retrieving revision 1.508
diff -u -p -r1.508 pfvar.h
--- sys/net/pfvar.h 26 Jun 2022 11:37:08 -0000 1.508
+++ sys/net/pfvar.h 19 Jul 2022 16:34:54 -0000
@@ -136,7 +136,7 @@ enum { PFTM_TCP_FIRST_PACKET, PFTM_TCP_O
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 10
#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;
