netfilter: unlock xt_table earlier in __do_replace
authorXin Long <lucien.xin@gmail.com>
Fri, 16 Feb 2018 10:04:56 +0000 (18:04 +0800)
committerPablo Neira Ayuso <pablo@netfilter.org>
Mon, 5 Mar 2018 22:15:43 +0000 (23:15 +0100)
Now it's doing cleanup_entry for oldinfo under the xt_table lock,
but it's not really necessary. After the replacement job is done
in xt_replace_table, oldinfo is not used elsewhere any more, and
it can be freed without xt_table lock safely.

The important thing is that rtnl_lock is called in some xt_target
destroy, which means rtnl_lock, a big lock is used in xt_table
lock, a smaller one. It usually could be the reason why a dead
lock may happen.

Besides, all xt_target/match checkentry is called out of xt_table
lock. It's better also to move all cleanup_entry calling out of
xt_table lock, just as do_replace_finish does for ebtables.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
net/ipv4/netfilter/arp_tables.c
net/ipv4/netfilter/ip_tables.c
net/ipv6/netfilter/ip6_tables.c

index c36ffce3c81249cceb3ecdf8a2ad8ebd1161e41e..a0c7ce76879c498d62d3794455c758ead6ea68da 100644 (file)
@@ -925,6 +925,8 @@ static int __do_replace(struct net *net, const char *name,
            (newinfo->number <= oldinfo->initial_entries))
                module_put(t->me);
 
+       xt_table_unlock(t);
+
        get_old_counters(oldinfo, counters);
 
        /* Decrease module usage counts and free resource */
@@ -939,7 +941,6 @@ static int __do_replace(struct net *net, const char *name,
                net_warn_ratelimited("arptables: counters copy to user failed while replacing table\n");
        }
        vfree(counters);
-       xt_table_unlock(t);
        return ret;
 
  put_module:
index d4f7584d2dbec7bddb8b6ffb6fdff26365916fed..4f7153e25e0bf7b1dc355755af9cc76b0a3f3cf6 100644 (file)
@@ -1087,6 +1087,8 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
            (newinfo->number <= oldinfo->initial_entries))
                module_put(t->me);
 
+       xt_table_unlock(t);
+
        get_old_counters(oldinfo, counters);
 
        /* Decrease module usage counts and free resource */
@@ -1100,7 +1102,6 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
                net_warn_ratelimited("iptables: counters copy to user failed while replacing table\n");
        }
        vfree(counters);
-       xt_table_unlock(t);
        return ret;
 
  put_module:
index 4de8ac1e5af4a6c4137f04b0757015aa2c10104b..6c44033decab368c9e75783d29fa4a8de41ac026 100644 (file)
@@ -1105,6 +1105,8 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
            (newinfo->number <= oldinfo->initial_entries))
                module_put(t->me);
 
+       xt_table_unlock(t);
+
        get_old_counters(oldinfo, counters);
 
        /* Decrease module usage counts and free resource */
@@ -1118,7 +1120,6 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
                net_warn_ratelimited("ip6tables: counters copy to user failed while replacing table\n");
        }
        vfree(counters);
-       xt_table_unlock(t);
        return ret;
 
  put_module: