netfilter: xt_RATEEST: acquire xt_rateest_mutex for hash insert

commit 7dc68e98757a8eccf8ca7a53a29b896f1eef1f76 upstream.

rateest_hash is supposed to be protected by xt_rateest_mutex,
and, as suggested by Eric, lookup and insert should be atomic,
so we should acquire the xt_rateest_mutex once for both.

So introduce a non-locking helper for internal use and keep the
locking one for external.

Reported-by: <syzbot+5cb189720978275e4c75@syzkaller.appspotmail.com>
Fixes: 5859034d7eb8 ("[NETFILTER]: x_tables: add RATEEST target")
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Reviewed-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
index f264032..309a520 100644 (file)
--- a/net/netfilter/xt_RATEEST.c
+++ b/net/netfilter/xt_RATEEST.c
@@ -40,24 +40,32 @@ static void xt_rateest_hash_insert(struct xt_rateest *est)
        hlist_add_head(&est->list, &rateest_hash[h]);
 }
 
        hlist_add_head(&est->list, &rateest_hash[h]);
 }
 

-struct xt_rateest *xt_rateest_lookup(const char *name)
+static struct xt_rateest *__xt_rateest_lookup(const char *name)

 {
        struct xt_rateest *est;
        struct hlist_node *n;
        unsigned int h;
 
        h = xt_rateest_hash(name);
 {
        struct xt_rateest *est;
        struct hlist_node *n;
        unsigned int h;
 
        h = xt_rateest_hash(name);

-       mutex_lock(&xt_rateest_mutex);

        hlist_for_each_entry(est, n, &rateest_hash[h], list) {
                if (strcmp(est->name, name) == 0) {
                        est->refcnt++;
        hlist_for_each_entry(est, n, &rateest_hash[h], list) {
                if (strcmp(est->name, name) == 0) {
                        est->refcnt++;

-                       mutex_unlock(&xt_rateest_mutex);

                        return est;
                }
        }
                        return est;
                }
        }

-       mutex_unlock(&xt_rateest_mutex);
+

        return NULL;
 }
        return NULL;
 }

+
+struct xt_rateest *xt_rateest_lookup(const char *name)
+{
+       struct xt_rateest *est;
+
+       mutex_lock(&xt_rateest_mutex);
+       est = __xt_rateest_lookup(name);
+       mutex_unlock(&xt_rateest_mutex);
+       return est;
+}

 EXPORT_SYMBOL_GPL(xt_rateest_lookup);
 
 void xt_rateest_put(struct xt_rateest *est)
 EXPORT_SYMBOL_GPL(xt_rateest_lookup);
 
 void xt_rateest_put(struct xt_rateest *est)


@@ -105,8 +113,10 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
                rnd_inited = true;
        }
 
                rnd_inited = true;
        }
 

-       est = xt_rateest_lookup(info->name);
+       mutex_lock(&xt_rateest_mutex);
+       est = __xt_rateest_lookup(info->name);

        if (est) {
        if (est) {

+               mutex_unlock(&xt_rateest_mutex);

                /*
                 * If estimator parameters are specified, they must match the
                 * existing estimator.
                /*
                 * If estimator parameters are specified, they must match the
                 * existing estimator.


@@ -144,11 +154,13 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
 
        info->est = est;
        xt_rateest_hash_insert(est);
 
        info->est = est;
        xt_rateest_hash_insert(est);

+       mutex_unlock(&xt_rateest_mutex);

        return 0;
 
 err2:
        kfree(est);
 err1:
        return 0;
 
 err2:
        kfree(est);
 err1:

+       mutex_unlock(&xt_rateest_mutex);

        return ret;
 }
 
        return ret;
 }