Close all pf transactions before opening a new one in DIOCGETRULES.

Processes like snmpd or systat open pf(4) once and then issue many
DIOCGETRULES calls over their runtime. This accumulates many pf_trans
structs over their lifetime. At some point the kernel runs out of
memory because of that. By closing all transactions before creating
a new one, long living processes do no longer leak transactions.

This probably needs further refinement once more transactions types are
added but for now this solves the problem.

Problem found by florian@
OK sashan@ kn@

diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index e5b930a..36779cf 100644 (file)
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf_ioctl.c,v 1.405 2023/05/26 12:13:26 kn Exp $ */
+/*     $OpenBSD: pf_ioctl.c,v 1.406 2023/06/26 07:49:48 claudio Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -118,6 +118,7 @@ int                  pf_states_clr(struct pfioc_state_kill *);
 int                     pf_states_get(struct pfioc_states *);
 
 struct pf_trans                *pf_open_trans(uint32_t);
+void                    pf_close_all_trans(uint32_t);
 struct pf_trans                *pf_find_trans(uint32_t, uint64_t);
 void                    pf_free_trans(struct pf_trans *);
 void                    pf_rollback_trans(struct pf_trans *);
@@ -1491,6 +1492,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
                PF_UNLOCK();
                NET_UNLOCK();
 
+               pf_close_all_trans(minor(dev));
                t = pf_open_trans(minor(dev));
                pf_init_tgetrule(t, ruleset->anchor, ruleset_version, rule);
                pr->ticket = t->pft_ticket;
@@ -3273,6 +3275,19 @@ pf_open_trans(uint32_t unit)
        return (t);
 }
 
+void
+pf_close_all_trans(uint32_t unit)
+{
+       struct pf_trans *t, *nt;
+
+       rw_assert_wrlock(&pfioctl_rw);
+
+       LIST_FOREACH_SAFE(t, &pf_ioctl_trans, pft_entry, nt) {
+               if (t->pft_unit == unit)
+                       pf_rollback_trans(t);
+       }
+}
+
 struct pf_trans *
 pf_find_trans(uint32_t unit, uint64_t ticket)
 {

diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c
index fbd4177..74736ce 100644 (file)
--- a/sys/net/rtsock.c
+++ b/sys/net/rtsock.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: rtsock.c,v 1.365 2023/04/20 21:43:17 mvs Exp $        */
+/*     $OpenBSD: rtsock.c,v 1.366 2023/06/26 07:49:48 claudio Exp $    */
 /*     $NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $  */
 
 /*
@@ -71,7 +71,7 @@
 #include <sys/domain.h>
 #include <sys/pool.h>
 #include <sys/protosw.h>
-#include <sys/srp.h>
+#include <sys/smr.h>
 
 #include <net/if.h>
 #include <net/if_dl.h>
@@ -107,8 +107,6 @@ struct walkarg {
 };
 
 void   route_prinit(void);
-void   rcb_ref(void *, void *);
-void   rcb_unref(void *, void *);
 int    route_output(struct mbuf *, struct socket *);
 int    route_ctloutput(int, struct socket *, int, int, struct mbuf *);
 int    route_attach(struct socket *, int, int);
@@ -155,7 +153,7 @@ int          rt_setsource(unsigned int, struct sockaddr *);
 struct rtpcb {
        struct socket           *rop_socket;            /* [I] */
 
-       SRPL_ENTRY(rtpcb)       rop_list;
+       SMR_LIST_ENTRY(rtpcb)   rop_list;
        struct refcnt           rop_refcnt;
        struct timeout          rop_timeout;
        unsigned int            rop_msgfilter;          /* [s] */
