A recently reported crash in the x_tables framework seems to stem from
a potential race condition between adding rules to a table and having a
packet traversing the table at the same time.
In the crash, the jumpstack being used by the table traversal was freed
by the table replace code. After performing some bisection, it seems that
commit f31e5f1a891f ("netfilter: unlock xt_table earlier in __do_replace")
exposed this race condition by unlocking the table before the
get_old_counters() routine was called to perform the synchronization.
Call Stack:
Unable to handle kernel paging request at virtual address
006b6b6b6b6b6bc5
pc : ipt_do_table+0x3b8/0x660
lr : ipt_do_table+0x31c/0x660
Call trace:
ipt_do_table+0x3b8/0x660
iptable_mangle_hook+0x58/0xf8
nf_hook_slow+0x48/0xd8
__ip_local_out+0xf4/0x138
__ip_queue_xmit+0x348/0x3a0
ip_queue_xmit+0x10/0x18
Signed-off-by: Sean Tranchetti <stranche@xxxxxxxxxxxxxx>
---
net/ipv4/netfilter/arp_tables.c | 3 +--
net/ipv4/netfilter/ip_tables.c | 3 +--
net/ipv6/netfilter/ip6_tables.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f1f78a7..85cb189 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -921,8 +921,6 @@ 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 */
@@ -937,6 +935,7 @@ 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:
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 10b91eb..9f98bc5 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1076,8 +1076,6 @@ static int get_info(struct net *net, void __user *user,
(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 */
@@ -1091,6 +1089,7 @@ static int get_info(struct net *net, void __user *user,
net_warn_ratelimited("iptables: counters copy to user failed while replacing table\n");
}
vfree(counters);
+ xt_table_unlock(t);
return ret;
put_module:
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index c973ace..f2637bfb 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1093,8 +1093,6 @@ static int get_info(struct net *net, void __user *user,
(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 */
@@ -1108,6 +1106,7 @@ static int get_info(struct net *net, void __user *user,
net_warn_ratelimited("ip6tables: counters copy to user failed while replacing table\n");
}
vfree(counters);
+ xt_table_unlock(t);
return ret;
put_module:
--
1.9.1
