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 c68d40b..0846307 100644 (file)
--- 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 50aaec5..6875f08 100644 (file)
--- 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 c37b5f8..9e86391 100644 (file)
--- 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 8b73f03..d64dec2 100644 (file)
--- 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);
 }