Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf

Pablo Neira Ayuso says:

====================
Netfilter fixes for net

The following patchset contains Netfilter fixes for your net tree:

1) Fix list corruption in device notifier in the masquerade
   infrastructure, from Florian Westphal.

2) Fix double-free of sets and use-after-free when deleting elements.

3) Don't bogusly return EBUSY when removing a set after flush command.

4) Use-after-free in dynamically allocate operations.

5) Don't report a new ruleset generation to userspace if transaction
   list is empty, this invalidates the userspace cache innecessarily.
   From Florian Westphal.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index c331e96a713b411f9a68e281c53c7ccbf7e06721..3e9ab643eedfe3056d74e4b247dfe08a0c153c06 100644 (file)
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -382,6 +382,7 @@ void nft_unregister_set(struct nft_set_type *type);
  *     @dtype: data type (verdict or numeric type defined by userspace)
  *     @objtype: object type (see NFT_OBJECT_* definitions)
  *     @size: maximum set size
+ *     @use: number of rules references to this set
  *     @nelems: number of elements
  *     @ndeact: number of deactivated elements queued for removal
  *     @timeout: default timeout value in jiffies
@@ -407,6 +408,7 @@ struct nft_set {
        u32                             dtype;
        u32                             objtype;
        u32                             size;
+       u32                             use;
        atomic_t                        nelems;
        u32                             ndeact;
        u64                             timeout;
@@ -416,7 +418,8 @@ struct nft_set {
        unsigned char                   *udata;
        /* runtime data below here */
        const struct nft_set_ops        *ops ____cacheline_aligned;
-       u16                             flags:14,
+       u16                             flags:13,
+                                       bound:1,
                                        genmask:2;
        u8                              klen;
        u8                              dlen;
@@ -466,6 +469,10 @@ struct nft_set_binding {
        u32                             flags;
 };
 
+enum nft_trans_phase;
+void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
+                             struct nft_set_binding *binding,
+                             enum nft_trans_phase phase);
 int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
                       struct nft_set_binding *binding);
 void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
