On Fri, May 05, 2017 at 16:21 +0200, Remi Barbier wrote:
> >Synopsis: PF: Evergrowing source tracking table.
> >Category: PF
> >Environment:
> System : OpenBSD 6.1
> Details : OpenBSD 6.1-current (GENERIC.MP) #50: Thu May 4
> 11:52:48 MDT 2017
>
> [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> <Expired entries in the source tracking table never get
> removed. Both current and 6.1 are impacted.>
> >How-To-Repeature:
> <Tested with tcp, ssh connections. Any rule tracking the sources.
> As shown by pfctl -vsS and pfctl -vsi the expired entries are not
> removed. pfctl -F Sources won't help.
>
> It seems that in the move from pf.c rev 1.1018 to 1.1019, calling
> pf_remove_src_node() was omitted:
>
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1018
> retrieving revision 1.1019
> diff -u -r1.1018 -r1.1019
> --- src/sys/net/pf.c 2017/03/09 05:47:28 1.1018
> +++ src/sys/net/pf.c 2017/03/17 17:19:16 1.1019
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: pf.c,v 1.1018 2017/03/09 05:47:28 claudio Exp $ */
> +/* $OpenBSD: pf.c,v 1.1019 2017/03/17 17:19:16 mpi Exp $ */
>
> /*
> * Copyright (c) 2001 Daniel Hartmeier
>
>
> @@ -1235,20 +1241,26 @@
> }
>
> void
> -pf_purge_expired_src_nodes(void)
> +pf_purge_expired_src_nodes(int waslocked)
> {
> struct pf_src_node *cur, *next;
> + int locked = waslocked;
>
> - NET_ASSERT_LOCKED();
> -
> for (cur = RB_MIN(pf_src_tree, &tree_src_tracking); cur; cur = next) {
> next = RB_NEXT(pf_src_tree, &tree_src_tracking, cur);
>
> if (cur->states == 0 && cur->expire <= time_uptime) {
> - next = RB_NEXT(pf_src_tree, &tree_src_tracking, cur);
> - pf_remove_src_node(cur);
> + if (! locked) {
> + rw_enter_write(&pf_consistency_lock);
> + next = RB_NEXT(pf_src_tree,
> + &tree_src_tracking, cur);
> + locked = 1;
> + }
> }
> }
> +
> + if (locked && !waslocked)
> + rw_exit_write(&pf_consistency_lock);
> }
> >
Thanks for the report. You're absolutely right.
diff --git sys/net/pf.c sys/net/pf.c
index 02af280c9ec..2af0c2da6c6 100644
--- sys/net/pf.c
+++ sys/net/pf.c
@@ -1254,10 +1254,11 @@ pf_purge_expired_src_nodes(int waslocked)
rw_enter_write(&pf_consistency_lock);
next = RB_NEXT(pf_src_tree,
&tree_src_tracking, cur);
locked = 1;
}
+ pf_remove_src_node(cur);
}
}
if (locked && !waslocked)
rw_exit_write(&pf_consistency_lock);
