netfilter: nft_connlimit: update the count if add was skipped

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>

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index d8893e172444d23940bee132d5baf9b129c11839..f1be4dd5cf85f5198233d365adffbbd91676bd43 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)
@@ -408,7 +408,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;
@@ -511,10 +511,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 41770bde39d337dfa057e2f66871de3a2887fb78..714a594859357fa319f377ee175e74e50e65fa85 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);