From: sashan <sashan@openbsd.org>
Date: Tue, 4 Jul 2023 11:34:19 +0000 (+0000)
Subject: The recent change to DIOCGETRULE allows applications which
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d2364f60079dfc6e8a6bc0ad89f58b3433f3bafe;p=openbsd

The recent change to DIOCGETRULE allows applications which
periodically read rules from pf(4) to consume all kernel
memory. The bug has been discovered and root caused by florian@.
In this particular case it was snmpd(8) what ate all kernel
memory.

This commit introduces DIOCXEND to pf(4) so applications such
as snmpd(8) and systat(1) to close ticket/transaction when
they are done with fetching the rules. This change also
updates snmpd(8) and systat(1) to use newly introduced
DIOCXEND ioctl(2).

OK claudio@, deraadt@, kn@
---

diff --git a/libexec/snmpd/snmpd_metrics/mib.c b/libexec/snmpd/snmpd_metrics/mib.c
index c68d40b1ec5..0846307f5d9 100644
--- a/libexec/snmpd/snmpd_metrics/mib.c
+++ b/libexec/snmpd/snmpd_metrics/mib.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mib.c,v 1.3 2023/04/19 12:58:15 jsg Exp $	*/
+/*	$OpenBSD: mib.c,v 1.4 2023/07/04 11:34:19 sashan Exp $	*/
 
 /*
  * Copyright (c) 2022 Martijn van Duren <martijn@openbsd.org>
@@ -1183,6 +1183,8 @@ struct carpif {
 	struct kif	 kif;
 };
 
+void	 mib_close_pftrans(struct agentx_varbind *, u_int32_t);
+
 void	 mib_pfinfo(struct agentx_varbind *);
 void	 mib_pfcounters(struct agentx_varbind *);
 void	 mib_pfscounters(struct agentx_varbind *);
@@ -1744,6 +1746,17 @@ mib_pftableaddrs(struct agentx_varbind *vb)
 		fatal("%s: Unexpected object", __func__);
 }
 
+void
+mib_close_pftrans(struct agentx_varbind *vb, u_int32_t ticket)
+{
+	extern int		devpf;
+
+	if (ioctl(devpf, DIOCXEND, &ticket) == -1) {
+		log_warn("DIOCXEND");
+		agentx_varbind_error(vb);
+	}
+}
+
 void
 mib_pflabelnum(struct agentx_varbind *vb)
 {
@@ -1765,6 +1778,7 @@ mib_pflabelnum(struct agentx_varbind *vb)
 		if (ioctl(devpf, DIOCGETRULE, &pr) == -1) {
 			log_warn("DIOCGETRULE");
 			agentx_varbind_error(vb);
+			mib_close_pftrans(vb, pr.ticket);
 			return;
 		}
 
@@ -1773,6 +1787,8 @@ mib_pflabelnum(struct agentx_varbind *vb)
 	}
 
 	agentx_varbind_integer(vb, lnr);
+
+	mib_close_pftrans(vb, pr.ticket);
 }
 
 void
@@ -1818,6 +1834,7 @@ mib_pflabels(struct agentx_varbind *vb)
 		if (ioctl(devpf, DIOCGETRULE, &pr) == -1) {
 			log_warn("DIOCGETRULE");
 			agentx_varbind_error(vb);
+			mib_close_pftrans(vb, pr.ticket);
 			return;
 		}
 
@@ -1827,6 +1844,8 @@ mib_pflabels(struct agentx_varbind *vb)
 		}
 	}
 
+	mib_close_pftrans(vb, pr.ticket);
+
 	if (r == NULL) {
 		agentx_varbind_notfound(vb);
 		return;
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 50aaec5dbf6..6875f08ee0a 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.412 2023/07/04 02:01:55 jsg Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.413 2023/07/04 11:34:19 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1144,6 +1144,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		case DIOCGETSRCNODES:
 		case DIOCIGETIFACES:
 		case DIOCGETSYNFLWATS:
+		case DIOCXEND:
 			break;
 		case DIOCRCLRTABLES:
 		case DIOCRADDTABLES:
@@ -2786,6 +2787,18 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		break;
 	}
 
+	case DIOCXEND: {
+		u_int32_t	*ticket = (u_int32_t *)addr;
+		struct pf_trans	*t;
+
+		t = pf_find_trans(minor(dev), *ticket);
+		if (t != NULL)
+			pf_rollback_trans(t);
+		else
+			error = ENXIO;
+		break;
+	}
+
 	case DIOCGETSRCNODES: {
 		struct pfioc_src_nodes	*psn = (struct pfioc_src_nodes *)addr;
 		struct pf_src_node	*n, *p, *pstore;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index c37b5f8ea2e..9e863915fbd 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.531 2023/05/26 12:13:26 kn Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.532 2023/07/04 11:34:19 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1573,6 +1573,7 @@ struct pfioc_synflwats {
 #define DIOCSETSYNFLWATS	_IOWR('D', 97, struct pfioc_synflwats)
 #define DIOCSETSYNCOOKIES	_IOWR('D', 98, u_int8_t)
 #define DIOCGETSYNFLWATS	_IOWR('D', 99, struct pfioc_synflwats)
+#define DIOCXEND	_IOWR('D', 100, u_int32_t)
 
 #ifdef _KERNEL
 
diff --git a/usr.bin/systat/pftop.c b/usr.bin/systat/pftop.c
index 8b73f03d76c..d64dec2e35e 100644
--- a/usr.bin/systat/pftop.c
+++ b/usr.bin/systat/pftop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: pftop.c,v 1.45 2019/10/17 21:54:29 millert Exp $	 */
+/* $OpenBSD: pftop.c,v 1.46 2023/07/04 11:34:19 sashan Exp $	 */
 /*
  * Copyright (c) 2001, 2007 Can Erkin Acar
  * Copyright (c) 2001 Daniel Hartmeier
@@ -942,6 +942,13 @@ add_rule_alloc(u_int32_t nr)
 
 int label_length;
 
+void
+close_pf_trans(u_int32_t ticket)
+{
+	if (ioctl(pf_dev, DIOCXEND, &ticket) == -1)
+		error("DIOCXEND: %s", strerror(errno));
+}
+
 int
 read_anchor_rules(char *anchor)
 {
@@ -968,6 +975,7 @@ read_anchor_rules(char *anchor)
 		pr.nr = nr;
 		if (ioctl(pf_dev, DIOCGETRULE, &pr) == -1) {
 			error("DIOCGETRULE: %s", strerror(errno));
+			close_pf_trans(pr.ticket);
 			return (-1);
 		}
 		/* XXX overload pr.anchor, to store a pointer to
@@ -979,6 +987,8 @@ read_anchor_rules(char *anchor)
 		rules[off + nr] = pr.rule;
 	}
 
+	close_pf_trans(pr.ticket);
+
 	return (num);
 }