From bc25482273e20358a0986a64a86a5586a415f488 Mon Sep 17 00:00:00 2001 From: claudio Date: Mon, 26 Jun 2023 07:49:48 +0000 Subject: [PATCH] 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@ --- sys/net/pf_ioctl.c | 17 ++++++++++++- sys/net/rtsock.c | 62 +++++++++++++++++++--------------------------- 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c index e5b930a978a..36779cfdfd3 100644 --- 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 fbd4177b177..74736ce52cf 100644 --- 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 #include #include -#include +#include #include #include @@ -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); } -- 2.20.1