netfilter: nft_connlimit: update the count if add was skipped

[ Upstream commit 69894e5b4c5e28cda5f32af33d4a92b7a4b93b0e ]

Connlimit expression can be used for all kind of packets and not only
for packets with connection state new. See this ruleset as example:

table ip filter {
        chain input {
                type filter hook input priority filter; policy accept;
                tcp dport 22 ct count over 4 counter
        }
}

Currently, if the connection count goes over the limit the counter will
count the packets. When a connection is closed, the connection count
won't decrement as it should because it is only updated for new
connections due to an optimization on __nf_conncount_add() that prevents
updating the list if the connection is duplicated.

To solve this problem, check whether the connection was skipped and if
so, update the list. Adjust count_tree() too so the same fix is applied
for xt_connlimit.

Fixes: 976afca1ceba ("netfilter: nf_conncount: Early exit in nf_conncount_lookup() and cleanup")
Closes: https://lore.kernel.org/netfilter/trinity-85c72a88-d762-46c3-be97-36f10e5d9796-1761173693813@3c-app-mailcom-bs12/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 0ffc5ff78a71426a3b8581c24bc2a78da7cca17b..b84cfb5616df4dab4219f09b06c2875336d89fe1 100644 (file)
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -179,7 +179,7 @@ static int __nf_conncount_add(struct net *net,
        if (ct && nf_ct_is_confirmed(ct)) {
                if (refcounted)
                        nf_ct_put(ct);
-               return 0;
+               return -EEXIST;
        }
 
        if ((u32)jiffies == list->last_gc)
@@ -398,7 +398,7 @@ restart:
                        int ret;
 
                        ret = nf_conncount_add_skb(net, skb, l3num, &rbconn->list);
-                       if (ret)
+                       if (ret && ret != -EEXIST)
                                count = 0; /* hotdrop */
                        else
                                count = rbconn->list.count;
@@ -501,10 +501,14 @@ count_tree(struct net *net,
                        /* same source network -> be counted! */
                        ret = __nf_conncount_add(net, skb, l3num, &rbconn->list);
                        spin_unlock_bh(&rbconn->list.list_lock);
-                       if (ret)
+                       if (ret && ret != -EEXIST) {
                                return 0; /* hotdrop */
-                       else
+                       } else {
+                               /* -EEXIST means add was skipped, update the list */
+                               if (ret == -EEXIST)
+                                       nf_conncount_gc_list(net, &rbconn->list);
                                return rbconn->list.count;
+                       }
                }
        }
 

diff --git a/net/netfilter/nft_connlimit.c b/net/netfilter/nft_connlimit.c
index 5df7134131d29f8bd0f1100ead0beca891f70e85..d4964087bbc5e245f2a23a5bcb8a762b0fd70d3b 100644 (file)
--- a/net/netfilter/nft_connlimit.c
+++ b/net/netfilter/nft_connlimit.c
@@ -29,8 +29,17 @@ static inline void nft_connlimit_do_eval(struct nft_connlimit *priv,
 
        err = nf_conncount_add_skb(nft_net(pkt), pkt->skb, nft_pf(pkt), priv->list);
        if (err) {
-               regs->verdict.code = NF_DROP;
-               return;
+               if (err == -EEXIST) {
+                       /* Call gc to update the list count if any connection has
+                        * been closed already. This is useful for softlimit
+                        * connections like limiting bandwidth based on a number
+                        * of open connections.
+                        */
+                       nf_conncount_gc_list(nft_net(pkt), priv->list);
+               } else {
+                       regs->verdict.code = NF_DROP;
+                       return;
+               }
        }
 
        count = READ_ONCE(priv->list->count);