@@ -1344,15 +1351,12 @@ struct nft_trans_rule {
 struct nft_trans_set {
        struct nft_set                  *set;
        u32                             set_id;
-       bool                            bound;
 };
 
 #define nft_trans_set(trans)   \
        (((struct nft_trans_set *)trans->data)->set)
 #define nft_trans_set_id(trans)        \
        (((struct nft_trans_set *)trans->data)->set_id)
-#define nft_trans_set_bound(trans)     \
-       (((struct nft_trans_set *)trans->data)->bound)
 
 struct nft_trans_chain {
        bool                            update;

diff --git a/net/netfilter/nf_nat_masquerade.c b/net/netfilter/nf_nat_masquerade.c
index 86fa4dcc63c555f3a5ed8a71dc78d5e47df27fb9..d85c4d902e7b1deca67975abdf13b3bf8326351f 100644 (file)
--- a/net/netfilter/nf_nat_masquerade.c
+++ b/net/netfilter/nf_nat_masquerade.c
@@ -11,7 +11,8 @@
 #include <net/netfilter/ipv6/nf_nat_masquerade.h>
 
 static DEFINE_MUTEX(masq_mutex);
-static unsigned int masq_refcnt __read_mostly;
+static unsigned int masq_refcnt4 __read_mostly;
+static unsigned int masq_refcnt6 __read_mostly;
 
 unsigned int
 nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
@@ -141,8 +142,13 @@ int nf_nat_masquerade_ipv4_register_notifier(void)
        int ret = 0;
 
        mutex_lock(&masq_mutex);
+       if (WARN_ON_ONCE(masq_refcnt4 == UINT_MAX)) {
+               ret = -EOVERFLOW;
+               goto out_unlock;
+       }
+
        /* check if the notifier was already set */
-       if (++masq_refcnt > 1)
+       if (++masq_refcnt4 > 1)
                goto out_unlock;
 
        /* Register for device down reports */
@@ -160,7 +166,7 @@ int nf_nat_masquerade_ipv4_register_notifier(void)
 err_unregister:
        unregister_netdevice_notifier(&masq_dev_notifier);
 err_dec:
-       masq_refcnt--;
+       masq_refcnt4--;
 out_unlock:
        mutex_unlock(&masq_mutex);
        return ret;
@@ -171,7 +177,7 @@ void nf_nat_masquerade_ipv4_unregister_notifier(void)
 {
        mutex_lock(&masq_mutex);
        /* check if the notifier still has clients */
-       if (--masq_refcnt > 0)
+       if (--masq_refcnt4 > 0)
                goto out_unlock;
 
        unregister_netdevice_notifier(&masq_dev_notifier);
@@ -321,25 +327,23 @@ int nf_nat_masquerade_ipv6_register_notifier(void)
        int ret = 0;
 
        mutex_lock(&masq_mutex);
-       /* check if the notifier is already set */
-       if (++masq_refcnt > 1)
+       if (WARN_ON_ONCE(masq_refcnt6 == UINT_MAX)) {
+               ret = -EOVERFLOW;
                goto out_unlock;
+       }
 
-       ret = register_netdevice_notifier(&masq_dev_notifier);
-       if (ret)
-               goto err_dec;
+       /* check if the notifier is already set */
+       if (++masq_refcnt6 > 1)
+               goto out_unlock;
 
        ret = register_inet6addr_notifier(&masq_inet6_notifier);
        if (ret)
-               goto err_unregister;
+               goto err_dec;
 
        mutex_unlock(&masq_mutex);
        return ret;
-
-err_unregister:
-       unregister_netdevice_notifier(&masq_dev_notifier);
 err_dec:
-       masq_refcnt--;
+       masq_refcnt6--;
 out_unlock:
        mutex_unlock(&masq_mutex);
        return ret;
@@ -350,11 +354,10 @@ void nf_nat_masquerade_ipv6_unregister_notifier(void)
 {
        mutex_lock(&masq_mutex);
        /* check if the notifier still has clients */
-       if (--masq_refcnt > 0)
+       if (--masq_refcnt6 > 0)
                goto out_unlock;
 
        unregister_inet6addr_notifier(&masq_inet6_notifier);
-       unregister_netdevice_notifier(&masq_dev_notifier);
 out_unlock:
        mutex_unlock(&masq_mutex);
 }

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index faf6bd10a19f91d561a2be34e497d35ae20e033f..513f931186043f2ded3f1844374decd79768c44b 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -142,7 +142,7 @@ static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set)
        list_for_each_entry_reverse(trans, &net->nft.commit_list, list) {
                if (trans->msg_type == NFT_MSG_NEWSET &&
                    nft_trans_set(trans) == set) {
-                       nft_trans_set_bound(trans) = true;
+                       set->bound = true;
                        break;
                }
        }
@@ -2162,9 +2162,11 @@ err1:
 static void nf_tables_expr_destroy(const struct nft_ctx *ctx,
                                   struct nft_expr *expr)
 {
+       const struct nft_expr_type *type = expr->ops->type;
+
        if (expr->ops->destroy)
                expr->ops->destroy(ctx, expr);
-       module_put(expr->ops->type->owner);
+       module_put(type->owner);
 }
 
 struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
@@ -3672,6 +3674,9 @@ err1:
 
 static void nft_set_destroy(struct nft_set *set)
 {
+       if (WARN_ON(set->use > 0))
+               return;
+
        set->ops->destroy(set);
        module_put(to_set_type(set->ops)->owner);
        kfree(set->name);
@@ -3712,7 +3717,7 @@ static int nf_tables_delset(struct net *net, struct sock *nlsk,
                NL_SET_BAD_ATTR(extack, attr);
                return PTR_ERR(set);
        }
-       if (!list_empty(&set->bindings) ||
+       if (set->use ||
            (nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0)) {
                NL_SET_BAD_ATTR(extack, attr);
                return -EBUSY;
@@ -3742,6 +3747,9 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
        struct nft_set_binding *i;
        struct nft_set_iter iter;
 
+       if (set->use == UINT_MAX)
+               return -EOVERFLOW;
+
        if (!list_empty(&set->bindings) && nft_set_is_anonymous(set))
                return -EBUSY;
 
@@ -3769,6 +3777,7 @@ bind:
        binding->chain = ctx->chain;
        list_add_tail_rcu(&binding->list, &set->bindings);
        nft_set_trans_bind(ctx, set);
+       set->use++;
 
        return 0;
 }
@@ -3788,6 +3797,25 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
 }
 EXPORT_SYMBOL_GPL(nf_tables_unbind_set);
 
+void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
+                             struct nft_set_binding *binding,
+                             enum nft_trans_phase phase)
+{
+       switch (phase) {
+       case NFT_TRANS_PREPARE:
+               set->use--;
+               return;
+       case NFT_TRANS_ABORT:
+       case NFT_TRANS_RELEASE:
+               set->use--;
+               /* fall through */
+       default:
+               nf_tables_unbind_set(ctx, set, binding,
+                                    phase == NFT_TRANS_COMMIT);
+       }
+}
+EXPORT_SYMBOL_GPL(nf_tables_deactivate_set);
+
 void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set)
 {
        if (list_empty(&set->bindings) && nft_set_is_anonymous(set))
@@ -6536,6 +6564,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
        struct nft_chain *chain;
        struct nft_table *table;
 
+       if (list_empty(&net->nft.commit_list)) {
+               mutex_unlock(&net->nft.commit_mutex);
+               return 0;
+       }
+
        /* 0. Validate ruleset, otherwise roll back for error reporting. */
        if (nf_tables_validate(net) < 0)
                return -EAGAIN;
@@ -6709,8 +6742,7 @@ static void nf_tables_abort_release(struct nft_trans *trans)
                nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
                break;
        case NFT_MSG_NEWSET:
-               if (!nft_trans_set_bound(trans))
-                       nft_set_destroy(nft_trans_set(trans));
+               nft_set_destroy(nft_trans_set(trans));
                break;
        case NFT_MSG_NEWSETELEM:
                nft_set_elem_destroy(nft_trans_elem_set(trans),
@@ -6783,8 +6815,11 @@ static int __nf_tables_abort(struct net *net)
                        break;
                case NFT_MSG_NEWSET:
                        trans->ctx.table->use--;
-                       if (!nft_trans_set_bound(trans))
-                               list_del_rcu(&nft_trans_set(trans)->list);
+                       if (nft_trans_set(trans)->bound) {
+                               nft_trans_destroy(trans);
+                               break;
+                       }
+                       list_del_rcu(&nft_trans_set(trans)->list);
                        break;
                case NFT_MSG_DELSET:
                        trans->ctx.table->use++;
@@ -6792,8 +6827,11 @@ static int __nf_tables_abort(struct net *net)
                        nft_trans_destroy(trans);
                        break;
                case NFT_MSG_NEWSETELEM:
+                       if (nft_trans_elem_set(trans)->bound) {
+                               nft_trans_destroy(trans);
+                               break;
+                       }
                        te = (struct nft_trans_elem *)trans->data;
-
                        te->set->ops->remove(net, te->set, &te->elem);
                        atomic_dec(&te->set->nelems);
                        break;

diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c
index a8a74a16f9c4bd2af02d00ef7daefe13a1eb5ab7..e461007558e8900f16bfb2ca3944d23987d5e669 100644 (file)
--- a/net/netfilter/nft_dynset.c
+++ b/net/netfilter/nft_dynset.c
@@ -240,11 +240,15 @@ static void nft_dynset_deactivate(const struct nft_ctx *ctx,
 {
        struct nft_dynset *priv = nft_expr_priv(expr);
 
-       if (phase == NFT_TRANS_PREPARE)
-               return;
+       nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
+}
+
+static void nft_dynset_activate(const struct nft_ctx *ctx,
+                               const struct nft_expr *expr)
+{
+       struct nft_dynset *priv = nft_expr_priv(expr);
 
-       nf_tables_unbind_set(ctx, priv->set, &priv->binding,
-                            phase == NFT_TRANS_COMMIT);
+       priv->set->use++;
 }
 
 static void nft_dynset_destroy(const struct nft_ctx *ctx,
@@ -292,6 +296,7 @@ static const struct nft_expr_ops nft_dynset_ops = {
        .eval           = nft_dynset_eval,
        .init           = nft_dynset_init,
        .destroy        = nft_dynset_destroy,
+       .activate       = nft_dynset_activate,
        .deactivate     = nft_dynset_deactivate,
        .dump           = nft_dynset_dump,
 };

diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c
index 14496da5141d3f1fa72ec4deb16fba3d8930c62c..161c3451a747a7632b0906159b3b982ef985a28a 100644 (file)
--- a/net/netfilter/nft_lookup.c
+++ b/net/netfilter/nft_lookup.c
@@ -127,11 +127,15 @@ static void nft_lookup_deactivate(const struct nft_ctx *ctx,
 {
        struct nft_lookup *priv = nft_expr_priv(expr);
 
-       if (phase == NFT_TRANS_PREPARE)
-               return;
+       nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
+}
+
+static void nft_lookup_activate(const struct nft_ctx *ctx,
+                               const struct nft_expr *expr)
+{
+       struct nft_lookup *priv = nft_expr_priv(expr);
 
-       nf_tables_unbind_set(ctx, priv->set, &priv->binding,
-                            phase == NFT_TRANS_COMMIT);
+       priv->set->use++;
 }
 
 static void nft_lookup_destroy(const struct nft_ctx *ctx,
@@ -222,6 +226,7 @@ static const struct nft_expr_ops nft_lookup_ops = {
        .size           = NFT_EXPR_SIZE(sizeof(struct nft_lookup)),
        .eval           = nft_lookup_eval,
        .init           = nft_lookup_init,
+       .activate       = nft_lookup_activate,
        .deactivate     = nft_lookup_deactivate,
        .destroy        = nft_lookup_destroy,
        .dump           = nft_lookup_dump,

diff --git a/net/netfilter/nft_objref.c b/net/netfilter/nft_objref.c
index 79ef074c18caf2b7323d7bbb41eec39c3432ea8f..457a9ceb46af2061546da95f46d05c3578c1826a 100644 (file)
--- a/net/netfilter/nft_objref.c
+++ b/net/netfilter/nft_objref.c
@@ -162,11 +162,15 @@ static void nft_objref_map_deactivate(const struct nft_ctx *ctx,
 {
        struct nft_objref_map *priv = nft_expr_priv(expr);
 
-       if (phase == NFT_TRANS_PREPARE)
-               return;
+       nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
+}
+
+static void nft_objref_map_activate(const struct nft_ctx *ctx,
+                                   const struct nft_expr *expr)
+{
+       struct nft_objref_map *priv = nft_expr_priv(expr);
 
-       nf_tables_unbind_set(ctx, priv->set, &priv->binding,
-                            phase == NFT_TRANS_COMMIT);
+       priv->set->use++;
 }
 
 static void nft_objref_map_destroy(const struct nft_ctx *ctx,
@@ -183,6 +187,7 @@ static const struct nft_expr_ops nft_objref_map_ops = {
        .size           = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)),
        .eval           = nft_objref_map_eval,
        .init           = nft_objref_map_init,
+       .activate       = nft_objref_map_activate,
        .deactivate     = nft_objref_map_deactivate,
        .destroy        = nft_objref_map_destroy,
        .dump           = nft_objref_map_dump,