@@ -168,8 +166,7 @@ struct rtpcb {
 #define        sotortpcb(so)   ((struct rtpcb *)(so)->so_pcb)
 
 struct rtptable {
-       SRPL_HEAD(, rtpcb)      rtp_list;
-       struct srpl_rc          rtp_rc;
+       SMR_LIST_HEAD(, rtpcb)  rtp_list;
        struct rwlock           rtp_lk;
        unsigned int            rtp_count;
 };
@@ -191,29 +188,12 @@ struct rtptable rtptable;
 void
 route_prinit(void)
 {
-       srpl_rc_init(&rtptable.rtp_rc, rcb_ref, rcb_unref, NULL);
        rw_init(&rtptable.rtp_lk, "rtsock");
-       SRPL_INIT(&rtptable.rtp_list);
+       SMR_LIST_INIT(&rtptable.rtp_list);
        pool_init(&rtpcb_pool, sizeof(struct rtpcb), 0,
            IPL_SOFTNET, PR_WAITOK, "rtpcb", NULL);
 }
 
-void
-rcb_ref(void *null, void *v)
-{
-       struct rtpcb *rop = v;
-
-       refcnt_take(&rop->rop_refcnt);
-}
-
-void
-rcb_unref(void *null, void *v)
-{
-       struct rtpcb *rop = v;
-
-       refcnt_rele_wake(&rop->rop_refcnt);
-}
-
 int
 route_attach(struct socket *so, int proto, int wait)
 {
@@ -232,7 +212,7 @@ route_attach(struct socket *so, int proto, int wait)
            PR_ZERO);
        if (rop == NULL)
                return (ENOBUFS);
-       so->so_pcb = rop;
+
        /* Init the timeout structure */
        timeout_set_proc(&rop->rop_timeout, rtm_senddesync_timer, so);
        refcnt_init(&rop->rop_refcnt);
@@ -242,12 +222,12 @@ route_attach(struct socket *so, int proto, int wait)
 
        rop->rop_rtableid = curproc->p_p->ps_rtableid;
 
+       so->so_pcb = rop;
        soisconnected(so);
        so->so_options |= SO_USELOOPBACK;
 
        rw_enter(&rtptable.rtp_lk, RW_WRITE);
-       SRPL_INSERT_HEAD_LOCKED(&rtptable.rtp_rc, &rtptable.rtp_list, rop,
-           rop_list);
+       SMR_LIST_INSERT_HEAD_LOCKED(&rtptable.rtp_list, rop, rop_list);
        rtptable.rtp_count++;
        rw_exit(&rtptable.rtp_lk);
 
@@ -268,13 +248,13 @@ route_detach(struct socket *so)
        rw_enter(&rtptable.rtp_lk, RW_WRITE);
 
        rtptable.rtp_count--;
-       SRPL_REMOVE_LOCKED(&rtptable.rtp_rc, &rtptable.rtp_list, rop, rtpcb,
-           rop_list);
+       SMR_LIST_REMOVE_LOCKED(rop, rop_list);
        rw_exit(&rtptable.rtp_lk);
 
        sounlock(so);
 
        /* wait for all references to drop */
+       smr_barrier();
        refcnt_finalize(&rop->rop_refcnt, "rtsockrefs");
        timeout_del_barrier(&rop->rop_timeout);
 
@@ -372,6 +352,8 @@ route_ctloutput(int op, struct socket *so, int level, int optname,
        if (level != AF_ROUTE)
                return (EINVAL);
 
+       soassertlocked(so);
+
        switch (op) {
        case PRCO_SETOPT:
                switch (optname) {
@@ -493,10 +475,9 @@ void
 route_input(struct mbuf *m0, struct socket *so0, sa_family_t sa_family)
 {
        struct socket *so;
-       struct rtpcb *rop;
+       struct rtpcb *rop, *nrop;
        struct rt_msghdr *rtm;
        struct mbuf *m = m0;
-       struct srp_ref sr;
 
        /* ensure that we can access the rtm_type via mtod() */
        if (m->m_len < offsetof(struct rt_msghdr, rtm_type) + 1) {
@@ -504,17 +485,22 @@ route_input(struct mbuf *m0, struct socket *so0, sa_family_t sa_family)
                return;
        }
 
-       SRPL_FOREACH(rop, &sr, &rtptable.rtp_list, rop_list) {
+       smr_read_enter();
+       rop = SMR_LIST_FIRST(&rtptable.rtp_list);
+       while (rop != NULL) {
                /*
                 * If route socket is bound to an address family only send
                 * messages that match the address family. Address family
                 * agnostic messages are always sent.
                 */
                if (sa_family != AF_UNSPEC && rop->rop_proto != AF_UNSPEC &&
-                   rop->rop_proto != sa_family)
+                   rop->rop_proto != sa_family) {
+                       rop = SMR_LIST_NEXT(rop, rop_list);
                        continue;
+               }
 
-
+               refcnt_take(&rop->rop_refcnt);
+               smr_read_leave();
                so = rop->rop_socket;
                solock(so);
 
@@ -574,8 +560,12 @@ route_input(struct mbuf *m0, struct socket *so0, sa_family_t sa_family)
                rtm_sendup(so, m);
 next:
                sounlock(so);
+               smr_read_enter();
+               nrop = SMR_LIST_NEXT(rop, rop_list);
+               refcnt_rele_wake(&rop->rop_refcnt);
+               rop = nrop;
        }
-       SRPL_LEAVE(&sr);
+       smr_read_leave();
 
        m_freem(m);
 }