Re: [PATCH] netfilter: xt_connlimit: fix race in connection counting

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Nov 19, 2018 at 10:17:38PM +0000, Alakesh Haloi wrote:
> An iptable rule like the following on a multicore systems will result in
> accepting more connections than set in the rule.
> 
> iptables  -A INPUT -p tcp -m tcp --syn --dport 7777 -m connlimit \
>       --connlimit-above 2000 --connlimit-mask 0 -j DROP
> 
> In check_hlist function, connections that are found in saved connections
> but not in netfilter conntrack are deleted, assuming that those
> connections do not exist anymore. But for multi core systems, there exists
> a small time window, when a connection has been added to the xt_connlimit
> maintained rb-tree but has not yet made to netfilter conntrack table. This
> causes concurrent connections to return incorrect counts and go over limit
> set in iptable rule.
> 
> Connection 1 on  Core 1                Connection 2 on Core 2
> 
> list_length = N
> conntrack_table_len = N
> spin_lock_bh()
> In check_hlist() function
> a. loop over saved connections
>   1.  call nf_conntrack_find_get()
>   2.  If not found in 1,
>     i.  call hlist_del()
> b. return total count to caller
> c. connection gets added to list
>    of  saved connections.
> spin_unlock_bh()
> 				      list_length = N + 1
>                                       spin_lock_bh()   on core 2
> 				      In check_hlist() function
>                                       a. loop over saved connection.
>                                         1. call nf_conntrack_find_get()
>                                         2. If not found in  1.
>                                           i.   call hlist_del()
> 					     [Connection 1 was in the
>                                               but not in nf_conntrack yet]
> 					  ii. connection 1 gets deleted
> 
> 				      list_length = N
> 				      conntrack_table_len = N
> 				      b. return total count to caller
> 				      c. connection 2 gets added to list
> 				      of saved connections
> 				      spin_unlock_bh()
> 
> d. connection 1 gets added to
>    nf_conntrack
> list_length = N + 1
> conntrack_table_len = N + 1
> 				      e. connection 2 gets added to
> 					 nf_conntrack
> 				      list_length = N + 1
> 				      conntrack_table_len = N + 2
> 
> So we end up with N + 1 connections in the list but N + 2 in nf_conntrack,
> allowing more number of connections eventually than set in the rule.
> 
> This fix adds an additional field to track such pending connections
> and prevent them from being deleted by another execution thread on
> a different core and returns correct count.
> 
> Signed-off-by: Alakesh Haloi <alakeshh@amazon.com>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: stable@vger.kernel.org # v4.15 and before
> ---
>  net/netfilter/xt_connlimit.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)

What is the git commit id of this patch in Linus's tree?

thanks,

greg k-h