From: sashan <sashan@openbsd.org>
Date: Tue, 4 Jul 2023 14:23:38 +0000 (+0000)
Subject: This diff limits the number of transactions/tickets
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=95c411ffdbcf2ce623d0736f4d65c433aa9cb37d;p=openbsd

This diff limits the number of transactions/tickets
pf_open_trans() can issue for each clone of /dev/pf
to 512. The pf_open_trans() is currently being used
by DIOCGETRULES ioctl(2). The limit avoids processes
to consume all kernel memory by asking DIOCGETRULES
for more tickets. If DIOCGETRULES hits the limit, then
the application will see EBUSY error.

This diff was fine tuned with feedback from cluadio@,
deraadt@ and kn@.

OK kn@
---

diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 6875f08ee0a..f20632df590 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.413 2023/07/04 11:34:19 sashan Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.414 2023/07/04 14:23:38 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -179,6 +179,10 @@ void			 pf_rtlabel_copyout(struct pf_addr_wrap *);
 
 LIST_HEAD(, pf_trans)	pf_ioctl_trans = LIST_HEAD_INITIALIZER(pf_trans);
 
+/* counts transactions opened by a device */
+unsigned int pf_tcount[CLONE_MAPSZ * NBBY];
+#define pf_unit2idx(_unit_)	((_unit_) >> CLONE_SHIFT)
+
 void
 pfattach(int num)
 {
@@ -1492,6 +1496,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		NET_UNLOCK();
 
 		t = pf_open_trans(minor(dev));
+		if (t == NULL) {
+			error = EBUSY;
+			goto fail;
+		}
 		pf_init_tgetrule(t, ruleset->anchor, ruleset_version, rule);
 		pr->ticket = t->pft_ticket;
 
@@ -3276,9 +3284,14 @@ pf_open_trans(uint32_t unit)
 
 	rw_assert_wrlock(&pfioctl_rw);
 
+	KASSERT(pf_unit2idx(unit) < nitems(pf_tcount));
+	if (pf_tcount[pf_unit2idx(unit)] >= (PF_ANCHOR_STACK_MAX * 8))
+		return (NULL);
+
 	t = malloc(sizeof(*t), M_PF, M_WAITOK|M_ZERO);
 	t->pft_unit = unit;
 	t->pft_ticket = ticket++;
+	pf_tcount[pf_unit2idx(unit)]++;
 
 	LIST_INSERT_HEAD(&pf_ioctl_trans, t, pft_entry);
 
@@ -3332,6 +3345,11 @@ pf_free_trans(struct pf_trans *t)
 		log(LOG_ERR, "%s unknown transaction type: %d\n",
 		    __func__, t->pft_type);
 	}
+
+	KASSERT(pf_unit2idx(t->pft_unit) < nitems(pf_tcount));
+	KASSERT(pf_tcount[pf_unit2idx(t->pft_unit)] >= 1);
+	pf_tcount[pf_unit2idx(t->pft_unit)]--;
+
 	free(t, M_PF, sizeof(*t));
 }