Skip to content

Commit

Permalink
netfilter: unlock xt_table earlier in __do_replace
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
  • Loading branch information
lxin authored and ummakynes committed Mar 5, 2018
1 parent 2db3fec commit f31e5f1
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 3 deletions.
3 changes: 2 additions & 1 deletion net/ipv4/netfilter/arp_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion net/ipv4/netfilter/ip_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion net/ipv6/netfilter/ip6_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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:
Expand Down

0 comments on commit f31e5f1

Please sign in to comment.