netfilter: nft_set_rbtree: fix null deref on element insertion

[ Upstream commit 61ae320a29b0540c16931816299eb86bf2b66c08 ]

There is no guarantee that rb_prev() will not return NULL in nft_rbtree_gc_elem():

general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
 nft_add_set_elem+0x14b0/0x2990
  nf_tables_newsetelem+0x528/0xb30

Furthermore, there is a possible use-after-free while iterating,
'node' can be free'd so we need to cache the next value to use.

Fixes: c9e6978e2725 ("netfilter: nft_set_rbtree: Switch to node list walk for overlap detection")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 19ea4d3c35535552a3acf1ac18f33b91645f944a..2f114aa10f1a7ad18c540e1cc1beba0345e4e85a 100644 (file)
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -221,7 +221,7 @@ static int nft_rbtree_gc_elem(const struct nft_set *__set,
 {
        struct nft_set *set = (struct nft_set *)__set;
        struct rb_node *prev = rb_prev(&rbe->node);
-       struct nft_rbtree_elem *rbe_prev;
+       struct nft_rbtree_elem *rbe_prev = NULL;
        struct nft_set_gc_batch *gcb;
 
        gcb = nft_set_gc_batch_check(set, NULL, GFP_ATOMIC);
@@ -229,17 +229,21 @@ static int nft_rbtree_gc_elem(const struct nft_set *__set,
                return -ENOMEM;
 
        /* search for expired end interval coming before this element. */
-       do {
+       while (prev) {
                rbe_prev = rb_entry(prev, struct nft_rbtree_elem, node);
                if (nft_rbtree_interval_end(rbe_prev))
                        break;
 
                prev = rb_prev(prev);
-       } while (prev != NULL);
+       }
+
+       if (rbe_prev) {
+               rb_erase(&rbe_prev->node, &priv->root);
+               atomic_dec(&set->nelems);
+       }
 
-       rb_erase(&rbe_prev->node, &priv->root);
        rb_erase(&rbe->node, &priv->root);
-       atomic_sub(2, &set->nelems);
+       atomic_dec(&set->nelems);
 
        nft_set_gc_batch_add(gcb, rbe);
        nft_set_gc_batch_complete(gcb);
@@ -268,7 +272,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
                               struct nft_set_ext **ext)
 {
        struct nft_rbtree_elem *rbe, *rbe_le = NULL, *rbe_ge = NULL;
-       struct rb_node *node, *parent, **p, *first = NULL;
+       struct rb_node *node, *next, *parent, **p, *first = NULL;
        struct nft_rbtree *priv = nft_set_priv(set);
        u8 genmask = nft_genmask_next(net);
        int d, err;
@@ -307,7 +311,9 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
         * Values stored in the tree are in reversed order, starting from
         * highest to lowest value.
         */
-       for (node = first; node != NULL; node = rb_next(node)) {
+       for (node = first; node != NULL; node = next) {
+               next = rb_next(node);
+
                rbe = rb_entry(node, struct nft_rbtree_elem, node);
 
                if (!nft_set_elem_active(&rbe->ext, genmask))