netfilter: nf_tables: restore chain change atomicity

Chain counter validation is performed after the chain policy has
potentially been changed. Move counter validation/setting before
changing of the chain policy to fix this.

Additionally fix a memory leak if chain counter allocation fails
for new chains, remove an unnecessary free_percpu() and move
counter allocation for new chains

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 30fad4f6322f33f868d1668029599a0224607292..d275d384bbc57863027a66eb6c23fd8517771054 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -880,9 +880,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
                    !IS_ERR(nf_tables_chain_lookup(table, nla[NFTA_CHAIN_NAME])))
                        return -EEXIST;
 
-               if (nla[NFTA_CHAIN_POLICY])
-                       nft_base_chain(chain)->policy = policy;
-
                if (nla[NFTA_CHAIN_COUNTERS]) {
                        if (!(chain->flags & NFT_BASE_CHAIN))
                                return -EOPNOTSUPP;
@@ -893,6 +890,9 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
                                return err;
                }
 
+               if (nla[NFTA_CHAIN_POLICY])
+                       nft_base_chain(chain)->policy = policy;
+
                if (nla[NFTA_CHAIN_HANDLE] && name)
                        nla_strlcpy(chain->name, name, NFT_CHAIN_MAXNAMELEN);
 
@@ -934,6 +934,24 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
                if (basechain == NULL)
                        return -ENOMEM;
 
+               if (nla[NFTA_CHAIN_COUNTERS]) {
+                       err = nf_tables_counters(basechain,
+                                                nla[NFTA_CHAIN_COUNTERS]);
+                       if (err < 0) {
+                               kfree(basechain);
+                               return err;
+                       }
+               } else {
+                       struct nft_stats __percpu *newstats;
+
+                       newstats = alloc_percpu(struct nft_stats);
+                       if (newstats == NULL) {
+                               kfree(basechain);
+                               return -ENOMEM;
+                       }
+                       rcu_assign_pointer(basechain->stats, newstats);
+               }
+
                basechain->type = type;
                chain = &basechain->chain;
 
@@ -953,25 +971,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 
                chain->flags |= NFT_BASE_CHAIN;
                basechain->policy = policy;
-
-               if (nla[NFTA_CHAIN_COUNTERS]) {
-                       err = nf_tables_counters(basechain,
-                                                nla[NFTA_CHAIN_COUNTERS]);
-                       if (err < 0) {
-                               free_percpu(basechain->stats);
-                               kfree(basechain);
-                               return err;
-                       }
-               } else {
-                       struct nft_stats __percpu *newstats;
-
-                       newstats = alloc_percpu(struct nft_stats);
-                       if (newstats == NULL)
-                               return -ENOMEM;
-
-                       rcu_assign_pointer(nft_base_chain(chain)->stats,
-                                          newstats);
-               }
        } else {
                chain = kzalloc(sizeof(*chain), GFP_KERNEL);
                if (chain == NULL)