netfilter: ipset: Fix data race between add and dump in all hash types

When adding a new entry to the next position in the existing hash bucket,
the position index was incremented too early and parallel dump could
read it before the entry was populated with the value. Move the setting
of the position index after populating the entry.

v2: Position counting fixed, noticed by Florian Westphal.

Fixes: 18f84d41d34f ("netfilter: ipset: Introduce RCU locking in hash:* types")
Reported-by: syzbot+786c889f046e8b003ca6@syzkaller.appspotmail.com
Reported-by: syzbot+1da17e4b41d795df059e@syzkaller.appspotmail.com
Reported-by: syzbot+421c5f3ff8e9493084d9@syzkaller.appspotmail.com
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index b79e5dd2af0367905d92fd0927d856eec675bb86..133ce4611eedb08b9f935e6b28b615361d2a487a 100644 (file)
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -844,7 +844,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
        const struct mtype_elem *d = value;
        struct mtype_elem *data;
        struct hbucket *n, *old = ERR_PTR(-ENOENT);
-       int i, j = -1, ret;
+       int i, j = -1, npos = 0, ret;
        bool flag_exist = flags & IPSET_FLAG_EXIST;
        bool deleted = false, forceadd = false, reuse = false;
        u32 r, key, multi = 0, elements, maxelem;
@@ -889,6 +889,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
                        ext_size(AHASH_INIT_SIZE, set->dsize);
                goto copy_elem;
        }
+       npos = n->pos;
        for (i = 0; i < n->pos; i++) {
                if (!test_bit(i, n->used)) {
                        /* Reuse first deleted entry */
@@ -962,7 +963,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
        }
 
 copy_elem:
-       j = n->pos++;
+       j = npos;
+       npos = n->pos + 1;
        data = ahash_data(n, j, set->dsize);
 copy_data:
        t->hregion[r].elements++;
@@ -985,6 +987,7 @@ overwrite_extensions:
        if (SET_WITH_TIMEOUT(set))
                ip_set_timeout_set(ext_timeout(data, set), ext->timeout);
        smp_mb__before_atomic();
+       n->pos = npos;
        set_bit(j, n->used);
        if (old != ERR_PTR(-ENOENT)) {
                rcu_assign_pointer(hbucket(t, key), n);