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@xxxxxxxxxx> > Cc: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > Cc: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx> > Cc: Florian Westphal <fw@xxxxxxxxx> > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx # 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
