Thanks for the quick confirmation.
PR closed.
On 5 May 2017 at 17:55, Mike Belopuhov <m...@belopuhov.com> wrote:
> 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
>>
>> dera...@amd64.openbsd.org:/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);
