netfilter: Can't fail and free after table replacement

commit c58dd2dd443c26d856a168db108a0cd11c285bf3 upstream.

All xtables variants suffer from the defect that the copy_to_user()
to copy the counters to user memory may fail after the table has
already been exchanged and thus exposed. Return an error at this
point will result in freeing the already exposed table. Any
subsequent packet processing will result in a kernel panic.

We can't copy the counters before exposing the new tables as we
want provide the counter state after the old table has been
unhooked. Therefore convert this into a silent error.

Cc: Florian Westphal <fw@strlen.de>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 5fe2ff3b01efe44d07292514b814b0ba7b01d4f0..f381fa16bdc94fd2efe0bd019aa707856176b458 100644 (file)
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1044,10 +1044,9 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
        if (repl->num_counters &&
           copy_to_user(repl->counters, counterstmp,
           repl->num_counters * sizeof(struct ebt_counter))) {
-               ret = -EFAULT;
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("ebtables: counters copy to user failed while replacing table\n");
        }
-       else
-               ret = 0;
 
        /* decrease module count and free resources */
        EBT_ENTRY_ITERATE(table->entries, table->entries_size,

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index fd7a3f68917f1c53e4e8f6051eaa1e4ed728c2b3..bcb6e6197595ec2c4a152032d2534d6f1ed1d0be 100644 (file)
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -1039,8 +1039,10 @@ static int __do_replace(struct net *net, const char *name,
 
        xt_free_table_info(oldinfo);
        if (copy_to_user(counters_ptr, counters,
-                        sizeof(struct xt_counters) * num_counters) != 0)
-               ret = -EFAULT;
+                        sizeof(struct xt_counters) * num_counters) != 0) {
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("arptables: counters copy to user failed while replacing table\n");
+       }
        vfree(counters);
        xt_table_unlock(t);
        return ret;

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 24e556e83a3ba97fe633525c10e63d4bcb767b34..f98a1cf54c5b2ffd8f9fbc5231fce2d72a3067ce 100644 (file)
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1227,8 +1227,10 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 
        xt_free_table_info(oldinfo);
        if (copy_to_user(counters_ptr, counters,
-                        sizeof(struct xt_counters) * num_counters) != 0)
-               ret = -EFAULT;
+                        sizeof(struct xt_counters) * num_counters) != 0) {
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("iptables: counters copy to user failed while replacing table\n");
+       }
        vfree(counters);
        xt_table_unlock(t);
        return ret;

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 9d4e155593190d81b99af8988899a0204a73ec80..6fe8ced0068f8cc69dcfdb9ec95d8948cad918fc 100644 (file)
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1236,8 +1236,10 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 
        xt_free_table_info(oldinfo);
        if (copy_to_user(counters_ptr, counters,
-                        sizeof(struct xt_counters) * num_counters) != 0)
-               ret = -EFAULT;
+                        sizeof(struct xt_counters) * num_counters) != 0) {
+               /* Silent error, can't fail, new table is already in place */
+               net_warn_ratelimited("ip6tables: counters copy to user failed while replacing table\n");
+       }
        vfree(counters);
        xt_table_unlock(t);
        return ret;